Skip to content
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

Fix negative port detection for IPv6 addresses on 32-bit #4911

Merged
merged 1 commit into from Jan 24, 2017

Conversation

rgacogne
Copy link
Member

Short description

On a 32-bit Arch, our test_ComboAddress unit test fails because ComboAddress("[::1]:-6") is considered valid. This is caused by stoul() not throwing for a negative value and returning an unsigned long value using unsigned integer wraparound rules. Since we used to store the result value in a signed int and treat negative values as if the port was not set, the test failed.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled and tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added regression tests
  • added unit tests

On a 32-bit Arch, our `test_ComboAddress` unit test fails because
`ComboAddress("[::1]:-6")` is considered valid. This is caused by
`stoul()` not throwing for a negative value and returning an `unsigned
long` value using unsigned integer wraparound rules. Since we used to
store the result value in a `signed int` and treat negative values
as if the port was not set, the test failed.
@pieterlexis pieterlexis merged commit 5753d90 into PowerDNS:master Jan 24, 2017
@rgacogne rgacogne deleted the invalid-v6-port-master branch January 24, 2017 12:09
pieterlexis added a commit that referenced this pull request Feb 17, 2017
Backport #4911: Fix negative port detection for IPv6 addresses on 32-bit
pieterlexis added a commit that referenced this pull request Feb 17, 2017
Backport #4911: Fix negative port detection for IPv6 addresses on 32-bit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants