pbojinov / request-ip

A Node.js module for retrieving a request's IP address on the server.
https://npmjs.com/package/request-ip
MIT License
823 stars 102 forks source link

Correct IP validation logic order-of-operations #92

Open YSaxon opened 10 months ago

YSaxon commented 10 months ago

This pull request fixes the logic used for validating IP addresses.

Old code:

(existy(value) && regexes.ipv4.test(value)) || regexes.ipv6.test(value)

In pseudocode

If value is not null and is IPv4, return true.
If value is IPv6, return true.
Otherwise, return false.

The existing code evaluates regexes.ipv6.test(value) regardless of whether existy(value) returns true or false. This happens due to the parenthesis placement, which only associates existy(value) with regexes.ipv4.test(value).

New code:

existy(value) && (regexes.ipv4.test(value) || regexes.ipv6.test(value))

In psuedocode:

If value is null, return false.
If value is IPv4, return true.
If value is IPv6, return true.
Otherwise, return false.

The updated logic ensures that both regexes.ipv4.test(value) and regexes.ipv6.test(value) are evaluated only if existy(value) is true. This is achieved by encompassing the OR condition within parentheses, applying the existy(value) check to both IPv4 and IPv6 conditions.

Presumably the ipv6 regex doesn't throw an error when handling a null value, or you would have had bug reports about this before. So you could also consider just removing the null check. But if you want to keep the null check in there for efficiency, to quickly return false without applying the ipv4 regex if there is nothing to check, then this pull request will double that efficiency gain by also avoiding applying the ipv6 regex check to null values, without any additional null checking.