CenterForDigitalHumanities / CenterForDigitalHumanities/TinyNode

tinyNode.js normalizePort() is not doing okay

Open
#52 0 comments 0 reactions 1 assignee Claimed by @cubap View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.