On 2022/03/10 20:09, Volker Schlecht wrote: > On 3/10/22 15:13, Stuart Henderson wrote: > > > > https://github.com/nodejs/node/blob/v12.x/doc/api/net.md > > > > > > "If host is omitted, the server will accept connections on the unspecified > > > IPv6 address (::) when IPv6 is available, or the unspecified IPv4 address > > > (0.0.0.0) otherwise. > > > > > > In most operating systems, listening to the unspecified IPv6 address (::) > > > may cause the net.Server to also listen on the unspecified IPv4 address > > > (0.0.0.0)." > > > > > > Any chance that the code you're running is banking on the behavior of an > > > operating system with dual stack sockets? > > > > I think it's likely that nearly all code trying to create a network > > listener will bank on this. > > It's a sizable amount, judging by https://github.com/nodejs/node/issues/9390 > and a host of stackoverflow posts.
That's mostly talking about a related but different case, where the hostname is explicitly set to '::'. Parts of it touch on the case of a bind with no address specified but the main discussion is about ::. It's a pity they didn't go with the suggestion in https://github.com/nodejs/node/issues/9390#issuecomment-257651051 because that's straightforward to use, covers all the cases, and makes complete sense. Use :: or 0.0.0.0 if that's what you want, unspecified = either family. But that would mean listening on two sockets. > Node's response to all that, was to clarify the documentation. So this seems > to be a very conscious design decision, and by and large applications seem > to cope. To my eye they are largely ignoring the case of OS which always force v6only (or are configured to do that e.g. linux with /proc/sys/net/ipv6/bindv6only). The doc change they made sees more about trying to warn people that an explicit bind to :: may listen to v4 on some OS, than giving any advice to users of forced v6only OS.. > > postcodes.io is trickier, it is presented as more of a complete package > > and config is expected to be done via an .env file which doesn't allow > > setting a bind address. Obviously patchable but it makes things much more > > of a pain. > Still an easy enough fix, though, and upstream might be open to change > > src/server.ts:9 > const server = app.listen(port, '0.0.0.0'); > > because from a quick glance, their docker setup looks like it's ipv4 only. > > > Because OpenBSD has the no-v4-on-v6-sockets thing, for software that > > listens only to a single v6 socket by default, we would usually patch > > it to use v4. I think we should probably do this for node too, taking > > it back to the runtime behaviour of the old port for 12.x, but make it > > explicit rather than via a side-effect. > > Yet here we wouldn't be changing the default listening socket type of a > single application, we'd be intentionally changing the behaviour of a ported > language's standard library to the exact opposite of what its documentation > says. > > To me personally that feels wrong. Having said that, if it's the OpenBSD > way, let's do it of course. I don't like making gratuitous changes to ports, but in this case it feels like the upstream default is simply wrong for OpenBSD. Other OS mostly won't care - as the table in the gh issue shows, when they tested Windows, OS X and Ubuntu, they all listen to v4+v6 by default. > > I tried patching the only likely-looking place I could find (diff > > below) but it doesn't seem to be working for me though...not sure why.. > > I think the right place might be here: > > https://github.com/nodejs/node/blob/5386c6ac0da5fc28a010c5a78f80b82777fa530f/lib/net.js#L1297 > > - rval = createServerHandle(DEFAULT_IPV6_ADDR, port, 6, fd, flags); > + rval = createServerHandle(DEFAULT_IPV4_ADDR, port, 4, fd, flags); Good catch, that does the trick. I think it wants a little more so that address/addresstype are set correctly for errors and connectionKey, diff for that below. The alternative would be to document the changed behaviour, but it's quite a jump to go from "servers listening to ipv6 is broken" that we had in the package from at least 2018-2022 to "servers only listen to ipv6 and not v4 unless you modify your software or at least config", so I do prefer this if you don't mind too much. (Not ideal but hey at least it's not as bad as JDK's "can only use ipv4 addresses or ipv6 addresses but not both at the same time"! Pity the rest of the ecosystem didn't jump on "v6 sockets are only for v6" as it makes a lot of sense when configuring ACLs, for logging, etc). Index: Makefile =================================================================== RCS file: /cvs/ports/lang/node/Makefile,v retrieving revision 1.97 diff -u -p -r1.97 Makefile --- Makefile 16 Feb 2022 14:24:37 -0000 1.97 +++ Makefile 10 Mar 2022 21:26:11 -0000 @@ -8,6 +8,7 @@ USE_WXNEEDED = Yes COMMENT = JavaScript runtime built on Chrome's V8 JavaScript engine NODE_VERSION = v16.14.0 +REVISION = 0 PLEDGE_VER = 1.1.2 DISTFILES = node-pledge-{}${PLEDGE_VER}.tar.gz:0 \ ${DISTNAME}-headers.tar.gz \ Index: patches/patch-lib_net_js =================================================================== RCS file: patches/patch-lib_net_js diff -N patches/patch-lib_net_js --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ patches/patch-lib_net_js 10 Mar 2022 21:26:11 -0000 @@ -0,0 +1,41 @@ +- If an address is not specified, for the "any address" bind, +node was trying to bind to v6 out of preference and dropping back +to v4 if that fails. + +OpenBSD doesn't permit v4 connections to sockets bound to a v6 +address, change to binding to v4 for "any address" so this is a +better match for typical expectations. + +(Ideally it would create two separate binds to 0.0.0.0 and :: +for "any address" but that's not really a straightforward change). + + +Index: lib/net.js +--- lib/net.js.orig ++++ lib/net.js +@@ -1292,22 +1292,12 @@ function setupListenHandle(address, port, addressType, + + let rval = null; + +- // Try to bind to the unspecified IPv6 address, see if IPv6 is available + if (!address && typeof fd !== 'number') { +- rval = createServerHandle(DEFAULT_IPV6_ADDR, port, 6, fd, flags); +- +- if (typeof rval === 'number') { +- rval = null; +- address = DEFAULT_IPV4_ADDR; +- addressType = 4; +- } else { +- address = DEFAULT_IPV6_ADDR; +- addressType = 6; +- } ++ address = DEFAULT_IPV4_ADDR; ++ addressType = 4; + } + +- if (rval === null) +- rval = createServerHandle(address, port, addressType, fd, flags); ++ rval = createServerHandle(address, port, addressType, fd, flags); + + if (typeof rval === 'number') { + const error = uvExceptionWithHostPort(rval, 'listen', address, port);
