-
Notifications
You must be signed in to change notification settings - Fork 519
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
Canonicalize IPv6 address text form in rdata. #1013
Conversation
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.
Looks good. The only question I'd have is whether it's worth adding a canonicalize
for IPv4 as well, for consistency.
I went light because it's impossible for the system to accept a non-canonical IPv4 address, but if I went for full consistency I'd do it for IPv4 and dns.inet too. I'm glad to add it. |
I agree that an IPv4 |
467e543
to
3fbf4d8
Compare
try: | ||
return dns.ipv6.canonicalize(text) | ||
except Exception: | ||
try: | ||
return dns.ipv4.canonicalize(text) | ||
except Exception: | ||
raise ValueError |
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.
It doesn't matter at all, but all of the other code in this module tries IPv4 first.
|
||
Raises ``dns.exception.SyntaxError`` if the text is not valid. | ||
""" | ||
# Note that inet_aton() only accepts canonial form, but we still run through |
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.
canonical
Dnspython did not canonicalize the textual form of IPv6 address in rdata read from text format, which could possibly cause problems in application software if the address was used as a key.