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

Canonicalize IPv6 address text form in rdata. #1013

Merged
merged 1 commit into from
Nov 16, 2023
Merged

Conversation

rthalley
Copy link
Owner

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.

Copy link
Collaborator

@bwelling bwelling left a 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.

@rthalley
Copy link
Owner Author

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.

@bwelling
Copy link
Collaborator

I agree that an IPv4 canonicalize would never return anything other than its input , but _as_ipv4_address could call it.

Comment on lines +191 to +197
try:
return dns.ipv6.canonicalize(text)
except Exception:
try:
return dns.ipv4.canonicalize(text)
except Exception:
raise ValueError
Copy link
Collaborator

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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

canonical

@rthalley rthalley merged commit cbb39a9 into master Nov 16, 2023
9 checks passed
@rthalley rthalley deleted the canonicalize-ipv6 branch November 16, 2023 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants