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

Don't pass port 0 to portscanner #166

Closed
mzgoddard opened this issue Feb 18, 2015 · 3 comments
Closed

Don't pass port 0 to portscanner #166

mzgoddard opened this issue Feb 18, 2015 · 3 comments
Labels

Comments

@mzgoddard
Copy link
Contributor

iojs and 0.12 error if Socket.connect receives port 0. https://github.com/joyent/node/blob/master/lib/net.js#L901

grunt-contrib-connect needs to make sure not to pass port 0. This might just need to be updating tests and documenting this but maybe some changes to the task too.

@mzgoddard mzgoddard added the bug label Feb 18, 2015
@sindresorhus
Copy link
Member

This is a Node issue. This used to work and is a regression in Node.

nodejs/node-v0.x-archive#9194

@mzgoddard
Copy link
Contributor Author

@sindresorhus Its a regression in Node but its pretty intentional and gives a clearer error than the prior EADDRNOTAVAIL. Even if its a regression in Node, it seems incorrect to me that the code even tries to check port 0 with portscanner. portscanner checks ports by attempting to connect to them and in Node's case it doesn't make sense to bind to port 0 thinking it'll actually be port 0 instead of resulting in a random port. portscanner will always say that port 0 is available.

This appears to me as a misuse of portscanner.

@sindresorhus
Copy link
Member

This happens even without portscanner. portscanner is for the https://github.com/gruntjs/grunt-contrib-connect#useavailableport option, setting port to 0 is to get a random port. Different.

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

No branches or pull requests

2 participants