Add port type and range validation to micro. #320
Conversation
|
Great idea, thanks! |
| @@ -82,6 +82,15 @@ if (!existsSync(file)) { | |||
| process.exit(1) | |||
| } | |||
|
|
|||
| const { port, host } = flags | |||
| if (isNaN(port) || (!isNaN(port) && (port < 1 || port >= 2 ** 16))) { | |||
There was a problem hiding this comment.
You should prefer Number.isNaN over isNaN.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isNaN
There was a problem hiding this comment.
The 2 ** 16 statement is causing trouble for me on Node 8.9.1 6.5.0.
if (isNaN(port) || (!isNaN(port) && (port < 1 || port >= 2 ** 16))) {
^
SyntaxError: Unexpected token *
Any reason to use this over Math.pow or just port > 65535?
Edit: nvm was misbehaving and running this under Node 6.5.0, but it works fine under 8.9.1. Feel free to ignore the above :)
There was a problem hiding this comment.
shouldn't we specify an engines field in the package.json so we at least get a npm warning when installing ?
There was a problem hiding this comment.
Nothing against Math.pow, just sugar syntax there. And since micro expects the usage of async/await i thought something that was available earlier wouldn't hurt.
There was a problem hiding this comment.
for some reason the error message was not displayed in my case with node v 6.2.3
upgraded to 8.7 and 👌
|
@rapzo ^ |
|
On it. Thanks @sindresorhus! |
Assuming that unix sockets are not supported since the message identifies them as
undefined.I'll gladly patch micro-dev with the same, if the change makes sense.