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

Check that a relative name plus the zone's origin is not too long. #997

Merged
merged 1 commit into from
Oct 21, 2023

Conversation

rthalley
Copy link
Owner

Previously it was possible to add very long relative names to a relative zone which could never be rendered due to being too long for wire format. Now we check this as part of _validate_name().

This code also removes duplicated name validation code from Zone and Version, consolidating it into one helper function.

Finally, we fix a few comments in get methods that have cut-and-paste typos from the find variant indicating they can raise KeyError when they cannot.

@rthalley
Copy link
Owner Author

This is a pretty basic change but I made it a PR for code review since I refactored some _validate_name code to remove duplication. The other thing I think is review-worthy is my decision to keep with the general API behavior of _validate_name() of raising KeyError when there are problems. Before this change it did this for all problems other than errors when converting a string to a name (probably that should have gone to KeyError too, but not worth fixing now).

dns/zone.py Outdated
name = name.relativize(origin)
elif not relativize:
# We have a relative name in a non-relative zone, so derelativize.
name = name.derelativize(origin)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The new code below converts the NameTooLong into KeyError, but this code doesn't do that. Should it?

Copy link
Owner Author

Choose a reason for hiding this comment

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

It should. I pushed a corrected and even simpler bit of code.

Previously it was possible to add very long relative names to a
relative zone which could never be rendered due to being too long for
wire format.  Now we check this as part of _validate_name().

This code also removes duplicated name validation code from Zone and
Version, consolidating it into one helper function.

Finally, we fix a few comments in get methods that have cut-and-paste
typos from the find variant indicating they can raise KeyError when
they cannot.
@rthalley rthalley merged commit 1957961 into master Oct 21, 2023
9 checks passed
@rthalley rthalley deleted the check-long-relative branch October 21, 2023 13:38
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.

None yet

2 participants