Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Random port: 0 doesn’t work on Node 0.12.0 #9194

Closed
jeffreydwalter opened this issue Feb 11, 2015 · 15 comments
Closed

Random port: 0 doesn’t work on Node 0.12.0 #9194

jeffreydwalter opened this issue Feb 11, 2015 · 15 comments
Assignees
Milestone

Comments

@jeffreydwalter
Copy link

gruntjs/grunt-contrib-connect#126

See the above issue link in the gruntjs project for details. This was apparently an issue in node v0.11.0, but no one filed it in the node project because v0.11.x was the experimental branch. This issue is now appearing in node v0.12.0.

To be clear, this issue doesn't appear to be an issue with gruntjs because I am seeing it in a completely different context in a project using the node-spdy library, which doesn't use gruntjs at all.

@trevnorris
Copy link

Can you tell me your OS? I'm not seeing this issue on Linux:

> process.version
'v0.12.1-pre'
> net.createServer().listen(0).address().port
48225

@jeffreydwalter
Copy link
Author

Sure, I'm running OSX 10.10.1. I haven't tested on Linux.

Did you happen to try the test that's in the link I pasted? I'd be curious to see if it passes on Linux.

@amir-s
Copy link

amir-s commented Feb 12, 2015

I dont have any problems in my Mac.

> process.version
'v0.12.0'
> net.createServer().listen(0).address().port
53114

@cjihrig
Copy link

cjihrig commented Feb 12, 2015

Without a stack trace, it's more difficult to say, but I'm guessing that the portscanner module used by grunt-contrib-connect is doing something like this:

require('net').createConnection({port: 0});

@mzgoddard
Copy link

I don't think this is a bug in node. This is a bug in grunt-contrib-connect and its use of node-portscanner by passing the 0 value as the port to start searching at. Sorry for the noise.

For clarity here is the related stack trace.

RangeError: port should be > 0 and < 65536: 0
    at Socket.connect (net.js:901:13)
    at Socket.connect (net.js:847:37)
    at Object.portscanner.checkPortStatus (/Users/zen/Code/grunt-contrib-connect/node_modules/portscanner/lib/portscanner.js:107:10)
    at checkNextPort (/Users/zen/Code/grunt-contrib-connect/node_modules/portscanner/lib/portscanner.js:124:17)
    at Object.async.until (/Users/zen/Code/grunt-contrib-connect/node_modules/portscanner/node_modules/async/lib/async.js:576:13)
    at findAPortWithStatus (/Users/zen/Code/grunt-contrib-connect/node_modules/portscanner/lib/portscanner.js:139:9)
    at Object.portscanner.findAPortNotInUse (/Users/zen/Code/grunt-contrib-connect/node_modules/portscanner/lib/portscanner.js:42:3)
    at /Users/zen/Code/grunt-contrib-connect/tasks/connect.js:172:21
    at fn (/Users/zen/Code/grunt-contrib-connect/node_modules/async/lib/async.js:641:34)
    at Immediate._onImmediate (/Users/zen/Code/grunt-contrib-connect/node_modules/async/lib/async.js:557:34)

@mzgoddard
Copy link

I don't know if I'm wrong about this or not but pointed out to me in the above issue, this is a regression from node 0.10. In node 0.10 connecting to port 0 or 65536 emits an EADDRNOTAVAIL error. Connecting to any port in the range that doesn't have a server bound to it emits an ECONNREFUSED error. Those match the RangeError but the the error has become synchronous and is creating this issue for portscanner and grunt-contrib-connect as portscanner was written expecting asynchronous errors.

Just figured I'd add this to the conversation having poked around a bit further.

@jeffreydwalter
Copy link
Author

I agree with @mzgoddard's astute observation. Right or wrong, this was not causing an issue in node 10.x. This appears to be a regression that is affecting multiple modules. I don't use the grunt modules but I am experiencing this issue with the node-spdy module.

@cjihrig
Copy link

cjihrig commented Feb 20, 2015

Some background on this:

The error message you are seeing was added to 0.11.8 in d80d131 to prevent non-int ports. However, it was possible that a DNS lookup would occur before validating the port, resulting in an asynchronously thrown (not emitted) exception. So, I moved the check to Socket.prototype.connect() in 0.11.15 in b636ba8. Because this is a check for invalid input, this should be thrown synchronously, as it is now.

The real question is whether or not calling createConnection({port: 0}) is a valid use case. If not, then this is not a regression. If so, then the check can be updated to allow 0.

@jeffreydwalter
Copy link
Author

I'm not sure if this is even related, but it seems like BSD-based operating systems treat port 0 as a "special" port, which may have something to do with the fact that this issue appears to only be affecting OSX and FreeBSD.

Found this on the web: "The designers of the original Berkeley UNIX "Sockets" interface, upon which much of the technology and practice we use today is based, set aside the specification of "port 0" to be used as a sort of "wild card" port. When programming the Sockets interface, the provision of a zero value is generally taken to mean "let the system choose one for me". Programmers who specify "port 0" know that it is an invalid port. They are asking the operating system to pick and assign whatever non-zero port is available and appropriate for their purpose."

Also, wikipedia (not that it's authoritative) lists 0 as a valid port: "The port numbers in the range from 0 to 1023 are the well-known ports or system ports."

@cjihrig
Copy link

cjihrig commented Feb 22, 2015

@trevnorris @tjfontaine @misterdjules thoughts? I don't see real harm in supporting port 0, even though I don't find it particularly useful on the client.

@trevnorris
Copy link

@cjihrig We should support port 0.

@cjihrig cjihrig self-assigned this Feb 23, 2015
@cjihrig cjihrig added this to the 0.12.1 milestone Feb 23, 2015
@jrabbe
Copy link

jrabbe commented Feb 24, 2015

It is standard behavior for most network system that binding to port 0 will give you a "random" available port. This has been the traditional functionality in node.js and it seems like a serious regression that it is no longer supported.

cjihrig added a commit that referenced this issue Mar 5, 2015
The added validation allows non-negative numbers and numeric
strings. All other values result in a thrown exception.

Fixes: #9194
PR-URL: #9268
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: James M Snell <jasnell@users.noreply.github.com>
@cjihrig
Copy link

cjihrig commented Mar 5, 2015

Closed in 1a2a4da

@quentindemetz
Copy link

When can we expect a release that includes this ? Thanks 😍

@misterdjules
Copy link

@quentindemetz The fix for this issue will be released in node v0.12.1. node v0.12.1 will be released when all issues and PRs in the the 0.12.1 milestone are closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants