Node 18 undici global about lab

[kanongil]

@devinivy I think that is overkill.

The number is very unlikely to change during a major, and for majors we usually have to update the globals whitelist anyway.

[kanongil]

I had a look into this, since I had already encountered the rogue global.

The global was added here to fix nodejs/undici#1331 and nodejs/node#42814. Interestingly, it only appears once some code has tried to use the experimental built-in implementation of fetch().

Essentially it allows the user-space undici package to expose a setGlobalDispatcher() method, that can customise how the new fetch() method handles requests.

I guess lab just needs to whitelist Symbol.for('undici.globalDispatcher.1') to fix this.

[devinivy]

It's noted in undici that the 1 is technically a version number, and will probably change in the future: https://github.com/nodejs/node/blob/5fad0b93667ffc6e4def52996b9529ac99b26319/deps/undici/src/lib/global.js#L3-L5. Would it be too weird for our automatic safe list to attempt to use fetch() in a way that eagerly fails? E.g. this usage await fetch() will fail with ERR_INVALID_URL but still set the global. This occurs in a worker thread, so I believe it shouldn't affect the globals on the main worker thread:

lab/lib/modules/leaks.js

Lines 112 to 126 in 0542146

	internals.getNodeGlobals = async () => {
	const nodeGlobalsWorker = new WorkerThreads.Worker(__filename);
	const [nodeGlobals] = await Events.once(nodeGlobalsWorker, 'message');
	return nodeGlobals;
	};
	if (!WorkerThreads.isMainThread) {
	// When this module is used as a worker, it posts back global property names and symbols
	WorkerThreads.parentPort.postMessage({
	allowed: Object.getOwnPropertyNames(globalThis),
	symbols: Object.getOwnPropertySymbols(globalThis).map(String)
	});
	}

[kanongil]

It's curious that node wants to pollute the global js scope, when it could have been solved by exposing a virtual node:undici module.

[devinivy]

I read that they wont consider adding it until fetch() is no longer experimental.

[dominykas]

Took a stab at implementation - there's a lazy require of undici when accessing FormData (and some other globals), which avoids having to call fetch() just to get it to throw.