Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: add comment for net.Server.listen IPv6 '::' #11134

Closed
wants to merge 1 commit into from

Conversation

jjqq2013
Copy link
Contributor

@jjqq2013 jjqq2013 commented Feb 3, 2017

See #9390

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. net Issues and PRs related to the net subsystem. labels Feb 3, 2017
@jjqq2013 jjqq2013 changed the title doc for net.Server.listen IPv6 '::'. See issues 9390 doc: add comment for net.Server.listen IPv6 '::' Feb 3, 2017
@jjqq2013 jjqq2013 force-pushed the doc-for-listen-all branch 3 times, most recently from 59dd9cb to 22536ef Compare February 3, 2017 07:38
doc/api/net.md Outdated
@@ -226,6 +226,10 @@ added: v0.1.90
Begin accepting connections on the specified `port` and `hostname`. If the
`hostname` is omitted, the server will accept connections on any IPv6 address
(`::`) when IPv6 is available, or any IPv4 address (`0.0.0.0`) otherwise.

Note that once IPv6 address (`::`) is listened, the OS may automatically
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I may suggest rewording this a bit:

*Note*: Depending on the operating system, listening to an IPv6 address such
as `'::'` may cause the `net.Server` to listen on an equivalent IPv4 address such
as `'0.0.0.0'`.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you. I will copy it.

@jjqq2013
Copy link
Contributor Author

jjqq2013 commented Feb 4, 2017

I'v changed comment as @jasnell recommended, just wrapped a word to keep line width. Please review again.

doc/api/net.md Outdated
@@ -226,6 +226,11 @@ added: v0.1.90
Begin accepting connections on the specified `port` and `hostname`. If the
`hostname` is omitted, the server will accept connections on any IPv6 address
(`::`) when IPv6 is available, or any IPv4 address (`0.0.0.0`) otherwise.

*Note*: Depending on the operating system, listening to an IPv6 address such
as `'::'` may cause the `net.Server` to listen on an equivalent IPv4 address
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. to listen -> to also listen (assuming that it is also not instead)
  2. What happens with normal IPv6 addresses (e.g. 2001:db8:85a3:0:0:8a2e:370:7334)? Does an equivalent address get listened on as well, or is it just unspecified (::->0.0.0.0) and localhost (::1->127.0.0.1)?
  3. Could we be more specific about which OSs support this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks.

  1. Modified: to listen -> to also listen.
  2. OK, i will check normal IPv6 address
  3. At least for ::, both Windows, Linux, Mac behave same.

Copy link
Contributor Author

@jjqq2013 jjqq2013 Feb 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. It seems NO problem with normal IPv6 addresses. At least i confirmed it at Mac which does not attempt to listen equivalent IPv4 address for normal IPv6 address. (On contrast, At same machine, for ::, it does).

Test result:

node -e 'net.createServer().listen({host:"fe80::82e6:50ff:fe16:4a2%en0", port:5555})'

Then run

netstat -an | grep 5555

On Mac OS X EI Capitan 10.11.6

tcp6       0      0  fe80::82e6:50ff:.5555  *.*                    LISTEN     

And indeed, nc IPv4:5555 not works.

On Ubuntu 16.04, unfortunately Node.JS 7.5.0 can not listen IPv6 address for some reason i don't know.
About windows, i will tested later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if it's just :: and ::1 that exhibit this behaviour, then we should probably just be explicit, something like:

... listening on one of the IPv6 addresses :: or ::1 may cause the net.Server to listen on the equivalent IPv4 address (0.0.0.0 or 127.0.0.1) ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems only for :: have this phenomenon.

OS Listen IPv6 address Does equivalent IPv4 auto listened
Windows 7Pro/10 :: YES
Mac OS X EI Capitan 10.11.6 :: YES
Ubuntu 14.04/16.04 :: YES
Windows 7Pro/10 ::1 NO
Mac OS X EI Capitan 10.11.6 ::1 NO
Ubuntu 14.04/16.04 ::1 NO
Windows 7Pro/10 ActualIPv6Address NO
Mac OS X EI Capitan 10.11.6 ActualIPv6Address NO
Ubuntu 14.04/16.04 ActualIPv6Address Unknown (failed to bind (invalid parameter)

@jjqq2013
Copy link
Contributor Author

jjqq2013 commented Feb 7, 2017

Someone help me please!

It seems only for :: have this confusing phenomenon, but i need someone do me a favour to test the last case in some Linux OS in Real Machine with IPv6 support.

OS Listen IPv6 address Does equivalent IPv4 auto listened
Windows 7Pro/10 :: YES
Mac OS X EI Capitan 10.11.6 :: YES
Ubuntu 14.04/16.04 :: YES
Windows 7Pro/10 ::1 NO
Mac OS X EI Capitan 10.11.6 ::1 NO
Ubuntu 14.04/16.04 ::1 NO
Windows 7Pro/10 ActualIPv6Address NO
Mac OS X EI Capitan 10.11.6 ActualIPv6Address NO
Ubuntu 14.04/16.04 ActualIPv6Address Unknown (failed to bind (invalid parameter)

Now the new problem is I can not confirm the last case in Linux, i'v tried on Amazon VPC, VirtualBox VM.

Both show error, even i use socat or nc it still show invalid parameter.

socat tcp6-listen:5555,bind=fe80:0000:0000:0000:04bf:1eff:fefe:8015,reuseaddr,fork -

Show

...socat[...] E bind(3, {AF=10 [fe80:0000:0000:0000:04bf:1eff:fefe:8015]:5555}, 28): Invalid argument

##Would anyone to test as following steps (use Node.js 7.0.0**+**) on a Linux OS ? ( must NOT inside a VM or VPC)

  1. Create a tcp server to listen at 5555 of some actual IPv5 address, please do not specify :: or ::1
node -e "net.createServer(c=>console.log).listen(5555,'AN_ACTUAL_IPv6_ADDR')" &
  1. Connect to the equivalent IPv4 address.
node -e "net.connect(5555,'THE_EQUIVALENT_IPv4_ADDR',()=>console.log('connected'))"

It's expected only step2 has error of ECONNREFUSED.

@jjqq2013
Copy link
Contributor Author

jjqq2013 commented Feb 7, 2017

It's difficult to confirm all environment, so I think maybe this pull request is not necessary, it is not problem of Node.js.

What i am concerned is that when user call listen(port) without specifying host, what the user really want is to listen any IPv6 and IPv4, not just any IPv6, but Node.js says it only listen IPv6 :: cause user worried.

doc/api/http.md Outdated
@@ -747,6 +747,10 @@ added: v0.1.90
Begin accepting connections on the specified `port` and `hostname`. If the
`hostname` is omitted, the server will accept connections on any IPv6 address
(`::`) when IPv6 is available, or any IPv4 address (`0.0.0.0`) otherwise.

*Note*: in most operating systems, listening to any IPv6 address (`'::'`) may
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe say to the unspecified IPv6 address instead of to any IPv6 address. I know it results in listening on all addresses, but saying any addresses suggests that it's true for other addresses like ::1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using any IPv6 address is just for historical reason: C socket() use the const INADDR_ANY.

Actually i want to change all any IPv? address to
the IPv? address INADDR_ANY(::), but i can not, just because this term are referenced in several places.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, @gibfahn , but we should define the term "unspecified", perhaps link to https://en.wikipedia.org/wiki/IPv6_address#Unspecified_address as well markup the word with _..._, so its clear that its a technical term. @QianJin2013 INADDR_ANY and :: are not the same thing, but I agree, any probably cames from the IPv4 macro name, which doesn't make sense when talking about an IPv6 address!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am glad to see more people want to use the term unspecified rather than traditional any. OK, i will modify it.

doc/api/http.md Outdated
@@ -747,6 +747,11 @@ added: v0.1.90
Begin accepting connections on the specified `port` and `hostname`. If the
`hostname` is omitted, the server will accept connections on any IPv6 address
(`::`) when IPv6 is available, or any IPv4 address (`0.0.0.0`) otherwise.

*Note*: in most operating systems, listening to `::`(_[unspecified IPv6
Copy link
Contributor

@sam-github sam-github Feb 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this markup work? I think it should be

listening to the [unspecified IPv6 address](https://en.wikipedia.org/wiki/IPv6_address#Unspecified_address) (`'::'`) may cause the `net.Server` to also listen on the _any_ IPv4 address (`'0.0.0.0'`).

so for both v4 and v6 the text has the name, and the parenthesis has the string/presentation format for the address class

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd change the the any IPv4 to the unspecified IPv4 as well.

Copy link
Contributor Author

@jjqq2013 jjqq2013 Feb 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sam-github thanks, the markup does work( see preview https://github.com/sjitech/node/blob/2c76b9e7218c5fc484c46c0564f6d99918bb8820/doc/api/http.md)

I do want to change for both v4 and v6, but i am not sure it's OK to do so.

@gibfahn thanks!

Copy link
Member

@gibfahn gibfahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM pending @sam-github's nit. Thanks for the work on this @QianJin2013!

@jjqq2013
Copy link
Contributor Author

jjqq2013 commented Feb 12, 2017

Finally, I'v also add a wiki link for IPv4 0.0.0.0. Can be viewed at https://github.com/sjitech/node/blob/4bc2604622d2fb27eaef24491b7bdaf25b59e4e2/doc/api/net.md#serverlistenport-hostname-backlog-callback

Begin accepting connections on the specified port and hostname. If the hostname 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.

Note: 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).

doc/api/http.md Outdated

*Note*: in most operating systems, listening to the [unspecified
IPv6 address](https://en.wikipedia.org/wiki/IPv6_address#Unspecified_address)
(`::`) may cause the `net.Server` to also listen on the [unspecified IPv4 address
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the actual link to the end and just use the [unspecified IPv4 address][] ref inline

doc/api/net.md Outdated
`hostname` is omitted, the server will accept connections on any IPv6 address
(`::`) when IPv6 is available, or any IPv4 address (`0.0.0.0`) otherwise.
`hostname` is omitted, the server will accept connections on the [unspecified
IPv6 address](https://en.wikipedia.org/wiki/IPv6_address#Unspecified_address)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this link to the end also with the [unspecified IPv6 address][] inline

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, done

jasnell pushed a commit that referenced this pull request Feb 17, 2017
Fixes: #9390
PR-URL: #11134
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@jasnell
Copy link
Member

jasnell commented Feb 17, 2017

Landed in 711e41b

@jasnell jasnell closed this Feb 17, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Feb 20, 2017
Fixes: nodejs#9390
PR-URL: nodejs#11134
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
italoacasas pushed a commit that referenced this pull request Feb 22, 2017
Fixes: #9390
PR-URL: #11134
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@italoacasas italoacasas mentioned this pull request Feb 25, 2017
@jasnell
Copy link
Member

jasnell commented Mar 7, 2017

would need a backport PR for v6 or v4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. net Issues and PRs related to the net subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants