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

Require single class zonefiles by default, and give context if possible on parsing errors. #477

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ximon18
Copy link
Member

@ximon18 ximon18 commented Jan 9, 2025

Extend the inplace zone parser to detect violations of the RFC 1035 section 5.2 requirement that a zonefile consist of exactly one class, enabled by default, and extend its error reporting to say what the wrong and expected classes were (and also use this contextual error support for other errors where applicable).

This is a breaking change as it enables the validation checks by default.

…ection 5.2 requirement that a zonefile consist of exactly one class, enabled by default, and extend its error reporting to say what the wrong and expected classes were (and also use this contextual error support for other errors where applicable).
@ximon18 ximon18 added enhancement New feature or request breaking A PR that includes a breaking change. labels Jan 9, 2025
@ximon18 ximon18 requested a review from a team January 9, 2025 10:19
Copy link
Contributor

@bal-e bal-e 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 @ximon18!

Comment on lines +1589 to +1601
#[cfg(not(feature = "std"))]
{
f.write_str(self.msg)
}

#[cfg(feature = "std")]
{
if let Some(context) = &self.context {
f.write_fmt(format_args!("{}: {}", self.msg, context))
} else {
f.write_str(self.msg)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It could be more concise if you write out the message unconditionally:

f.write_str(self.msg)?;
#[cfg(feature = "std")]
if let Some(context) = &self.context {
    write!(f, ": {}", context)?;
}


(None, Some(last_class)) => last_class,

(None, None) => {
Copy link
Member

Choose a reason for hiding this comment

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

I think this is not correct. There is no default class according to RFC 1035. Do other parsers accept leaving out the class entirely?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking A PR that includes a breaking change. enhancement New feature or request needs-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants