CenterForDigitalHumanities / CenterForDigitalHumanities/TinyNode
tinyNode.js normalizePort() is not doing okay
- Dominant language
- JavaScript
- Stars
- 1
- Forks
- 3
- Avg merge
- 29m
- Merged PRs (30d)
- 1
Description
```
var port = normalizePort(process.env.PORT || '3002');
```
This is checking for PORT on .env or assigning "3002". `port` is supposed to be a `` so using the String '3002' is already goofy. Even if we cannot trust the .env, the `|| '3002'` (or `??`) should be outside the parenthesis.
```
function normalizePort(val) {
var port = parseInt(val, 10);
if (isNaN(port)) {
// named pipe
return val;
}
if (port >= 0) {
// port number
return port;
}
return false;
}
```
* Returning false here breaks `.listen()` which does not take `false` as a value.
* Detecting a named pipe also fails, since `.listen()` wants a number. It can fail through to a hostname or ip address, but that's not really what's happening.
* The `port >= 0` check does not actually validate the port, which must be an integer between 1-65535.
* `parseInt(val, 10)` does not need the "10" which is already the default. It also will parse things that are not like numbers ("1Love" => 1) without errors. `Number(val).toFixed()` will take any number-like and give back the integer piece.
* With all this, this function is probably only one line and may be better to just omit and decide whether an error should `catch` to apply the default or break the app.
Contributor guide
Assessment
This issue has not been assessed yet.