From 09efc0aa67c3d80a653f3b4adc5dde6022dd9533 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Mon, 16 Sep 2024 17:10:36 +0200 Subject: [PATCH 1/7] [base/name] Rewrite 'NameBuilder' --- src/base/name/builder.rs | 960 +++++++++++---------------------------- 1 file changed, 271 insertions(+), 689 deletions(-) diff --git a/src/base/name/builder.rs b/src/base/name/builder.rs index 4d551526b..c27a6096f 100644 --- a/src/base/name/builder.rs +++ b/src/base/name/builder.rs @@ -1,768 +1,348 @@ //! Building a domain name. //! -//! This is a private module for tidiness. `NameBuilder` and `PushError` +//! This is a private module for tidiness. `NameBuilder` and `BuildError` //! are re-exported by the parent module. -use super::super::scan::{BadSymbol, Symbol, SymbolCharsError, Symbols}; +use core::borrow::{Borrow, BorrowMut}; +use core::fmt; + use super::absolute::Name; -use super::relative::{RelativeName, RelativeNameError}; +use super::relative::RelativeName; +use super::uncertain::UncertainName; use super::traits::{ToName, ToRelativeName}; use super::Label; -#[cfg(feature = "bytes")] -use bytes::BytesMut; -use core::fmt; -use octseq::builder::{EmptyBuilder, FreezeBuilder, OctetsBuilder, ShortBuf}; -#[cfg(feature = "std")] -use std::vec::Vec; //------------ NameBuilder -------------------------------------------------- -/// Builds a domain name step by step by appending data. +/// An incremental builder for domain names. +/// +/// A [`NameBuilder`] manages a buffer and provides high-level operations to +/// incrementally construct a domain name in that buffer. The domain name can +/// be built out of individual bytes and/or whole labels. Once building is +/// complete, an absolute or relative domain name can be extracted. +/// +/// This type does not support internationalized domain names directly. Such +/// domain names must be punycoded before being passed in. +/// +/// # Usage /// -/// The domain name builder is the most fundamental way to construct a new -/// domain name. It wraps an octets builder that assembles the name step by -/// step. +/// To construct the builder, call [`new()`] (providing a pre-constructed +/// buffer) or [`default()`] (automatically constructing a buffer). /// -/// The methods [`push`][Self::push] and [`append_slice`][Self::append_slice] -/// to add the octets of a label to end of the builder. Once a label is -/// complete, [`end_label`][Self::end_label] finishes the current label and -/// starts a new one. +/// [`new()`]: Self::new() +/// [`default()`]: Self::default() /// -/// The method [`append_label`][Self::append_label] combines this process -/// and appends the given octets as a label. +/// If the domain name being constructed is already available as a sequence of +/// labels (where each label is a slice of bytes), call [`append_label()`] +/// repeatedly. If a label is available as a sequence of byte slices, it can be +/// incrementally constructed with [`append_slice()`], terminating with +/// [`end_label()`]. /// -/// The name builder currently is not aware of internationalized domain -/// names. The octets passed to it are used as is and are not converted. +/// [`append_label()`]: Self::append_label() +/// [`append_slice()`]: Self::append_slice() +/// [`end_label()`]: Self::end_label() +/// +/// Special care must be taken with absolute domain names, which contain a root +/// label (which is of zero length). The root label must attached with a call +/// to [`append()`], without calling [`end_label()`] after it. +/// +/// Once the entire domain name has been constructed, it can be extracted out of +/// the builder. Depending on the type of domain name required, call +/// [`as_absolute()`], [`as_relative()`], or [`as_uncertain()`]. These are also +/// available as [`From`] implementations. +/// +/// [`as_absolute()`]: Self::as_absolute() +/// [`as_relative()`]: Self::as_relative() +/// [`as_uncertain()`]: Self::as_uncertain() #[derive(Clone)] -pub struct NameBuilder { - /// The buffer to build the name in. - builder: Builder, +pub struct NameBuilder { + /// The offset to write the next byte to. + write_offset: u8, - /// The position in `octets` where the current label started. + /// The offset of the length octet of the current label. /// - /// If this is `None` we currently do not have a label. - head: Option, -} - -impl NameBuilder { - /// Creates a new domain name builder from an octets builder. + /// Invariants: /// - /// Whatever is in the buffer already is considered to be a relative - /// domain name. Since that may not be the case, this function is - /// unsafe. - pub(super) unsafe fn from_builder_unchecked(builder: Builder) -> Self { - NameBuilder { - builder, - head: None, - } - } - - /// Creates a new, empty name builder. - #[must_use] - pub fn new() -> Self - where - Builder: EmptyBuilder, - { - unsafe { NameBuilder::from_builder_unchecked(Builder::empty()) } - } + /// - `label_offset <= write_offset` + /// - if `label_offset < write_offset`: + /// - a label is currently being built. + /// - `buffer[label_offset] == 0`. + label_offset: u8, - /// Creates a new, empty builder with a given capacity. - #[must_use] - pub fn with_capacity(capacity: usize) -> Self - where - Builder: EmptyBuilder, - { - unsafe { - NameBuilder::from_builder_unchecked(Builder::with_capacity( - capacity, - )) - } - } - - /// Creates a new domain name builder atop an existing octets builder. + /// The buffer to build the name in. /// - /// The function checks that whatever is in the builder already - /// consititutes a correctly encoded relative domain name. - pub fn from_builder(builder: Builder) -> Result - where - Builder: OctetsBuilder + AsRef<[u8]>, - { - RelativeName::check_slice(builder.as_ref())?; - Ok(unsafe { NameBuilder::from_builder_unchecked(builder) }) - } -} - -#[cfg(feature = "std")] -impl NameBuilder> { - /// Creates an empty domain name builder atop a `Vec`. - #[must_use] - pub fn new_vec() -> Self { - Self::new() - } - - /// Creates an empty builder atop a `Vec` with given capacity. + /// Invariants: /// - /// Names are limited to a length of 255 octets, but you can provide any - /// capacity you like here. - #[must_use] - pub fn vec_with_capacity(capacity: usize) -> Self { - Self::with_capacity(capacity) - } + /// - `buffer[.. write_offset]` is initialized. + buffer: Buffer, } -#[cfg(feature = "bytes")] -impl NameBuilder { - /// Creates an empty domain name bulider atop a bytes value. - pub fn new_bytes() -> Self { - Self::new() - } - - /// Creates an empty bulider atop a bytes value with given capacity. +/// # Preparing +impl NameBuilder { + /// Construct a new [`NameBuilder`] using the given buffer. /// - /// Names are limited to a length of 255 octets, but you can provide any - /// capacity you like here. - pub fn bytes_with_capacity(capacity: usize) -> Self { - Self::with_capacity(capacity) + /// Any existing contents in the buffer will be ignored and overwritten. + #[must_use] + pub const fn new(buffer: Buffer) -> Self { + Self { + write_offset: 0, + label_offset: 0, + buffer, + } } } -impl> NameBuilder { - /// Returns the already assembled domain name as an octets slice. - pub fn as_slice(&self) -> &[u8] { - self.builder.as_ref() - } - - /// Returns the length of the already assembled domain name. +impl NameBuilder { + /// The total size of the built domain name, in bytes. pub fn len(&self) -> usize { - self.builder.as_ref().len() + self.write_offset as usize } - /// Returns whether the name is still empty. + /// Whether the builder is empty. pub fn is_empty(&self) -> bool { - self.builder.as_ref().is_empty() + self.write_offset == 0 } } -impl NameBuilder -where - Builder: OctetsBuilder + AsRef<[u8]> + AsMut<[u8]>, -{ - /// Returns whether there currently is a label under construction. +/// # Building +impl NameBuilder +where Buffer: Borrow<[u8; 256]> + BorrowMut<[u8; 256]> { + /// Append a whole label as a slice of bytes. /// - /// This returns `false` if the name is still empty or if the last thing - /// that happend was a call to [`end_label`]. + /// The label is added to the end of the domain name. /// - /// [`end_label`]: NameBuilder::end_label - pub fn in_label(&self) -> bool { - self.head.is_some() - } - - /// Attempts to append a slice to the underlying builder. + /// # Errors /// - /// This method doesn’t perform any checks but only does the necessary - /// error conversion. - fn _append_slice(&mut self, slice: &[u8]) -> Result<(), PushError> { - self.builder - .append_slice(slice) - .map_err(|_| PushError::ShortBuf) - } - - /// Pushes an octet to the end of the domain name. + /// Returns an error if the label is too big, or if appending it to the + /// domain name would make the domain name too big. /// - /// Starts a new label if necessary. Returns an error if pushing the - /// octet would exceed the size limits for labels or domain names. - pub fn push(&mut self, ch: u8) -> Result<(), PushError> { - let len = self.len(); - if len >= 254 { - return Err(PushError::LongName); - } - if let Some(head) = self.head { - if len - head > Label::MAX_LEN { - return Err(PushError::LongLabel); - } - self._append_slice(&[ch])?; - } else { - self.head = Some(len); - self._append_slice(&[0, ch])?; - } + /// # Panics + /// + /// Panics if a label was already being built. + pub fn append_label(&mut self, label: &[u8]) -> Result<(), BuildError> { + assert!(self.label_offset == self.write_offset, + "cannot append a whole label to a partially-built one"); + + // Ensure the new label will fit. + self.can_fit_label(label.len())?; + + let buffer = self.buffer.borrow_mut(); + buffer[self.write_offset as usize] = label.len() as u8; + buffer[self.write_offset as usize + 1 ..][.. label.len()] + .copy_from_slice(label); + self.write_offset += (1 + label.len()) as u8; + self.label_offset = self.write_offset; + Ok(()) } - /// Pushes a symbol to the end of the domain name. + /// Append a slice of bytes to the current label. /// - /// The symbol is iterpreted as part of the presentation format of a - /// domain name, i.e., an unescaped dot is considered a label separator. - pub fn push_symbol(&mut self, sym: Symbol) -> Result<(), FromStrError> { - if matches!(sym, Symbol::Char('.')) { - if !self.in_label() { - return Err(PresentationErrorEnum::EmptyLabel.into()); - } - self.end_label(); - Ok(()) - } else if matches!(sym, Symbol::SimpleEscape(b'[')) - && !self.in_label() - { - Err(LabelFromStrErrorEnum::BinaryLabel.into()) - } else { - self.push(sym.into_octet()?).map_err(Into::into) - } - } - - /// Appends the content of an octets slice to the end of the domain name. + /// If no label exists, a new label will be created. /// - /// Starts a new label if necessary. Returns an error if pushing - /// would exceed the size limits for labels or domain names. + /// # Errors /// - /// If `slice` is empty, does absolutely nothing. - pub fn append_slice(&mut self, slice: &[u8]) -> Result<(), PushError> { - if slice.is_empty() { - return Ok(()); - } - if let Some(head) = self.head { - if slice.len() > Label::MAX_LEN - (self.len() - head) { - return Err(PushError::LongLabel); - } - } else { - if slice.len() > Label::MAX_LEN { - return Err(PushError::LongLabel); - } - if self.len() + slice.len() > 254 { - return Err(PushError::LongName); - } - self.head = Some(self.len()); - self._append_slice(&[0])?; + /// Returns an error if the label being built is too big, or if appending it + /// to the current domain name would make the domain name too big. + pub fn append_slice(&mut self, data: &[u8]) -> Result<(), BuildError> { + // Ensure the label being built will fit. + self.can_fit_label(data.len() + self.cur_label() + .map_or(0, |l| l.len()))?; + + let buffer = self.buffer.borrow_mut(); + if self.label_offset == self.write_offset { + // Start a new label. + buffer[self.write_offset as usize] = 0; + self.write_offset += 1; } - self._append_slice(slice)?; + + // Append the new data. + buffer[self.write_offset as usize ..][.. data.len()] + .copy_from_slice(data); + self.write_offset += data.len() as u8; + Ok(()) } - /// Ends the current label. + /// End the current label, if any. + /// + /// # Panics /// - /// If there isn’t a current label, does nothing. + /// Panics if this is a root label (i.e. has zero length). pub fn end_label(&mut self) { - if let Some(head) = self.head { - let len = self.len() - head - 1; - self.builder.as_mut()[head] = len as u8; - self.head = None; + if self.label_offset < self.write_offset { + let len = self.write_offset - self.label_offset - 1; + assert!(len > 0, "cannot end a root label"); + self.buffer.borrow_mut()[self.label_offset as usize] = len; } } - /// Appends an octets slice as a complete label. + /// Check that a label can be appended to this builder. /// - /// If there currently is a label under construction, it will be ended - /// before appending `label`. - /// - /// Returns an error if `label` exceeds the label size limit of 63 bytes - /// or appending the label would exceed the domain name size limit of - /// 255 bytes. - pub fn append_label(&mut self, label: &[u8]) -> Result<(), PushError> { - let head = self.head; - self.end_label(); - if let Err(err) = self.append_slice(label) { - self.head = head; - return Err(err); + /// This ignores any partially-built label already in the builder; its size + /// should be included in the provided `label_size` parameter. + const fn can_fit_label(&self, label_size: usize) -> Result<(), BuildError> { + if label_size > Label::MAX_LEN { + Err(BuildError::LongLabel) + } else if self.label_offset as usize + 1 + label_size > Name::MAX_LEN { + Err(BuildError::LongName) + } else { + Ok(()) } - self.end_label(); - Ok(()) } +} - /// Appends a label with the decimal representation of `u8`. - /// - /// If there currently is a label under construction, it will be ended - /// before appending `label`. +/// # Inspecting +impl NameBuilder +where Buffer: Borrow<[u8; 256]> { + /// The domain name built thus far. /// - /// Returns an error if appending would result in a name longer than 254 - /// bytes. - pub fn append_dec_u8_label( - &mut self, - value: u8, - ) -> Result<(), PushError> { - self.end_label(); - let hecto = value / 100; - if hecto > 0 { - self.push(hecto + b'0')?; - } - let deka = (value / 10) % 10; - if hecto > 0 || deka > 0 { - self.push(deka + b'0')?; - } - self.push(value % 10 + b'0')?; - self.end_label(); - Ok(()) + /// This does not include any partially-built label. + pub fn cur_slice(&self) -> &[u8] { + &self.buffer.borrow()[.. self.label_offset as usize] } - /// Appends a label with the hex digit. - /// - /// If there currently is a label under construction, it will be ended - /// before appending `label`. + /// The current label being built, if any. /// - /// Returns an error if appending would result in a name longer than 254 - /// bytes. - pub fn append_hex_digit_label( - &mut self, - nibble: u8, - ) -> Result<(), PushError> { - fn hex_digit(nibble: u8) -> u8 { - match nibble & 0x0F { - 0 => b'0', - 1 => b'1', - 2 => b'2', - 3 => b'3', - 4 => b'4', - 5 => b'5', - 6 => b'6', - 7 => b'7', - 8 => b'8', - 9 => b'9', - 10 => b'A', - 11 => b'B', - 12 => b'C', - 13 => b'D', - 14 => b'E', - 15 => b'F', - _ => unreachable!(), - } + /// This does not include the length octet for the label. + pub fn cur_label(&self) -> Option<&[u8]> { + if self.label_offset < self.write_offset { + let label = self.label_offset as usize; + let write = self.write_offset as usize; + Some(&self.buffer.borrow()[label + 1 .. write]) + } else { + None } - - self.end_label(); - self.push(hex_digit(nibble))?; - self.end_label(); - Ok(()) } +} - /// Appends a relative domain name. +/// # Extracting +impl NameBuilder +where Buffer: Borrow<[u8; 256]> { + /// Extract an absolute domain name from the builder. /// - /// If there currently is a label under construction, it will be ended - /// before appending `name`. + /// If the name does not end with the root label (which has length zero), + /// [`None`] is returned. /// - /// Returns an error if appending would result in a name longer than 254 - /// bytes. - // - // XXX NEEDS TESTS - pub fn append_name( - &mut self, - name: &N, - ) -> Result<(), PushNameError> { - let head = self.head.take(); - self.end_label(); - if self.len() + usize::from(name.compose_len()) > 254 { - self.head = head; - return Err(PushNameError::LongName); - } - for label in name.iter_labels() { - label - .compose(&mut self.builder) - .map_err(|_| PushNameError::ShortBuf)?; - } - Ok(()) - } - - /// Appends a name from a sequence of symbols. + /// # Errors /// - /// If there currently is a label under construction, it will be ended - /// before appending `chars`. + /// Fails if the octet sequence type underlying the [`Name`] cannot + /// allocate enough space to store the domain name. /// - /// The character sequence must result in a domain name in representation - /// format. That is, its labels should be separated by dots, - /// actual dots, white space, backslashes and byte values that are not - /// printable ASCII characters should be escaped. + /// # Panics /// - /// The last label will only be ended if the last character was a dot. - /// Thus, you can determine if that was the case via - /// [`in_label`][Self::in_label]. - pub fn append_symbols>( - &mut self, - symbols: Sym, - ) -> Result<(), FromStrError> { - symbols - .into_iter() - .try_for_each(|symbol| self.push_symbol(symbol)) + /// Panics if a non-empty label was in the process of being built. + pub fn as_absolute<'a, Octs>(&'a self) + -> Result>, Octs::Error> + where Octs: TryFrom<&'a [u8]> { + assert!(self.write_offset <= self.label_offset + 1, + "cannot extract a domain name while a label is being built"); + + if self.write_offset == self.label_offset { + let buffer = &self.buffer.borrow()[.. self.write_offset as usize]; + let octseq = Octs::try_from(buffer)?; + Ok(Some(unsafe { + // SAFETY: `buffer` contains a valid name that was built through + // `NameBuilder`. A single root label is present, at the end. + Name::from_octets_unchecked(octseq) + })) + } else { + Ok(None) + } } - /// Appends a name from a sequence of characters. + /// Extract a relative domain name from the builder. + /// + /// # Errors /// - /// If there currently is a label under construction, it will be ended - /// before appending `chars`. + /// Fails if the octet sequence type underlying the [`RelativeName`] cannot + /// allocate enough space to store the domain name. /// - /// The character sequence must result in a domain name in representation - /// format. That is, its labels should be separated by dots, - /// actual dots, white space and backslashes should be escaped by a - /// preceeding backslash, and any byte value that is not a printable - /// ASCII character should be encoded by a backslash followed by its - /// three digit decimal value. + /// # Panics /// - /// The last label will only be ended if the last character was a dot. - /// Thus, you can determine if that was the case via - /// [`in_label`][Self::in_label]. - pub fn append_chars>( - &mut self, - chars: C, - ) -> Result<(), FromStrError> { - Symbols::with(chars.into_iter(), |symbols| { - self.append_symbols(symbols) + /// Panics if a label was in the process of being built. + pub fn as_relative<'a, Octs>(&'a self) + -> Result, Octs::Error> + where Octs: TryFrom<&'a [u8]> { + assert!(self.label_offset == self.write_offset, + "cannot extract a domain name while a label is being built"); + + let buffer = &self.buffer.borrow()[.. self.write_offset as usize]; + let octseq = Octs::try_from(buffer)?; + Ok(unsafe { + // SAFETY: `buffer` contains a valid name that was built through + // `NameBuilder`. No root labels are present. + // TODO: Worry about a 255-byte relative name? + RelativeName::from_octets_unchecked(octseq) }) } - /// Finishes building the name and returns the resulting relative name. + /// Extract an absolute or relative domain name from the builder. /// - /// If there currently is a label being built, ends the label first - /// before returning the name. I.e., you don’t have to call [`end_label`] - /// explicitely. + /// # Errors /// - /// This method converts the builder into a relative name. If you would - /// like to turn it into an absolute name, use [`into_name`] which - /// appends the root label before finishing. + /// Fails if the octet sequence type underlying the [`UncertainName`] cannot + /// allocate enough space to store the domain name. /// - /// [`end_label`]: NameBuilder::end_label - /// [`into_name`]: NameBuilder::into_name - pub fn finish(mut self) -> RelativeName - where - Builder: FreezeBuilder, - { - self.end_label(); - unsafe { RelativeName::from_octets_unchecked(self.builder.freeze()) } - } - - /// Appends the root label to the name and returns it as a [`Name`]. + /// # Panics /// - /// If there currently is a label under construction, ends the label. - /// Then adds the empty root label and transforms the name into a - /// [`Name`]. - pub fn into_name(mut self) -> Result, PushError> - where - Builder: FreezeBuilder, - { - self.end_label(); - self._append_slice(&[0])?; - Ok(unsafe { Name::from_octets_unchecked(self.builder.freeze()) }) - } - - /// Appends an origin and returns the resulting [`Name`]. - /// - /// If there currently is a label under construction, ends the label. - /// Then adds the `origin` and transforms the name into a - /// [`Name`]. - // - // XXX NEEDS TESTS - pub fn append_origin( - mut self, - origin: &N, - ) -> Result, PushNameError> - where - Builder: FreezeBuilder, - { - self.end_label(); - if self.len() + usize::from(origin.compose_len()) > Name::MAX_LEN { - return Err(PushNameError::LongName); - } - for label in origin.iter_labels() { - label - .compose(&mut self.builder) - .map_err(|_| PushNameError::ShortBuf)?; - } - Ok(unsafe { Name::from_octets_unchecked(self.builder.freeze()) }) - } -} - -//--- Default - -impl Default for NameBuilder { - fn default() -> Self { - Self::new() - } -} - -//--- AsRef + /// Panics if a non-empty label was in the process of being built. + pub fn as_uncertain<'a, Octs>(&'a self) + -> Result, Octs::Error> + where Octs: TryFrom<&'a [u8]> { + assert!(self.write_offset <= self.label_offset + 1, + "cannot extract a domain name while a label is being built"); -impl> AsRef<[u8]> for NameBuilder { - fn as_ref(&self) -> &[u8] { - self.builder.as_ref() - } -} - -//------------ Santa’s Little Helpers ---------------------------------------- - -/// Parses the contents of an escape sequence from `chars`. -/// -/// The backslash should already have been taken out of `chars`. -pub(super) fn parse_escape( - chars: &mut C, - in_label: bool, -) -> Result -where - C: Iterator, -{ - let ch = chars.next().ok_or(SymbolCharsError::short_input())?; - if ch.is_ascii_digit() { - let v = ch.to_digit(10).unwrap() * 100 - + chars - .next() - .ok_or(SymbolCharsError::short_input()) - .and_then(|c| { - c.to_digit(10).ok_or(SymbolCharsError::bad_escape()) - })? - * 10 - + chars - .next() - .ok_or(SymbolCharsError::short_input()) - .and_then(|c| { - c.to_digit(10).ok_or(SymbolCharsError::bad_escape()) - })?; - if v > 255 { - return Err(SymbolCharsError::bad_escape().into()); - } - Ok(v as u8) - } else if ch == '[' { - // `\[` at the start of a label marks a binary label which we don’t - // support. Within a label, the sequence is fine. - if in_label { - Ok(b'[') + let buffer = &self.buffer.borrow()[.. self.write_offset as usize]; + let octseq = Octs::try_from(buffer)?; + if self.write_offset == self.label_offset { + Ok(unsafe { + // SAFETY: `buffer` contains a valid name that was built through + // `NameBuilder`. A single root label is present, at the end. + Name::from_octets_unchecked(octseq).into() + }) } else { - Err(LabelFromStrErrorEnum::BinaryLabel.into()) + Ok(unsafe { + // SAFETY: `buffer` contains a valid name that was built through + // `NameBuilder`. No root labels are present. + // TODO: Worry about a 255-byte relative name? + RelativeName::from_octets_unchecked(octseq).into() + }) } - } else { - Ok(ch as u8) } } -//============ Error Types =================================================== - -//------------ PushError ----------------------------------------------------- - -/// An error happened while trying to push data to a domain name builder. -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum PushError { - /// The current label would exceed the limit of 63 bytes. - LongLabel, - - /// The name would exceed the limit of 255 bytes. - LongName, - - /// The buffer is too short to contain the name. - ShortBuf, -} - -//--- From - -impl From for PushError { - fn from(_: ShortBuf) -> PushError { - PushError::ShortBuf - } -} - -//--- Display and Error - -impl fmt::Display for PushError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match *self { - PushError::LongLabel => f.write_str("long label"), - PushError::LongName => f.write_str("long domain name"), - PushError::ShortBuf => ShortBuf.fmt(f), - } - } -} - -#[cfg(feature = "std")] -impl std::error::Error for PushError {} - -//------------ PushNameError ------------------------------------------------- - -/// An error happened while trying to push a name to a domain name builder. -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum PushNameError { - /// The name would exceed the limit of 255 bytes. - LongName, - - /// The buffer is too short to contain the name. - ShortBuf, -} - -//--- From - -impl From for PushNameError { - fn from(_: ShortBuf) -> Self { - PushNameError::ShortBuf - } -} - -//--- Display and Error - -impl fmt::Display for PushNameError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match *self { - PushNameError::LongName => f.write_str("long domain name"), - PushNameError::ShortBuf => ShortBuf.fmt(f), - } +impl Default for NameBuilder { + fn default() -> Self { + Self::new(Buffer::default()) } } -#[cfg(feature = "std")] -impl std::error::Error for PushNameError {} - -//------------ LabelFromStrError --------------------------------------------- +// TODO: impl 'ToName' / 'ToRelativeName' -/// An error occured while reading a label from a string. -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub struct LabelFromStrError(LabelFromStrErrorEnum); +//------------ BuildError ---------------------------------------------------- +/// An error in building a domain name. #[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub(super) enum LabelFromStrErrorEnum { - SymbolChars(SymbolCharsError), - - BadSymbol(BadSymbol), - - /// A binary label was encountered. - BinaryLabel, - - /// The label would exceed the limit of 63 bytes. +pub enum BuildError { + /// The label being built would exceed the 63-byte limit. LongLabel, -} - -//--- From - -impl From for LabelFromStrError { - fn from(inner: LabelFromStrErrorEnum) -> Self { - Self(inner) - } -} - -impl From for LabelFromStrError { - fn from(err: SymbolCharsError) -> Self { - Self(LabelFromStrErrorEnum::SymbolChars(err)) - } -} - -impl From for LabelFromStrError { - fn from(err: BadSymbol) -> Self { - Self(LabelFromStrErrorEnum::BadSymbol(err)) - } -} - -//--- Display and Error -impl fmt::Display for LabelFromStrError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self.0 { - LabelFromStrErrorEnum::SymbolChars(err) => err.fmt(f), - LabelFromStrErrorEnum::BadSymbol(err) => err.fmt(f), - LabelFromStrErrorEnum::BinaryLabel => { - f.write_str("a binary label was encountered") - } - LabelFromStrErrorEnum::LongLabel => { - f.write_str("label length limit exceeded") - } - } - } -} - -#[cfg(feature = "std")] -impl std::error::Error for LabelFromStrError {} - -//------------ FromStrError -------------------------------------------------- - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum FromStrError { - /// The string content was wrongly formatted. - Presentation(PresentationError), - - /// The buffer is too short to contain the name. - ShortBuf, -} - -impl FromStrError { - pub(super) fn empty_label() -> Self { - Self::Presentation(PresentationErrorEnum::EmptyLabel.into()) - } -} - -//--- From - -impl From for FromStrError { - fn from(err: PushError) -> FromStrError { - match err { - PushError::LongLabel => LabelFromStrErrorEnum::LongLabel.into(), - PushError::LongName => PresentationErrorEnum::LongName.into(), - PushError::ShortBuf => FromStrError::ShortBuf, - } - } -} - -impl From for FromStrError { - fn from(err: PushNameError) -> FromStrError { - match err { - PushNameError::LongName => PresentationErrorEnum::LongName.into(), - PushNameError::ShortBuf => FromStrError::ShortBuf, - } - } -} - -impl> From for FromStrError { - fn from(err: T) -> Self { - Self::Presentation(err.into()) - } -} - -//--- Display and Error - -impl fmt::Display for FromStrError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match *self { - FromStrError::Presentation(err) => err.fmt(f), - FromStrError::ShortBuf => ShortBuf.fmt(f), - } - } -} - -#[cfg(feature = "std")] -impl std::error::Error for FromStrError {} - -//------------ PresentationError --------------------------------------------- - -/// An illegal presentation format was encountered. -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub struct PresentationError(PresentationErrorEnum); - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -enum PresentationErrorEnum { - BadLabel(LabelFromStrError), - - /// An empty label was encountered. - EmptyLabel, - - /// The name has more than 255 characters. + /// The name being built would exceed the 255-byte limit. LongName, } -//--- From - -impl From for PresentationError { - fn from(err: PresentationErrorEnum) -> Self { - Self(err) - } -} - -impl> From for PresentationError { - fn from(err: T) -> Self { - Self(PresentationErrorEnum::BadLabel(err.into())) - } -} - -//--- Display and Error - -impl fmt::Display for PresentationError { +impl fmt::Display for BuildError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self.0 { - PresentationErrorEnum::BadLabel(ref err) => err.fmt(f), - PresentationErrorEnum::EmptyLabel => f.write_str("empty label"), - PresentationErrorEnum::LongName => { - f.write_str("long domain name") - } - } + f.write_str(match *self { + Self::LongLabel => "domain name label longer than 63 bytes", + Self::LongName => "domain name longer than 255 bytes", + }) } } #[cfg(feature = "std")] -impl std::error::Error for PresentationError {} +impl std::error::Error for BuildError {} //============ Testing ======================================================= @@ -773,96 +353,98 @@ mod test { #[test] fn compose() { - let mut builder = NameBuilder::new_vec(); - builder.push(b'w').unwrap(); + let mut builder = NameBuilder::new([0u8; 256]); + builder.append_slice(b"w").unwrap(); builder.append_slice(b"ww").unwrap(); builder.end_label(); builder.append_slice(b"exa").unwrap(); - builder.push(b'm').unwrap(); - builder.push(b'p').unwrap(); + builder.append_slice(b"m").unwrap(); + builder.append_slice(b"p").unwrap(); builder.append_slice(b"le").unwrap(); builder.end_label(); builder.append_slice(b"com").unwrap(); - assert_eq!(builder.finish().as_slice(), b"\x03www\x07example\x03com"); + builder.end_label(); + assert_eq!(builder.cur_slice(), b"\x03www\x07example\x03com"); } #[test] fn build_by_label() { - let mut builder = NameBuilder::new_vec(); + let mut builder = NameBuilder::new([0u8; 256]); builder.append_label(b"www").unwrap(); builder.append_label(b"example").unwrap(); builder.append_label(b"com").unwrap(); - assert_eq!(builder.finish().as_slice(), b"\x03www\x07example\x03com"); + assert_eq!(builder.cur_slice(), b"\x03www\x07example\x03com"); } #[test] fn build_mixed() { - let mut builder = NameBuilder::new_vec(); - builder.push(b'w').unwrap(); + let mut builder = NameBuilder::new([0u8; 256]); + builder.append_slice(b"w").unwrap(); builder.append_slice(b"ww").unwrap(); builder.append_label(b"example").unwrap(); builder.append_slice(b"com").unwrap(); - assert_eq!(builder.finish().as_slice(), b"\x03www\x07example\x03com"); + assert_eq!(builder.cur_slice(), b"\x03www\x07example\x03com"); } #[test] fn name_limit() { - let mut builder = NameBuilder::new_vec(); + let mut builder = NameBuilder::new([0u8; 256]); for _ in 0..25 { // 9 bytes label is 10 bytes in total builder.append_label(b"123456789").unwrap(); } - assert_eq!(builder.append_label(b"12345"), Err(PushError::LongName)); + assert_eq!(builder.append_label(b"12345"), Err(BuildError::LongName)); assert_eq!(builder.clone().append_label(b"1234"), Ok(())); - assert_eq!(builder.append_slice(b"12345"), Err(PushError::LongName)); + assert_eq!(builder.append_slice(b"12345"), Err(BuildError::LongName)); assert_eq!(builder.clone().append_slice(b"1234"), Ok(())); assert_eq!(builder.append_slice(b"12"), Ok(())); - assert_eq!(builder.push(b'3'), Ok(())); - assert_eq!(builder.push(b'4'), Err(PushError::LongName)) + assert_eq!(builder.append_slice(b"3"), Ok(())); + assert_eq!(builder.append_slice(b"4"), Err(BuildError::LongName)); } #[test] fn label_limit() { - let mut builder = NameBuilder::new_vec(); + let mut builder = NameBuilder::new([0u8; 256]); builder.append_label(&[0u8; 63][..]).unwrap(); assert_eq!( builder.append_label(&[0u8; 64][..]), - Err(PushError::LongLabel) + Err(BuildError::LongLabel) ); assert_eq!( builder.append_label(&[0u8; 164][..]), - Err(PushError::LongLabel) + Err(BuildError::LongLabel) ); builder.append_slice(&[0u8; 60][..]).unwrap(); builder.clone().append_label(b"123").unwrap(); - assert_eq!(builder.append_slice(b"1234"), Err(PushError::LongLabel)); + assert_eq!(builder.append_slice(b"1234"), Err(BuildError::LongLabel)); builder.append_slice(b"12").unwrap(); - builder.push(b'3').unwrap(); - assert_eq!(builder.push(b'4'), Err(PushError::LongLabel)); + builder.append_slice(b"3").unwrap(); + assert_eq!(builder.append_slice(b"4"), Err(BuildError::LongLabel)); } #[test] fn finish() { - let mut builder = NameBuilder::new_vec(); + let mut builder = NameBuilder::new([0u8; 256]); builder.append_label(b"www").unwrap(); builder.append_label(b"example").unwrap(); builder.append_slice(b"com").unwrap(); - assert_eq!(builder.finish().as_slice(), b"\x03www\x07example\x03com"); + assert_eq!(builder.cur_slice(), b"\x03www\x07example\x03com"); } #[test] - fn into_name() { - let mut builder = NameBuilder::new_vec(); + fn as_absolute() { + let mut builder = NameBuilder::new([0u8; 256]); builder.append_label(b"www").unwrap(); builder.append_label(b"example").unwrap(); builder.append_slice(b"com").unwrap(); + let name: Option> = builder.as_absolute().unwrap(); assert_eq!( - builder.into_name().unwrap().as_slice(), - b"\x03www\x07example\x03com\x00" + name.as_ref().map(Name::as_slice), + Some(&b"\x03www\x07example\x03com\x00"[..]) ); } } From 6d7f65fb508600e02b7453e34ec7c016ef85be70 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Tue, 17 Sep 2024 10:33:56 +0000 Subject: [PATCH 2/7] [base/name] Reformat 'builder.rs' --- src/base/name/builder.rs | 92 ++++++++++++++++++++++++++-------------- 1 file changed, 60 insertions(+), 32 deletions(-) diff --git a/src/base/name/builder.rs b/src/base/name/builder.rs index c27a6096f..7a292ce1c 100644 --- a/src/base/name/builder.rs +++ b/src/base/name/builder.rs @@ -8,8 +8,8 @@ use core::fmt; use super::absolute::Name; use super::relative::RelativeName; -use super::uncertain::UncertainName; use super::traits::{ToName, ToRelativeName}; +use super::uncertain::UncertainName; use super::Label; //------------ NameBuilder -------------------------------------------------- @@ -106,7 +106,9 @@ impl NameBuilder { /// # Building impl NameBuilder -where Buffer: Borrow<[u8; 256]> + BorrowMut<[u8; 256]> { +where + Buffer: Borrow<[u8; 256]> + BorrowMut<[u8; 256]>, +{ /// Append a whole label as a slice of bytes. /// /// The label is added to the end of the domain name. @@ -120,15 +122,17 @@ where Buffer: Borrow<[u8; 256]> + BorrowMut<[u8; 256]> { /// /// Panics if a label was already being built. pub fn append_label(&mut self, label: &[u8]) -> Result<(), BuildError> { - assert!(self.label_offset == self.write_offset, - "cannot append a whole label to a partially-built one"); + assert!( + self.label_offset == self.write_offset, + "cannot append a whole label to a partially-built one" + ); // Ensure the new label will fit. self.can_fit_label(label.len())?; let buffer = self.buffer.borrow_mut(); buffer[self.write_offset as usize] = label.len() as u8; - buffer[self.write_offset as usize + 1 ..][.. label.len()] + buffer[self.write_offset as usize + 1..][..label.len()] .copy_from_slice(label); self.write_offset += (1 + label.len()) as u8; self.label_offset = self.write_offset; @@ -146,8 +150,9 @@ where Buffer: Borrow<[u8; 256]> + BorrowMut<[u8; 256]> { /// to the current domain name would make the domain name too big. pub fn append_slice(&mut self, data: &[u8]) -> Result<(), BuildError> { // Ensure the label being built will fit. - self.can_fit_label(data.len() + self.cur_label() - .map_or(0, |l| l.len()))?; + self.can_fit_label( + data.len() + self.cur_label().map_or(0, |l| l.len()), + )?; let buffer = self.buffer.borrow_mut(); if self.label_offset == self.write_offset { @@ -157,7 +162,7 @@ where Buffer: Borrow<[u8; 256]> + BorrowMut<[u8; 256]> { } // Append the new data. - buffer[self.write_offset as usize ..][.. data.len()] + buffer[self.write_offset as usize..][..data.len()] .copy_from_slice(data); self.write_offset += data.len() as u8; @@ -181,10 +186,14 @@ where Buffer: Borrow<[u8; 256]> + BorrowMut<[u8; 256]> { /// /// This ignores any partially-built label already in the builder; its size /// should be included in the provided `label_size` parameter. - const fn can_fit_label(&self, label_size: usize) -> Result<(), BuildError> { + const fn can_fit_label( + &self, + label_size: usize, + ) -> Result<(), BuildError> { if label_size > Label::MAX_LEN { Err(BuildError::LongLabel) - } else if self.label_offset as usize + 1 + label_size > Name::MAX_LEN { + } else if self.label_offset as usize + 1 + label_size > Name::MAX_LEN + { Err(BuildError::LongName) } else { Ok(()) @@ -194,12 +203,14 @@ where Buffer: Borrow<[u8; 256]> + BorrowMut<[u8; 256]> { /// # Inspecting impl NameBuilder -where Buffer: Borrow<[u8; 256]> { +where + Buffer: Borrow<[u8; 256]>, +{ /// The domain name built thus far. /// /// This does not include any partially-built label. pub fn cur_slice(&self) -> &[u8] { - &self.buffer.borrow()[.. self.label_offset as usize] + &self.buffer.borrow()[..self.label_offset as usize] } /// The current label being built, if any. @@ -209,7 +220,7 @@ where Buffer: Borrow<[u8; 256]> { if self.label_offset < self.write_offset { let label = self.label_offset as usize; let write = self.write_offset as usize; - Some(&self.buffer.borrow()[label + 1 .. write]) + Some(&self.buffer.borrow()[label + 1..write]) } else { None } @@ -218,7 +229,9 @@ where Buffer: Borrow<[u8; 256]> { /// # Extracting impl NameBuilder -where Buffer: Borrow<[u8; 256]> { +where + Buffer: Borrow<[u8; 256]>, +{ /// Extract an absolute domain name from the builder. /// /// If the name does not end with the root label (which has length zero), @@ -232,14 +245,19 @@ where Buffer: Borrow<[u8; 256]> { /// # Panics /// /// Panics if a non-empty label was in the process of being built. - pub fn as_absolute<'a, Octs>(&'a self) - -> Result>, Octs::Error> - where Octs: TryFrom<&'a [u8]> { - assert!(self.write_offset <= self.label_offset + 1, - "cannot extract a domain name while a label is being built"); + pub fn as_absolute<'a, Octs>( + &'a self, + ) -> Result>, Octs::Error> + where + Octs: TryFrom<&'a [u8]>, + { + assert!( + self.write_offset <= self.label_offset + 1, + "cannot extract a domain name while a label is being built" + ); if self.write_offset == self.label_offset { - let buffer = &self.buffer.borrow()[.. self.write_offset as usize]; + let buffer = &self.buffer.borrow()[..self.write_offset as usize]; let octseq = Octs::try_from(buffer)?; Ok(Some(unsafe { // SAFETY: `buffer` contains a valid name that was built through @@ -261,13 +279,18 @@ where Buffer: Borrow<[u8; 256]> { /// # Panics /// /// Panics if a label was in the process of being built. - pub fn as_relative<'a, Octs>(&'a self) - -> Result, Octs::Error> - where Octs: TryFrom<&'a [u8]> { - assert!(self.label_offset == self.write_offset, - "cannot extract a domain name while a label is being built"); + pub fn as_relative<'a, Octs>( + &'a self, + ) -> Result, Octs::Error> + where + Octs: TryFrom<&'a [u8]>, + { + assert!( + self.label_offset == self.write_offset, + "cannot extract a domain name while a label is being built" + ); - let buffer = &self.buffer.borrow()[.. self.write_offset as usize]; + let buffer = &self.buffer.borrow()[..self.write_offset as usize]; let octseq = Octs::try_from(buffer)?; Ok(unsafe { // SAFETY: `buffer` contains a valid name that was built through @@ -287,13 +310,18 @@ where Buffer: Borrow<[u8; 256]> { /// # Panics /// /// Panics if a non-empty label was in the process of being built. - pub fn as_uncertain<'a, Octs>(&'a self) - -> Result, Octs::Error> - where Octs: TryFrom<&'a [u8]> { - assert!(self.write_offset <= self.label_offset + 1, - "cannot extract a domain name while a label is being built"); + pub fn as_uncertain<'a, Octs>( + &'a self, + ) -> Result, Octs::Error> + where + Octs: TryFrom<&'a [u8]>, + { + assert!( + self.write_offset <= self.label_offset + 1, + "cannot extract a domain name while a label is being built" + ); - let buffer = &self.buffer.borrow()[.. self.write_offset as usize]; + let buffer = &self.buffer.borrow()[..self.write_offset as usize]; let octseq = Octs::try_from(buffer)?; if self.write_offset == self.label_offset { Ok(unsafe { From a190e7a4f7cfc55ccd9ff9c2c0d851a2afe6a3a0 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Tue, 17 Sep 2024 10:34:25 +0000 Subject: [PATCH 3/7] [base/name/builder] Implement scanning This is a simple implementation with a different API from the previous code, designed to be replaced with a more efficient implementation in the future. --- src/base/name/builder.rs | 178 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 174 insertions(+), 4 deletions(-) diff --git a/src/base/name/builder.rs b/src/base/name/builder.rs index 7a292ce1c..968314a6f 100644 --- a/src/base/name/builder.rs +++ b/src/base/name/builder.rs @@ -11,6 +11,7 @@ use super::relative::RelativeName; use super::traits::{ToName, ToRelativeName}; use super::uncertain::UncertainName; use super::Label; +use crate::base::scan::{self, Symbol, Symbols}; //------------ NameBuilder -------------------------------------------------- @@ -123,7 +124,7 @@ where /// Panics if a label was already being built. pub fn append_label(&mut self, label: &[u8]) -> Result<(), BuildError> { assert!( - self.label_offset == self.write_offset, + self.cur_label().is_none(), "cannot append a whole label to a partially-built one" ); @@ -201,6 +202,117 @@ where } } +/// # Scanning +/// +/// The presentation format for domain names is somewhat underspecified. For +/// the purposes of this implementation, it is defined thusly: +/// +/// A domain name in the presentation format is a sequence of ASCII characters. +/// It is divided into a sequence of period-separated labels (possibly with one +/// terminating period representing the root label). Labels can contain almost +/// any printable ASCII character, except `"[].` (unless they are escaped). +/// +/// Backslashes represent escape sequences. A backslash followed by three ASCII +/// digits is interpreted as an octal-encoded byte value (with digits ordered +/// from most to least significant). If it is followed by a printable ASCII +/// character that is not a decimal digit, it is interpreted as that character +/// verbatim, even if that character is otherwise not allowed in a label. +/// +/// A binary label is one that begins and ends with escaped square brackets. It +/// is defined by a now-obsolete RFC and we choose not to support it. However, +/// as end-users may still write binary labels, we will detect them and provide +/// an appropriate error message instead of silently doing the wrong thing. +impl NameBuilder +where + Buffer: Borrow<[u8; 256]> + BorrowMut<[u8; 256]>, +{ + /// Scan and a label from the presentation format and append it. + /// + /// Given a label encoded in the presentation format, this function will + /// parse it, decode escape sequences within it, and append it to the name. + /// + /// # Errors + /// + /// Returns an error if the label is misformatted or too big, or if + /// appending it to the domain name would make the domain name too big. + /// + /// # Panics + /// + /// Panics if a label was already being built. + pub fn scan_label(&mut self, label: &str) -> Result<(), ScanError> { + assert!( + self.cur_label().is_none(), + "cannot append a whole label to a partially-built one" + ); + + // TODO: Rewrite to process the entire label in one go. + Symbols::with(label.chars(), |symbols| { + for symbol in symbols { + // Ensure it is not one of the disallowed characters. + if matches!( + symbol, + Symbol::Char('"' | '[' | ']' | '.' | ' ') + | Symbol::SimpleEscape(b' ' | b'\t' | b'\r' | b'\n') + ) { + return Err(ScanError::DisallowedChar); + } else if self.cur_label().is_none() + && matches!(symbol, Symbol::SimpleEscape(b'[')) + { + return Err(ScanError::BinaryLabel); + } + + self.append_slice(&[symbol.into_octet()?])?; + } + + Ok(()) + }) + } + + /// Scan a whole domain name from the presentation format and append it. + /// + /// Given a domain name encoded in the presentation format, this function + /// will parse it, decode escape sequences within it, and append it. + /// + /// # Errors + /// + /// Returns an error if the label is misformatted or too big, or if + /// appending it to the domain name would make the domain name too big. + /// + /// # Panics + /// + /// Panics if a label was already being built. + pub fn scan_name(&mut self, name: &str) -> Result<(), ScanError> { + assert!( + self.cur_label().is_none(), + "cannot append a whole name to a partially-built label" + ); + + // TODO: Rewrite to process the entire name in one go. + Symbols::with(name.chars(), |symbols| { + for symbol in symbols { + // Ensure it is not one of the disallowed characters. + if matches!( + symbol, + Symbol::Char('"' | '[' | ']' | ' ') + | Symbol::SimpleEscape(b' ' | b'\t' | b'\r' | b'\n') + ) { + return Err(ScanError::DisallowedChar); + } else if matches!(symbol, Symbol::Char('.')) { + self.end_label(); + } else { + self.append_slice(&[symbol.into_octet()?])?; + } + } + + // In case there was a root label, we want to append an empty slice. + // If there wasn't one, this has no effect. + self.append_slice(&[])?; + + Ok(()) + }) + } +} + /// # Inspecting impl NameBuilder where @@ -252,11 +364,11 @@ where Octs: TryFrom<&'a [u8]>, { assert!( - self.write_offset <= self.label_offset + 1, + !self.cur_label().is_some_and(|l| !l.is_empty()), "cannot extract a domain name while a label is being built" ); - if self.write_offset == self.label_offset { + if self.cur_label().is_some() { let buffer = &self.buffer.borrow()[..self.write_offset as usize]; let octseq = Octs::try_from(buffer)?; Ok(Some(unsafe { @@ -286,7 +398,7 @@ where Octs: TryFrom<&'a [u8]>, { assert!( - self.label_offset == self.write_offset, + self.cur_label().is_none(), "cannot extract a domain name while a label is being built" ); @@ -372,6 +484,64 @@ impl fmt::Display for BuildError { #[cfg(feature = "std")] impl std::error::Error for BuildError {} +//------------ ScanError ----------------------------------------------------- + +/// An error in scanning a domain name. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum ScanError { + /// An invalid escape sequence was encountered. + InvalidEscape, + + /// A disallowed character was encountered. + DisallowedChar, + + /// A binary label (which is unsupported) was encountered. + BinaryLabel, + + /// The label being built would exceed the 63-byte limit. + LongLabel, + + /// The name being built would exceed the 255-byte limit. + LongName, +} + +impl From for ScanError { + fn from(value: BuildError) -> Self { + match value { + BuildError::LongLabel => Self::LongLabel, + BuildError::LongName => Self::LongName, + } + } +} + +impl From for ScanError { + fn from(_value: scan::SymbolCharsError) -> Self { + Self::InvalidEscape + } +} + +impl From for ScanError { + fn from(_value: scan::BadSymbol) -> Self { + // TODO: If we could inspect BadSymbolEnum, we could be more specific. + Self::DisallowedChar + } +} + +impl fmt::Display for ScanError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.write_str(match *self { + Self::InvalidEscape => "invalid escape sequence found", + Self::DisallowedChar => "disallowed character found", + Self::BinaryLabel => "binary labels are unsupported", + Self::LongLabel => "domain name label longer than 63 bytes", + Self::LongName => "domain name longer than 255 bytes", + }) + } +} + +#[cfg(feature = "std")] +impl std::error::Error for ScanError {} + //============ Testing ======================================================= #[cfg(test)] From 2a9b312c1673532cdae1ff8419e931b10a03df68 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Tue, 17 Sep 2024 22:30:49 +0200 Subject: [PATCH 4/7] [WIP] further integrate the new 'NameBuilder' Most of the domain name types have been integrated with 'NameBuilder', abandoning previous support for 'from_chars()' or 'from_symbols()' and instead focusing on 'FromStr'. More complex inputs should be processed directly using 'NameBuilder' instead. --- src/base/name/absolute.rs | 142 +++++++++++--------------------------- src/base/name/builder.rs | 104 ++++++++++++++++++++++------ src/base/name/label.rs | 36 +++------- src/base/name/mod.rs | 6 +- src/base/name/relative.rs | 112 ++++++++++-------------------- 5 files changed, 172 insertions(+), 228 deletions(-) diff --git a/src/base/name/absolute.rs b/src/base/name/absolute.rs index 4633d2048..99e95902c 100644 --- a/src/base/name/absolute.rs +++ b/src/base/name/absolute.rs @@ -4,9 +4,9 @@ use super::super::cmp::CanonicalOrd; use super::super::net::IpAddr; -use super::super::scan::{Scanner, Symbol, SymbolCharsError, Symbols}; +use super::super::scan::Scanner; use super::super::wire::{FormError, ParseError}; -use super::builder::{FromStrError, NameBuilder, PushError}; +use super::builder::{BuildError, NameBuilder, ScanError}; use super::label::{Label, LabelTypeError, SplitLabelError}; use super::relative::{NameIter, RelativeName}; use super::traits::{FlattenInto, ToLabelIter, ToName}; @@ -15,9 +15,7 @@ use bytes::Bytes; use core::ops::{Bound, RangeBounds}; use core::str::FromStr; use core::{borrow, cmp, fmt, hash, mem, str}; -use octseq::builder::{ - EmptyBuilder, FreezeBuilder, FromBuilder, OctetsBuilder, Truncate, -}; +use octseq::builder::Truncate; use octseq::octets::{Octets, OctetsFrom}; use octseq::parse::Parser; #[cfg(feature = "serde")] @@ -87,76 +85,6 @@ impl Name { Ok(unsafe { Self::from_octets_unchecked(octets) }) } - pub fn from_symbols(symbols: Sym) -> Result - where - Octs: FromBuilder, - ::Builder: EmptyBuilder - + FreezeBuilder - + AsRef<[u8]> - + AsMut<[u8]>, - Sym: IntoIterator, - { - // NameBuilder can’t deal with a single dot, so we need to special - // case that. - let mut symbols = symbols.into_iter(); - let first = match symbols.next() { - Some(first) => first, - None => return Err(SymbolCharsError::short_input().into()), - }; - if first == Symbol::Char('.') { - if symbols.next().is_some() { - return Err(FromStrError::empty_label()); - } else { - // Make a root name. - let mut builder = - ::Builder::with_capacity(1); - builder - .append_slice(b"\0") - .map_err(|_| FromStrError::ShortBuf)?; - return Ok(unsafe { - Self::from_octets_unchecked(builder.freeze()) - }); - } - } - - let mut builder = NameBuilder::::new(); - builder.push_symbol(first)?; - builder.append_symbols(symbols)?; - builder.into_name().map_err(Into::into) - } - - /// Creates a domain name from a sequence of characters. - /// - /// The sequence must result in a domain name in representation format. - /// That is, its labels should be separated by dots. - /// Actual dots, white space and backslashes should be escaped by a - /// preceeding backslash, and any byte value that is not a printable - /// ASCII character should be encoded by a backslash followed by its - /// three digit decimal value. - /// - /// If Internationalized Domain Names are to be used, the labels already - /// need to be in punycode-encoded form. - /// - /// The name will always be an absolute name. If the last character in the - /// sequence is not a dot, the function will quietly add a root label, - /// anyway. In most cases, this is likely what you want. If it isn’t, - /// though, use [`UncertainName`] instead to be able to check. - /// - /// [`UncertainName`]: crate::base::name::UncertainName - pub fn from_chars(chars: C) -> Result - where - Octs: FromBuilder, - ::Builder: EmptyBuilder - + FreezeBuilder - + AsRef<[u8]> - + AsMut<[u8]>, - C: IntoIterator, - { - Symbols::with(chars.into_iter(), |symbols| { - Self::from_symbols(symbols) - }) - } - /// Reads a name in presentation format from the beginning of a scanner. pub fn scan>( scanner: &mut S, @@ -186,36 +114,45 @@ impl Name { /// /// The returned name will use the standard suffixes of `in-addr.arpa.` /// for IPv4 addresses and `ip6.arpa.` for IPv6. - pub fn reverse_from_addr(addr: IpAddr) -> Result + pub fn reverse_from_addr(addr: IpAddr) -> Result where - Octs: FromBuilder, - ::Builder: EmptyBuilder - + FreezeBuilder - + AsRef<[u8]> - + AsMut<[u8]>, + Octs: for<'a> TryFrom<&'a [u8]>, { - let mut builder = - NameBuilder::<::Builder>::new(); + let mut buffer = [0u8; 256]; + let mut builder = NameBuilder::new(&mut buffer); match addr { IpAddr::V4(addr) => { - let [a, b, c, d] = addr.octets(); - builder.append_dec_u8_label(d)?; - builder.append_dec_u8_label(c)?; - builder.append_dec_u8_label(b)?; - builder.append_dec_u8_label(a)?; + for octet in addr.octets().iter().rev() { + // Manually translate into decimal digits. + let mut buffer = [0u8; 3]; + buffer[0] = b'0' + octet / 100; + buffer[1] = b'0' + octet / 10 % 10; + buffer[2] = b'0' + octet % 10; + let len = octet.checked_ilog10().unwrap_or(1); + builder.append_label(&buffer[3 - len as usize..])?; + } builder.append_label(b"in-addr")?; builder.append_label(b"arpa")?; + builder.append_slice(b"")?; } IpAddr::V6(addr) => { for &item in addr.octets().iter().rev() { - builder.append_hex_digit_label(item)?; - builder.append_hex_digit_label(item >> 4)?; + // Manually translate into hexadecimal digits. + for nibble in [item, item >> 4] { + let base = b"0123456789ABCDEF"; + let label = [base[nibble as usize]]; + builder.append_label(&label)?; + } } builder.append_label(b"ip6")?; builder.append_label(b"arpa")?; + builder.append_slice(b"")?; } } - builder.into_name() + builder + .as_absolute() + .map(Option::unwrap) // We always add a root label. + .map_err(|_| BuildError::ShortBuf) } } @@ -292,7 +229,7 @@ impl Name> { } /// Creates a domain name atop a `Vec` from its string representation. - pub fn vec_from_str(s: &str) -> Result { + pub fn vec_from_str(s: &str) -> Result { FromStr::from_str(s) } } @@ -305,7 +242,7 @@ impl Name { } /// Creates a domain name atop a Bytes from its string representation. - pub fn bytes_from_str(s: &str) -> Result { + pub fn bytes_from_str(s: &str) -> Result { FromStr::from_str(s) } } @@ -770,13 +707,9 @@ where impl FromStr for Name where - Octs: FromBuilder, - ::Builder: EmptyBuilder - + FreezeBuilder - + AsRef<[u8]> - + AsMut<[u8]>, + Octs: for<'a> TryFrom<&'a [u8]>, { - type Err = FromStrError; + type Err = ScanError; /// Parses a string into an absolute domain name. /// @@ -790,8 +723,15 @@ where /// between those two cases, you can use [`UncertainDname`] instead. /// /// [`UncertainDname`]: struct.UncertainDname.html - fn from_str(s: &str) -> Result { - Self::from_chars(s.chars()) + fn from_str(name: &str) -> Result { + let mut builder = NameBuilder::new([0u8; 256]); + builder.scan_name(name)?; + // Append an empty slice for a root label. + builder.append_slice(&[]); + builder + .as_absolute() + .map(Option::unwrap) // We ensure there is a root label. + .map_err(|_| ScanError::ShortBuf) } } diff --git a/src/base/name/builder.rs b/src/base/name/builder.rs index 968314a6f..04b24e284 100644 --- a/src/base/name/builder.rs +++ b/src/base/name/builder.rs @@ -8,9 +8,8 @@ use core::fmt; use super::absolute::Name; use super::relative::RelativeName; -use super::traits::{ToName, ToRelativeName}; use super::uncertain::UncertainName; -use super::Label; +use super::{Label, ToLabelIter}; use crate::base::scan::{self, Symbol, Symbols}; //------------ NameBuilder -------------------------------------------------- @@ -183,6 +182,39 @@ where } } + /// Append a relative name to the current label. + /// + /// # Errors + /// + /// Returns an error if the domain name would become too big. + /// + /// # Panics + /// + /// Panics if a label was already being built. + pub fn append_relative_name( + &mut self, + name: RelativeName, + ) -> Result<(), BuildError> + where + Octs: AsRef<[u8]>, + { + assert!( + self.cur_label().is_none(), + "cannot append a name to a partially-built label" + ); + + if self.write_offset as usize + name.len() > Name::MAX_LEN { + return Err(BuildError::LongName); + } + + let buffer = self.buffer.borrow_mut(); + buffer[self.write_offset as usize..][..name.len()] + .copy_from_slice(name.as_ref()); + self.write_offset += name.len() as u8; + self.label_offset += name.len() as u8; + Ok(()) + } + /// Check that a label can be appended to this builder. /// /// This ignores any partially-built label already in the builder; its size @@ -251,8 +283,9 @@ where // Ensure it is not one of the disallowed characters. if matches!( symbol, - Symbol::Char('"' | '[' | ']' | '.' | ' ') - | Symbol::SimpleEscape(b' ' | b'\t' | b'\r' | b'\n') + Symbol::Char( + '"' | '[' | ']' | '.' | ' ' | '\t' | '\r' | '\n' + ) | Symbol::SimpleEscape(b' ' | b'\t' | b'\r' | b'\n') ) { return Err(ScanError::DisallowedChar); } else if self.cur_label().is_none() @@ -264,6 +297,10 @@ where self.append_slice(&[symbol.into_octet()?])?; } + if !label.is_empty() { + self.end_label(); + } + Ok(()) }) } @@ -293,20 +330,27 @@ where // Ensure it is not one of the disallowed characters. if matches!( symbol, - Symbol::Char('"' | '[' | ']' | ' ') - | Symbol::SimpleEscape(b' ' | b'\t' | b'\r' | b'\n') + Symbol::Char( + '"' | '[' | ']' | '.' | ' ' | '\t' | '\r' | '\n' + ) | Symbol::SimpleEscape(b' ' | b'\t' | b'\r' | b'\n') ) { return Err(ScanError::DisallowedChar); } else if matches!(symbol, Symbol::Char('.')) { + if self.cur_label().is_none() { + return Err(ScanError::EmptyLabel); + } self.end_label(); } else { self.append_slice(&[symbol.into_octet()?])?; } } - // In case there was a root label, we want to append an empty slice. - // If there wasn't one, this has no effect. - self.append_slice(&[])?; + // End the last label, or create the root label. + if self.cur_label().is_some() { + self.end_label(); + } else { + self.append_slice(&[])?; + } Ok(()) }) @@ -383,6 +427,9 @@ where /// Extract a relative domain name from the builder. /// + /// If the name ends with the root label (which has length zero), [`None`] + /// is returned. + /// /// # Errors /// /// Fails if the octet sequence type underlying the [`RelativeName`] cannot @@ -393,23 +440,27 @@ where /// Panics if a label was in the process of being built. pub fn as_relative<'a, Octs>( &'a self, - ) -> Result, Octs::Error> + ) -> Result>, Octs::Error> where Octs: TryFrom<&'a [u8]>, { assert!( - self.cur_label().is_none(), + !self.cur_label().is_some_and(|l| !l.is_empty()), "cannot extract a domain name while a label is being built" ); - let buffer = &self.buffer.borrow()[..self.write_offset as usize]; - let octseq = Octs::try_from(buffer)?; - Ok(unsafe { - // SAFETY: `buffer` contains a valid name that was built through - // `NameBuilder`. No root labels are present. - // TODO: Worry about a 255-byte relative name? - RelativeName::from_octets_unchecked(octseq) - }) + if self.cur_label().is_none() { + let buffer = &self.buffer.borrow()[..self.write_offset as usize]; + let octseq = Octs::try_from(buffer)?; + Ok(Some(unsafe { + // SAFETY: `buffer` contains a valid name that was built through + // `NameBuilder`. No root labels are present. + // TODO: Worry about a 255-byte relative name? + RelativeName::from_octets_unchecked(octseq) + })) + } else { + Ok(None) + } } /// Extract an absolute or relative domain name from the builder. @@ -458,13 +509,14 @@ impl Default for NameBuilder { } } -// TODO: impl 'ToName' / 'ToRelativeName' - //------------ BuildError ---------------------------------------------------- /// An error in building a domain name. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum BuildError { + /// The backing buffer is too short to store the data. + ShortBuf, + /// The label being built would exceed the 63-byte limit. LongLabel, @@ -475,6 +527,7 @@ pub enum BuildError { impl fmt::Display for BuildError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str(match *self { + Self::ShortBuf => "the backing buffer was too short", Self::LongLabel => "domain name label longer than 63 bytes", Self::LongName => "domain name longer than 255 bytes", }) @@ -498,6 +551,12 @@ pub enum ScanError { /// A binary label (which is unsupported) was encountered. BinaryLabel, + /// An empty (non-root) label was encountered. + EmptyLabel, + + /// The backing buffer is too short to store the data. + ShortBuf, + /// The label being built would exceed the 63-byte limit. LongLabel, @@ -508,6 +567,7 @@ pub enum ScanError { impl From for ScanError { fn from(value: BuildError) -> Self { match value { + BuildError::ShortBuf => Self::ShortBuf, BuildError::LongLabel => Self::LongLabel, BuildError::LongName => Self::LongName, } @@ -533,6 +593,8 @@ impl fmt::Display for ScanError { Self::InvalidEscape => "invalid escape sequence found", Self::DisallowedChar => "disallowed character found", Self::BinaryLabel => "binary labels are unsupported", + Self::EmptyLabel => "empty domain name label", + Self::ShortBuf => "the backing buffer was too short", Self::LongLabel => "domain name label longer than 63 bytes", Self::LongName => "domain name longer than 255 bytes", }) diff --git a/src/base/name/label.rs b/src/base/name/label.rs index 52d8b6817..fc1250137 100644 --- a/src/base/name/label.rs +++ b/src/base/name/label.rs @@ -3,11 +3,8 @@ //! This is a private module. Its public types are re-exported by the parent //! module. -use super::super::scan::BadSymbol; use super::super::wire::{FormError, ParseError}; -use super::builder::{ - parse_escape, LabelFromStrError, LabelFromStrErrorEnum, -}; +use super::{NameBuilder, ScanError}; use core::str::FromStr; use core::{borrow, cmp, fmt, hash, iter, mem, ops, slice}; use octseq::builder::OctetsBuilder; @@ -478,26 +475,6 @@ impl OwnedLabel { OwnedLabel(res) } - /// Creates a label from a sequence of chars. - pub fn from_chars( - mut chars: impl Iterator, - ) -> Result { - let mut res = [0u8; 64]; - while let Some(ch) = chars.next() { - if res[0] as usize >= Label::MAX_LEN { - return Err(LabelFromStrErrorEnum::LongLabel.into()); - } - let ch = match ch { - ' '..='-' | '/'..='[' | ']'..='~' => ch as u8, - '\\' => parse_escape(&mut chars, res[0] > 0)?, - _ => return Err(BadSymbol::non_ascii().into()), - }; - res[(res[0] as usize) + 1] = ch; - res[0] += 1; - } - Ok(OwnedLabel(res)) - } - /// Converts the label into the canonical form. /// /// This form has all octets representing ASCII letters converted to their @@ -539,10 +516,15 @@ impl<'a> From<&'a Label> for OwnedLabel { //--- FromStr impl FromStr for OwnedLabel { - type Err = LabelFromStrError; + type Err = ScanError; - fn from_str(s: &str) -> Result { - Self::from_chars(s.chars()) + fn from_str(label: &str) -> Result { + let mut builder = NameBuilder::new([0u8; 256]); + builder.scan_label(label)?; + // SAFETY: `NameBuilder` only builds valid labels. + let label = + unsafe { Label::from_slice_unchecked(builder.cur_slice()) }; + Ok(label.into()) } } diff --git a/src/base/name/mod.rs b/src/base/name/mod.rs index 42d079251..676ae0e14 100644 --- a/src/base/name/mod.rs +++ b/src/base/name/mod.rs @@ -92,9 +92,7 @@ //! [_punycode_]: pub use self::absolute::{Name, NameError}; -pub use self::builder::{ - FromStrError, NameBuilder, PresentationError, PushError, PushNameError, -}; +pub use self::builder::{BuildError, NameBuilder, ScanError}; pub use self::chain::{Chain, ChainIter, LongChainError, UncertainChainIter}; pub use self::label::{ Label, LabelTypeError, LongLabelError, OwnedLabel, SliceLabelsIter, @@ -102,7 +100,7 @@ pub use self::label::{ }; pub use self::parsed::{ParsedName, ParsedNameIter, ParsedSuffixIter}; pub use self::relative::{ - NameIter, RelativeFromStrError, RelativeName, RelativeNameError, + NameIter, RelativeName, RelativeNameError, RelativeScanError, StripSuffixError, }; pub use self::traits::{FlattenInto, ToLabelIter, ToName, ToRelativeName}; diff --git a/src/base/name/relative.rs b/src/base/name/relative.rs index e568d7f85..3e05e09af 100644 --- a/src/base/name/relative.rs +++ b/src/base/name/relative.rs @@ -4,7 +4,7 @@ use super::super::wire::ParseError; use super::absolute::Name; -use super::builder::{FromStrError, NameBuilder, PushError}; +use super::builder::{BuildError, NameBuilder, ScanError}; use super::chain::{Chain, LongChainError}; use super::label::{Label, LabelTypeError, SplitLabelError}; use super::traits::{ToLabelIter, ToRelativeName}; @@ -14,9 +14,7 @@ use core::cmp::Ordering; use core::ops::{Bound, RangeBounds}; use core::str::FromStr; use core::{borrow, cmp, fmt, hash, mem}; -use octseq::builder::{ - EmptyBuilder, FreezeBuilder, FromBuilder, IntoBuilder, Truncate, -}; +use octseq::builder::Truncate; use octseq::octets::{Octets, OctetsFrom}; #[cfg(feature = "serde")] use octseq::serde::{DeserializeOctets, SerializeOctets}; @@ -94,35 +92,6 @@ impl RelativeName { RelativeName::from_octets_unchecked(b"\x01*".as_ref().into()) } } - - /// Creates a domain name from a sequence of characters. - /// - /// The sequence must result in a domain name in representation format. - /// That is, its labels should be separated by dots. - /// Actual dots, white space and backslashes should be escaped by a - /// preceeding backslash, and any byte value that is not a printable - /// ASCII character should be encoded by a backslash followed by its - /// three digit decimal value. - /// - /// If Internationalized Domain Names are to be used, the labels already - /// need to be in punycode-encoded form. - pub fn from_chars(chars: C) -> Result - where - Octs: FromBuilder, - ::Builder: EmptyBuilder - + FreezeBuilder - + AsRef<[u8]> - + AsMut<[u8]>, - C: IntoIterator, - { - let mut builder = NameBuilder::::new(); - builder.append_chars(chars)?; - if builder.in_label() || builder.is_empty() { - Ok(builder.finish()) - } else { - Err(RelativeFromStrError::AbsoluteName) - } - } } impl RelativeName<[u8]> { @@ -211,7 +180,7 @@ impl RelativeName> { } /// Parses a string into a relative name atop a `Vec`. - pub fn vec_from_str(s: &str) -> Result { + pub fn vec_from_str(s: &str) -> Result { FromStr::from_str(s) } } @@ -229,7 +198,7 @@ impl RelativeName { } /// Parses a string into a relative name atop a `Bytes`. - pub fn bytes_from_str(s: &str) -> Result { + pub fn bytes_from_str(s: &str) -> Result { FromStr::from_str(s) } } @@ -281,29 +250,18 @@ impl RelativeName { } impl RelativeName { - /// Converts the name into a domain name builder for appending data. - /// - /// This method is only available for octets sequences that have an - /// associated octets builder such as `Vec` or `Bytes`. - pub fn into_builder(self) -> NameBuilder<::Builder> - where - Octs: IntoBuilder, - { - unsafe { NameBuilder::from_builder_unchecked(self.0.into_builder()) } - } - /// Converts the name into an absolute name by appending the root label. - /// - /// This manipulates the name itself and thus is only available for - /// octets sequences that can be converted into an octets builder and back - /// such as `Vec`. - pub fn into_absolute(self) -> Result, PushError> + pub fn to_absolute(&self) -> Result, BuildError> where - Octs: IntoBuilder, - ::Builder: - FreezeBuilder + AsRef<[u8]> + AsMut<[u8]>, + Octs: AsRef<[u8]> + for<'a> TryFrom<&'a [u8]>, { - self.into_builder().into_name() + let mut builder = NameBuilder::new([0u8; 256]); + builder.append_relative_name(self.for_ref())?; + builder.append_slice(&[])?; + builder + .as_absolute() + .map(Option::unwrap) // We added a root label manually. + .map_err(|_| BuildError::ShortBuf) } /// Chains another name to the end of this name. @@ -609,13 +567,9 @@ where impl FromStr for RelativeName where - Octs: FromBuilder, - ::Builder: EmptyBuilder - + FreezeBuilder - + AsRef<[u8]> - + AsMut<[u8]>, + Octs: for<'a> TryFrom<&'a [u8]>, { - type Err = RelativeFromStrError; + type Err = RelativeScanError; /// Parses a string into an absolute domain name. /// @@ -626,8 +580,16 @@ where /// /// This implementation will error if the name ends in a dot since that /// indicates an absolute name. - fn from_str(s: &str) -> Result { - Self::from_chars(s.chars()) + fn from_str(name: &str) -> Result { + let mut builder = NameBuilder::new([0u8; 256]); + builder.scan_name(name)?; + // If there was a root label, fail. + if builder.cur_label().is_some() { + return Err(RelativeScanError::AbsoluteName); + } + builder + .as_relative() + .map_err(|_| ScanError::ShortBuf.into()) } } @@ -1022,13 +984,13 @@ impl fmt::Display for RelativeNameError { #[cfg(feature = "std")] impl std::error::Error for RelativeNameError {} -//------------ RelativeFromStrError ------------------------------------------ +//------------ RelativeScanError ------------------------------------------ #[derive(Clone, Copy, Debug, Eq, PartialEq)] #[non_exhaustive] -pub enum RelativeFromStrError { +pub enum RelativeScanError { /// The name could not be parsed. - FromStr(FromStrError), + FromStr(ScanError), /// The parsed name was ended in a dot. AbsoluteName, @@ -1036,19 +998,19 @@ pub enum RelativeFromStrError { //--- From -impl From for RelativeFromStrError { - fn from(src: FromStrError) -> Self { +impl From for RelativeScanError { + fn from(src: ScanError) -> Self { Self::FromStr(src) } } //--- Display and Error -impl fmt::Display for RelativeFromStrError { +impl fmt::Display for RelativeScanError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - RelativeFromStrError::FromStr(err) => err.fmt(f), - RelativeFromStrError::AbsoluteName => { + RelativeScanError::FromStr(err) => err.fmt(f), + RelativeScanError::AbsoluteName => { f.write_str("absolute domain name") } } @@ -1056,7 +1018,7 @@ impl fmt::Display for RelativeFromStrError { } #[cfg(feature = "std")] -impl std::error::Error for RelativeFromStrError {} +impl std::error::Error for RelativeScanError {} //------------ StripSuffixError ---------------------------------------------- @@ -1236,13 +1198,13 @@ mod test { #[test] #[cfg(feature = "std")] - fn into_absolute() { + fn to_absolute() { assert_eq!( RelativeName::from_octets(Vec::from( b"\x03www\x07example\x03com".as_ref() )) .unwrap() - .into_absolute() + .to_absolute() .unwrap() .as_slice(), b"\x03www\x07example\x03com\0" @@ -1258,7 +1220,7 @@ mod test { tmp.extend_from_slice(b"\x03123"); RelativeName::from_octets(tmp) .unwrap() - .into_absolute() + .to_absolute() .unwrap(); } From fa78603f75ea713b455ec6ed84b310bb2472e30a Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Wed, 18 Sep 2024 13:54:56 +0200 Subject: [PATCH 5/7] Finish integrating 'NameBuilder' --- src/base/name/absolute.rs | 2 +- src/base/name/builder.rs | 2 +- src/base/name/relative.rs | 30 +++++++++--------- src/base/name/uncertain.rs | 63 ++++++-------------------------------- src/base/question.rs | 18 +++++------ src/base/scan.rs | 12 ++++---- 6 files changed, 41 insertions(+), 86 deletions(-) diff --git a/src/base/name/absolute.rs b/src/base/name/absolute.rs index 99e95902c..06d543d42 100644 --- a/src/base/name/absolute.rs +++ b/src/base/name/absolute.rs @@ -727,7 +727,7 @@ where let mut builder = NameBuilder::new([0u8; 256]); builder.scan_name(name)?; // Append an empty slice for a root label. - builder.append_slice(&[]); + builder.append_slice(&[])?; builder .as_absolute() .map(Option::unwrap) // We ensure there is a root label. diff --git a/src/base/name/builder.rs b/src/base/name/builder.rs index 04b24e284..f27af66af 100644 --- a/src/base/name/builder.rs +++ b/src/base/name/builder.rs @@ -7,9 +7,9 @@ use core::borrow::{Borrow, BorrowMut}; use core::fmt; use super::absolute::Name; +use super::label::Label; use super::relative::RelativeName; use super::uncertain::UncertainName; -use super::{Label, ToLabelIter}; use crate::base::scan::{self, Symbol, Symbols}; //------------ NameBuilder -------------------------------------------------- diff --git a/src/base/name/relative.rs b/src/base/name/relative.rs index 3e05e09af..e926daa10 100644 --- a/src/base/name/relative.rs +++ b/src/base/name/relative.rs @@ -14,7 +14,7 @@ use core::cmp::Ordering; use core::ops::{Bound, RangeBounds}; use core::str::FromStr; use core::{borrow, cmp, fmt, hash, mem}; -use octseq::builder::Truncate; +use octseq::builder::{FreezeBuilder, IntoBuilder, OctetsBuilder, Truncate}; use octseq::octets::{Octets, OctetsFrom}; #[cfg(feature = "serde")] use octseq::serde::{DeserializeOctets, SerializeOctets}; @@ -251,17 +251,20 @@ impl RelativeName { impl RelativeName { /// Converts the name into an absolute name by appending the root label. - pub fn to_absolute(&self) -> Result, BuildError> + pub fn into_absolute(self) -> Result, BuildError> where - Octs: AsRef<[u8]> + for<'a> TryFrom<&'a [u8]>, + Octs: AsRef<[u8]> + IntoBuilder, + Octs::Builder: FreezeBuilder, { - let mut builder = NameBuilder::new([0u8; 256]); - builder.append_relative_name(self.for_ref())?; - builder.append_slice(&[])?; + let mut builder = self.0.into_builder(); builder - .as_absolute() - .map(Option::unwrap) // We added a root label manually. - .map_err(|_| BuildError::ShortBuf) + .append_slice(&[0]) + .map_err(|_| BuildError::ShortBuf)?; + let buffer = builder.freeze(); + // SAFETY: We added a root label to a valid relative name, making it a + // valid absolute name. Relative names are at most 254 bytes, so the + // constructed absolute name is at most 255 bytes. + Ok(unsafe { Name::from_octets_unchecked(buffer) }) } /// Chains another name to the end of this name. @@ -583,13 +586,10 @@ where fn from_str(name: &str) -> Result { let mut builder = NameBuilder::new([0u8; 256]); builder.scan_name(name)?; - // If there was a root label, fail. - if builder.cur_label().is_some() { - return Err(RelativeScanError::AbsoluteName); - } builder .as_relative() .map_err(|_| ScanError::ShortBuf.into()) + .and_then(|n| n.ok_or(RelativeScanError::AbsoluteName)) } } @@ -1204,7 +1204,7 @@ mod test { b"\x03www\x07example\x03com".as_ref() )) .unwrap() - .to_absolute() + .into_absolute() .unwrap() .as_slice(), b"\x03www\x07example\x03com\0" @@ -1220,7 +1220,7 @@ mod test { tmp.extend_from_slice(b"\x03123"); RelativeName::from_octets(tmp) .unwrap() - .to_absolute() + .into_absolute() .unwrap(); } diff --git a/src/base/name/uncertain.rs b/src/base/name/uncertain.rs index 666465c11..29ebd22ff 100644 --- a/src/base/name/uncertain.rs +++ b/src/base/name/uncertain.rs @@ -5,7 +5,7 @@ use super::super::scan::Scanner; use super::super::wire::ParseError; use super::absolute::Name; -use super::builder::{FromStrError, NameBuilder, PushError}; +use super::builder::{BuildError, NameBuilder, ScanError}; use super::chain::{Chain, LongChainError}; use super::label::{Label, LabelTypeError, SplitLabelError}; use super::relative::{NameIter, RelativeName}; @@ -13,9 +13,7 @@ use super::traits::ToLabelIter; #[cfg(feature = "bytes")] use bytes::Bytes; use core::{fmt, hash, str}; -use octseq::builder::{ - EmptyBuilder, FreezeBuilder, FromBuilder, IntoBuilder, -}; +use octseq::builder::{FreezeBuilder, IntoBuilder}; #[cfg(feature = "serde")] use octseq::serde::{DeserializeOctets, SerializeOctets}; #[cfg(feature = "std")] @@ -104,44 +102,6 @@ impl UncertainName { } } - /// Creates a domain name from a sequence of characters. - /// - /// The sequence must result in a domain name in zone file - /// representation. That is, its labels should be separated by dots, - /// while actual dots, white space and backslashes should be escaped by a - /// preceeding backslash, and any byte value that is not a printable - /// ASCII character should be encoded by a backslash followed by its - /// three digit decimal value. - /// - /// If Internationalized Domain Names are to be used, the labels already - /// need to be in punycode-encoded form. - /// - /// If the last character is a dot, the name will be absolute, otherwise - /// it will be relative. - /// - /// If you have a string, you can also use the [`FromStr`] trait, which - /// really does the same thing. - /// - /// [`FromStr`]: std::str::FromStr - pub fn from_chars(chars: C) -> Result - where - Octets: FromBuilder, - ::Builder: FreezeBuilder - + EmptyBuilder - + AsRef<[u8]> - + AsMut<[u8]>, - C: IntoIterator, - { - let mut builder = - NameBuilder::<::Builder>::new(); - builder.append_chars(chars)?; - if builder.in_label() || builder.is_empty() { - Ok(builder.finish().into()) - } else { - Ok(builder.into_name()?.into()) - } - } - pub fn scan>>( scanner: &mut S, ) -> Result { @@ -225,11 +185,10 @@ impl UncertainName { /// /// If the name is relative, appends the root label to it using /// [`RelativeName::into_absolute`]. - pub fn into_absolute(self) -> Result, PushError> + pub fn to_absolute(self) -> Result, BuildError> where Octets: AsRef<[u8]> + IntoBuilder, - ::Builder: - FreezeBuilder + AsRef<[u8]> + AsMut<[u8]>, + Octets::Builder: FreezeBuilder, { match self { UncertainName::Absolute(name) => Ok(name), @@ -313,16 +272,14 @@ impl From> for UncertainName { impl str::FromStr for UncertainName where - Octets: FromBuilder, - ::Builder: EmptyBuilder - + FreezeBuilder - + AsRef<[u8]> - + AsMut<[u8]>, + Octets: for<'a> TryFrom<&'a [u8]>, { - type Err = FromStrError; + type Err = ScanError; - fn from_str(s: &str) -> Result { - Self::from_chars(s.chars()) + fn from_str(name: &str) -> Result { + let mut builder = NameBuilder::new([0u8; 256]); + builder.scan_name(name)?; + builder.as_uncertain().map_err(|_| ScanError::ShortBuf) } } diff --git a/src/base/question.rs b/src/base/question.rs index 92b8f370e..278737638 100644 --- a/src/base/question.rs +++ b/src/base/question.rs @@ -128,7 +128,7 @@ impl From<(N, Rtype)> for Question { } } -impl> FromStr for Question { +impl> FromStr for Question { type Err = FromStrError; /// Parses a question from a string. @@ -380,13 +380,11 @@ pub enum FromStrError { //--- From -impl From for FromStrError { - fn from(err: name::FromStrError) -> FromStrError { +impl From for FromStrError { + fn from(err: name::ScanError) -> FromStrError { match err { - name::FromStrError::Presentation(err) => { - Self::Presentation(err.into()) - } - name::FromStrError::ShortBuf => Self::ShortBuf, + name::ScanError::ShortBuf => Self::ShortBuf, + err => Self::Presentation(err.into()), } } } @@ -419,7 +417,7 @@ pub struct PresentationError(PresentationErrorEnum); #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum PresentationErrorEnum { - BadName(name::PresentationError), + BadName(name::ScanError), MissingQname, MissingClassAndQtype, MissingQtype, @@ -436,8 +434,8 @@ impl From for PresentationError { } } -impl From for PresentationError { - fn from(err: name::PresentationError) -> Self { +impl From for PresentationError { + fn from(err: name::ScanError) -> Self { Self(PresentationErrorEnum::BadName(err)) } } diff --git a/src/base/scan.rs b/src/base/scan.rs index 85756f898..858fa44fc 100644 --- a/src/base/scan.rs +++ b/src/base/scan.rs @@ -847,7 +847,7 @@ impl Scanner for IterScanner where Item: AsRef, Iter: Iterator, - Octets: FromBuilder, + Octets: FromBuilder + for<'a> TryFrom<&'a [u8]>, ::Builder: EmptyBuilder + Composer, { type Octets = Octets; @@ -959,11 +959,11 @@ where } fn scan_name(&mut self) -> Result { - let token = match self.iter.next() { - Some(token) => token, - None => return Err(StrError::end_of_entry()), - }; - Name::from_symbols(Symbols::new(token.as_ref().chars())) + self.iter + .next() + .ok_or_else(StrError::end_of_entry)? + .as_ref() + .parse() .map_err(|_| StrError::custom("invalid domain name")) } From 365b8ebb394bcd2bab529a6d862a13d8e91a0948 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Wed, 18 Sep 2024 14:04:03 +0200 Subject: [PATCH 6/7] Resolve warnings and clippy --- src/base/name/builder.rs | 4 ++-- src/base/scan.rs | 29 ++++++++++------------------- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/base/name/builder.rs b/src/base/name/builder.rs index f27af66af..9fc32372e 100644 --- a/src/base/name/builder.rs +++ b/src/base/name/builder.rs @@ -408,7 +408,7 @@ where Octs: TryFrom<&'a [u8]>, { assert!( - !self.cur_label().is_some_and(|l| !l.is_empty()), + self.cur_label().map_or(true, |l| l.is_empty()), "cannot extract a domain name while a label is being built" ); @@ -445,7 +445,7 @@ where Octs: TryFrom<&'a [u8]>, { assert!( - !self.cur_label().is_some_and(|l| !l.is_empty()), + self.cur_label().map_or(true, |l| l.is_empty()), "cannot extract a domain name while a label is being built" ); diff --git a/src/base/scan.rs b/src/base/scan.rs index 858fa44fc..25c7ac308 100644 --- a/src/base/scan.rs +++ b/src/base/scan.rs @@ -361,16 +361,6 @@ impl Symbol { pub fn from_chars>( chars: &mut C, ) -> Result, SymbolCharsError> { - #[inline] - fn bad_escape() -> SymbolCharsError { - SymbolCharsError(SymbolCharsEnum::BadEscape) - } - - #[inline] - fn short_input() -> SymbolCharsError { - SymbolCharsError(SymbolCharsEnum::ShortInput) - } - let ch = match chars.next() { Some(ch) => ch, None => return Ok(None), @@ -384,32 +374,33 @@ impl Symbol { let ch2 = match chars.next() { Some(ch) => match ch.to_digit(10) { Some(ch) => ch * 10, - None => return Err(bad_escape()), + None => return Err(SymbolCharsError::bad_escape()), }, - None => return Err(short_input()), + None => return Err(SymbolCharsError::short_input()), }; let ch3 = match chars.next() { Some(ch) => match ch.to_digit(10) { Some(ch) => ch, - None => return Err(bad_escape()), + None => return Err(SymbolCharsError::bad_escape()), }, - None => return Err(short_input()), + None => return Err(SymbolCharsError::short_input()), }; let res = ch + ch2 + ch3; if res > 255 { - return Err(bad_escape()); + return Err(SymbolCharsError::bad_escape()); } Ok(Some(Symbol::DecimalEscape(res as u8))) } Some(ch) => { - let ch = u8::try_from(ch).map_err(|_| bad_escape())?; + let ch = u8::try_from(ch) + .map_err(|_| SymbolCharsError::bad_escape())?; if ch < 0x20 || ch > 0x7e { - Err(bad_escape()) + Err(SymbolCharsError::bad_escape()) } else { Ok(Some(Symbol::SimpleEscape(ch))) } } - None => Err(short_input()), + None => Err(SymbolCharsError::short_input()), } } @@ -636,7 +627,7 @@ impl Symbol { if ch.is_ascii() && ch >= '\u{20}' && ch <= '\u{7E}' { Ok(ch as u8) } else { - Err(BadSymbol(BadSymbolEnum::NonAscii)) + Err(BadSymbol::non_ascii()) } } Symbol::SimpleEscape(ch) | Symbol::DecimalEscape(ch) => Ok(ch), From ef0be9bccfbc12eb5110612ae457a1a75af9f484 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Thu, 19 Sep 2024 12:03:22 +0200 Subject: [PATCH 7/7] Leave WIP until API discussion --- src/base/name/absolute.rs | 25 +++++++++-------------- src/base/name/relative.rs | 36 ++++++++++++++++------------------ src/base/name/uncertain.rs | 22 +++++++-------------- src/resolv/stub/conf.rs | 4 ++-- src/stelline/parse_stelline.rs | 1 + src/validator/context.rs | 15 ++------------ 6 files changed, 38 insertions(+), 65 deletions(-) diff --git a/src/base/name/absolute.rs b/src/base/name/absolute.rs index 06d543d42..e9c6df0e0 100644 --- a/src/base/name/absolute.rs +++ b/src/base/name/absolute.rs @@ -243,7 +243,8 @@ impl Name { /// Creates a domain name atop a Bytes from its string representation. pub fn bytes_from_str(s: &str) -> Result { - FromStr::from_str(s) + let name: Name> = s.parse()?; + Ok(Self(name.0.into())) } } @@ -941,11 +942,7 @@ where #[cfg(feature = "serde")] impl<'de, Octs> serde::Deserialize<'de> for Name where - Octs: FromBuilder + DeserializeOctets<'de>, - ::Builder: FreezeBuilder - + EmptyBuilder - + AsRef<[u8]> - + AsMut<[u8]>, + Octs: AsRef<[u8]> + for<'a> TryFrom<&'a [u8]> + DeserializeOctets<'de>, { fn deserialize>( deserializer: D, @@ -956,11 +953,9 @@ where impl<'de, Octs> serde::de::Visitor<'de> for InnerVisitor<'de, Octs> where - Octs: FromBuilder + DeserializeOctets<'de>, - ::Builder: FreezeBuilder - + EmptyBuilder - + AsRef<[u8]> - + AsMut<[u8]>, + Octs: AsRef<[u8]> + + for<'a> TryFrom<&'a [u8]> + + DeserializeOctets<'de>, { type Value = Name; @@ -999,11 +994,9 @@ where impl<'de, Octs> serde::de::Visitor<'de> for NewtypeVisitor where - Octs: FromBuilder + DeserializeOctets<'de>, - ::Builder: EmptyBuilder - + FreezeBuilder - + AsRef<[u8]> - + AsMut<[u8]>, + Octs: AsRef<[u8]> + + for<'a> TryFrom<&'a [u8]> + + DeserializeOctets<'de>, { type Value = Name; diff --git a/src/base/name/relative.rs b/src/base/name/relative.rs index e926daa10..0d757dcc9 100644 --- a/src/base/name/relative.rs +++ b/src/base/name/relative.rs @@ -199,7 +199,8 @@ impl RelativeName { /// Parses a string into a relative name atop a `Bytes`. pub fn bytes_from_str(s: &str) -> Result { - FromStr::from_str(s) + let name: RelativeName> = s.parse()?; + Ok(RelativeName(name.0.into())) } } @@ -766,11 +767,7 @@ where #[cfg(feature = "serde")] impl<'de, Octs> serde::Deserialize<'de> for RelativeName where - Octs: FromBuilder + DeserializeOctets<'de>, - ::Builder: FreezeBuilder - + EmptyBuilder - + AsRef<[u8]> - + AsMut<[u8]>, + Octs: AsRef<[u8]> + for<'a> TryFrom<&'a [u8]> + DeserializeOctets<'de>, { fn deserialize>( deserializer: D, @@ -781,11 +778,9 @@ where impl<'de, Octs> serde::de::Visitor<'de> for InnerVisitor<'de, Octs> where - Octs: FromBuilder + DeserializeOctets<'de>, - ::Builder: FreezeBuilder - + EmptyBuilder - + AsRef<[u8]> - + AsMut<[u8]>, + Octs: AsRef<[u8]> + + for<'a> TryFrom<&'a [u8]> + + DeserializeOctets<'de>, { type Value = RelativeName; @@ -797,9 +792,14 @@ where self, v: &str, ) -> Result { - let mut builder = NameBuilder::::new(); - builder.append_chars(v.chars()).map_err(E::custom)?; - Ok(builder.finish()) + let mut builder = NameBuilder::new([0u8; 256]); + builder.scan_name(v).map_err(E::custom)?; + builder + .as_relative() + .map_err(|_| E::custom(ScanError::ShortBuf)) + .and_then(|n| { + n.ok_or_else(|| E::custom("needed a relative name")) + }) } fn visit_borrowed_bytes( @@ -826,11 +826,9 @@ where impl<'de, Octs> serde::de::Visitor<'de> for NewtypeVisitor where - Octs: FromBuilder + DeserializeOctets<'de>, - ::Builder: FreezeBuilder - + EmptyBuilder - + AsRef<[u8]> - + AsMut<[u8]>, + Octs: AsRef<[u8]> + + for<'a> TryFrom<&'a [u8]> + + DeserializeOctets<'de>, { type Value = RelativeName; diff --git a/src/base/name/uncertain.rs b/src/base/name/uncertain.rs index 29ebd22ff..d1d7cf37d 100644 --- a/src/base/name/uncertain.rs +++ b/src/base/name/uncertain.rs @@ -418,11 +418,7 @@ where #[cfg(feature = "serde")] impl<'de, Octets> serde::Deserialize<'de> for UncertainName where - Octets: FromBuilder + DeserializeOctets<'de>, - ::Builder: EmptyBuilder - + FreezeBuilder - + AsRef<[u8]> - + AsMut<[u8]>, + Octets: AsRef<[u8]> + for<'a> TryFrom<&'a [u8]> + DeserializeOctets<'de>, { fn deserialize>( deserializer: D, @@ -433,11 +429,9 @@ where impl<'de, Octets> serde::de::Visitor<'de> for InnerVisitor<'de, Octets> where - Octets: FromBuilder + DeserializeOctets<'de>, - ::Builder: EmptyBuilder - + FreezeBuilder - + AsRef<[u8]> - + AsMut<[u8]>, + Octets: AsRef<[u8]> + + for<'a> TryFrom<&'a [u8]> + + DeserializeOctets<'de>, { type Value = UncertainName; @@ -478,11 +472,9 @@ where impl<'de, Octets> serde::de::Visitor<'de> for NewtypeVisitor where - Octets: FromBuilder + DeserializeOctets<'de>, - ::Builder: EmptyBuilder - + FreezeBuilder - + AsRef<[u8]> - + AsMut<[u8]>, + Octets: AsRef<[u8]> + + for<'a> TryFrom<&'a [u8]> + + DeserializeOctets<'de>, { type Value = UncertainName; diff --git a/src/resolv/stub/conf.rs b/src/resolv/stub/conf.rs index 8a06ce116..5d60ca5ff 100644 --- a/src/resolv/stub/conf.rs +++ b/src/resolv/stub/conf.rs @@ -707,8 +707,8 @@ impl convert::From for Error { } } -impl convert::From for Error { - fn from(_: name::FromStrError) -> Error { +impl convert::From for Error { + fn from(_: name::ScanError) -> Error { Error::ParseError } } diff --git a/src/stelline/parse_stelline.rs b/src/stelline/parse_stelline.rs index 09b005108..aaeb9533f 100644 --- a/src/stelline/parse_stelline.rs +++ b/src/stelline/parse_stelline.rs @@ -478,6 +478,7 @@ fn parse_section>>( match section { Section::Question => { + use core::str::FromStr; sections .question .push(Question::from_str(clean_line).unwrap()); diff --git a/src/validator/context.rs b/src/validator/context.rs index d9e44ba1a..cbe845ca4 100644 --- a/src/validator/context.rs +++ b/src/validator/context.rs @@ -2307,9 +2307,6 @@ pub enum Error { /// Error adding data while building a DNS message. PushError, - /// Error adding a label to a name. - PushNameError, - /// Error reading from a file. ReadError(Arc), @@ -2323,18 +2320,12 @@ impl From for Error { } } -impl From for Error { - fn from(_: name::PushError) -> Self { +impl From for Error { + fn from(_: name::BuildError) -> Self { Error::PushError } } -impl From for Error { - fn from(_: name::PushNameError) -> Self { - Error::PushNameError - } -} - impl From for Error { fn from(_: wire::ParseError) -> Self { Error::ParseError @@ -2355,7 +2346,6 @@ impl fmt::Display for Error { Error::OctetsConversion => write!(f, "OctetsConversion"), Error::ParseError => write!(f, "ParseError"), Error::PushError => write!(f, "PushError"), - Error::PushNameError => write!(f, "PushNameError"), Error::ReadError(_) => write!(f, "FormError"), Error::ShortMessage => write!(f, "ShortMEssage"), } @@ -2370,7 +2360,6 @@ impl error::Error for Error { Error::OctetsConversion => None, Error::ParseError => None, Error::PushError => None, - Error::PushNameError => None, Error::ReadError(err) => Some(err), Error::ShortMessage => None, }