Skip to content

Commit

Permalink
Add more detailed Domain errors
Browse files Browse the repository at this point in the history
Also, a bug in `Domain.try_from` was found while working on this.
The check for empty domain was incorrect, as the `split` will always
return at least one element (the original element when the separator is
never found) and so `labels.is_empty()` would never return `false`.
  • Loading branch information
matei-radu committed Jul 7, 2024
1 parent 8896dc4 commit c0bd067
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 10 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "dns_lib"
version = "0.6.0"
version = "0.7.0"
description = "An implementation of the DNS protocol from scratch based on the many DNS RFCs."

rust-version.workspace = true
Expand Down
38 changes: 30 additions & 8 deletions lib/src/domain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,19 @@ const MAX_LABEL_LENGTH: usize = 63;
const LABEL_SEPARATOR: char = '.';

#[derive(Debug)]
pub struct InvalidDomainError;
pub enum DomainError {
EmptyDomain,
InvalidLabelFormat,
}

impl fmt::Display for DomainError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Self::EmptyDomain => write!(f, "domain is empty"),
Self::InvalidLabelFormat => write!(f, "label is invalid"),
}
}
}

/// Representation of a DNS domain name.
///
Expand All @@ -39,7 +51,7 @@ pub struct Domain {
}

impl TryFrom<String> for Domain {
type Error = InvalidDomainError;
type Error = DomainError;

/// Tries to convert a [`String`] into a `Domain`.
///
Expand Down Expand Up @@ -67,7 +79,7 @@ impl TryFrom<String> for Domain {
}

impl TryFrom<&[u8]> for Domain {
type Error = InvalidDomainError;
type Error = DomainError;

/// Tries to convert a slice `&[u8]` into a `Domain`.
///
Expand All @@ -90,15 +102,15 @@ impl TryFrom<&[u8]> for Domain {
///
/// [RFC 1034, Section 3.5]: https://datatracker.ietf.org/doc/html/rfc1034#section-3.5
fn try_from(value: &[u8]) -> Result<Self, Self::Error> {
let labels: Vec<&[u8]> = value.split(|&byte| byte == LABEL_SEPARATOR as u8).collect();

if labels.is_empty() {
return Err(InvalidDomainError);
if value.is_empty() {
return Err(DomainError::EmptyDomain);
}

let labels: Vec<&[u8]> = value.split(|&byte| byte == LABEL_SEPARATOR as u8).collect();

for label in &labels {
if !bytes_are_label(label) {
return Err(InvalidDomainError);
return Err(DomainError::InvalidLabelFormat);
}
}

Expand Down Expand Up @@ -226,6 +238,16 @@ mod tests {
assert_eq!(result.unwrap(), ok);
}

#[rstest]
#[case("-.com", "label is invalid".to_string())]
#[case("sübway.com", "label is invalid".to_string())]
#[case("", "domain is empty".to_string())]
fn domain_try_from_string_fails(#[case] input: String, #[case] error_msg: String) {
let result = Domain::try_from(input);
assert!(result.is_err());
assert_eq!(result.unwrap_err().to_string(), error_msg);
}

#[rstest]
#[case("example")]
#[case("mercedes-benz.de")]
Expand Down

0 comments on commit c0bd067

Please sign in to comment.