-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpFoundation] Fixes /0 subnet handling in IpUtils #16177
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
Conversation
array(true, '192.168.1.1', array('1.2.3.4/1', '192.168.1.0/24')), | ||
array(true, '192.168.1.1', array('192.168.1.0/24', '1.2.3.4/1')), | ||
array(false, '192.168.1.1', array('1.2.3.4/1', '4.3.2.1/1')), | ||
array(true, '1.2.3.4', '0.0.0.0/0'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this case must be kept
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The case verifying 1.2.3.4
is in subnet 192.168.1.0/0
covers this code path, but I'll add it back anyway.
The AppVeyor build seems to have failed for reasons unrelated to this pull request - the Travis build passed which includes the unit tests I updated. Do all of the checks need to pass before the pull request can be merged, even if unrelated to this PR? |
Don't we have the same issue for the IPv6 check? |
Thank you @ultrafez. |
…afez) This PR was squashed before being merged into the 2.3 branch (closes #16177). Discussion ---------- [HttpFoundation] Fixes /0 subnet handling in IpUtils | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #16055 | License | MIT | Doc PR | Not needed Fixes bug #16055. For IP addresses with CIDR subnet length 0, the IP address must be valid - IPs with subnet masks greater than zero are implicitly validated due to the use of `ip2long` and `substr_compare` (although it's not particularly robust - there could be some future work to improve this here). Commits ------- d9ac571 [HttpFoundation] Fixes /0 subnet handling in IpUtils
Yes, there is the same issue with IPv6 addresses - I'm not as familiar with them as I am with IPv4, but if I get a chance I'll take a look and submit it as a new PR. |
Fixes bug #16055. For IP addresses with CIDR subnet length 0, the IP address must be valid - IPs with subnet masks greater than zero are implicitly validated due to the use of
ip2long
andsubstr_compare
(although it's not particularly robust - there could be some future work to improve this here).