[PedroRosalba]

Under -sNODERAWSOCKETS, lookupHost kept dns.lookup({all: true}) entries whose address failed inetPton4/inetPton6. A resolver that returns a non-address entry (for example a CNAME target such as github.com.) then made getaddrinfo throw TypeError: Cannot read properties of null (reading '0') in allocaddrinfo.

Such entries are now skipped. If none are left, the lookup fails with EAI_NONAME, the same as for an empty result.

test_dns_unparsed.c stubs dns.lookup via --pre-js, once with a mixed list and once with names only. It fails on main with the TypeError above and passes with this change.

Found while testing Tokio on Cloudflare Workers, whose dns.lookup currently returns CNAME names as addresses.

[gemini-code-assist]

Code Review

This pull request updates the Node.js socket library to skip DNS resolver entries that cannot be parsed (such as CNAME target names) instead of passing them as null addresses, resolving to EAI_NONAME if no valid addresses remain. It also adds a C test and a Python test stubbing dns.lookup to verify this behavior. Feedback on the changes suggests defensively guarding against null or undefined address entries in libsockfs_node.js to prevent potential runtime errors, and adding a fallback to require('dns') in the test stub to ensure compatibility with older Node.js versions that lack process.getBuiltinModule.

[gemini-code-assist]

To ensure robustness against malformed or unexpected resolver outputs (especially from non-conforming or custom environments), we should defensively guard against cases where a or a.address might be null or undefined. If a.address is missing, calling inetPton4 or inetPton6 will throw a TypeError (e.g., when trying to call .split() or other string methods on undefined).

          var entries = addresses.map((a) => {
            if (!a || !a.address) return null;
            return a.family === 6 ?
              { family: {{{ cDefs.AF_INET6 }}}, addr: inetPton6(a.address) } :
              { family: {{{ cDefs.AF_INET }}}, addr: inetPton4(a.address) };
          }).filter((e) => e !== null && e.addr !== null);

[gemini-code-assist]

The DNS stub currently only attempts to load the dns module via process.getBuiltinModule, which was introduced in Node.js v20.12.0 / v22.0.0. On older Node.js versions (such as Node.js v18 LTS or older v20 releases), process.getBuiltinModule will be undefined, causing the stub to be skipped and the test to fail. Adding a fallback to require('dns') ensures compatibility across all supported Node.js versions.

Suggested change

	if (typeof process == 'object' && process.getBuiltinModule) {
	var stubbed = {
	'mixed.test': [{ address: 'github.com.', family: 4 },
	{ address: '20.250.119.67', family: 4 },
	{ address: 'github.com.', family: 6 }],
	'names.test': [{ address: 'github.com.', family: 4 }],
	};
	var dns = process.getBuiltinModule('dns');
	var lookup = dns.lookup;
	dns.lookup = (name, opts, cb) => {
	if (!stubbed[name]) return lookup(name, opts, cb);
	setTimeout(() => cb(null, stubbed[name]), 0);
	};
	}
	var dns = (typeof process === 'object') && (process.getBuiltinModule ? process.getBuiltinModule('dns') : (typeof require === 'function' && require('dns')));
	if (dns) {
	var stubbed = {
	'mixed.test': [{ address: 'github.com.', family: 4 },
	{ address: '20.250.119.67', family: 4 },
	{ address: 'github.com.', family: 6 }],
	'names.test': [{ address: 'github.com.', family: 4 }],
	};
	var lookup = dns.lookup;
	dns.lookup = (name, opts, cb) => {
	if (!stubbed[name]) return lookup(name, opts, cb);
	setTimeout(() => cb(null, stubbed[name]), 0);
	};
	}

[guybedford]

Thank you for reporting this one. This is actually a workerd bug in the implementation of node:dns not following Node.js compatibility, and not a NODERAWSOCKETS issue itself. Under node:dns only valid IPs should be returned such that any validations here should not be needed.

I've instead posted a fix for this at cloudflare/workerd#7890.