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

Add support for CAA record type #434

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

weilence
Copy link
Contributor

@weilence weilence commented Nov 6, 2024

@ximon18 ximon18 added needs-review enhancement New feature or request labels Nov 8, 2024
@weilence weilence marked this pull request as ready for review November 26, 2024 14:51
@weilence
Copy link
Contributor Author

@ximon18 The Caa value field should be displayed as a CharStr, but it isn't actually a CharStr. I used CharStr::from_octets_unchecked in the Display and Debug implementations, but I’m not sure if this is acceptable or safe.

@@ -380,6 +380,22 @@ impl<Octs: AsRef<[u8]> + ?Sized> CharStr<Octs> {
.compose(target)?;
target.append_slice(self.0.as_ref())
}

/// Appends the canonical wire format representation to an octets builder.
pub fn compose_canonical<Target: OctetsBuilder + ?Sized>(
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 method produces the same result as compose only slower?

I don’t think we need an extra method here – I think using compose is actually better since it makes it clear that we don’t do anything special. (This is less important for char strings that don’t anything special, but for domain names it is important to signal that they are not lower cased and for consistency, using the same strategy here seems better to me.)

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

Successfully merging this pull request may close these issues.

3 participants