From 905c438f18c4d07acc926eeab8d345b0053a0c17 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Mon, 23 Sep 2024 14:26:42 +0200 Subject: [PATCH 1/4] [base/message_builder] Introduce 'HashCompressor' I noticed that 'TreeCompressor' will likely have a lot of overhead, as it has to copy each label into its internal tree and it maintains a separate hashmap for every known name (where each name likely has one or two parents at the most, even though hashmaps have a large capacity). 'HashCompressor' is an alternative name compressor built on top of the 'hashbrown' crate, which offers lower-level hash table access that lets entries indirectly reference the built message for hashing and equality checks. It maintains a single hashmap, it should be faster, and it does not require as much memory. The implementation here makes it clear that domain names should have been stored in reverse order, at least in the wire format. A decent chunk of the logic goes into correctly reversing the domain name. In fact, the implementation likely quadratic runtime because of this (at least when 'Name' or 'RelativeName' are used, as backward iteration on them has quadratic runtime). While 'std' essentially exposes the same data structures as 'hashbrown', it does not currently offer a way to perform low-level hash table access (although there is a nightly "raw entry" feature). This may change in the future. If not for the added 'hashbrown' dependency, I would suggest deprecating 'TreeCompressor' entirely. --- Cargo.lock | 30 +++- Cargo.toml | 2 + src/base/message_builder.rs | 294 ++++++++++++++++++++++++++++++++++++ 3 files changed, 319 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9f9bb8ba4..58dfde030 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -26,6 +26,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "allocator-api2" +version = "0.2.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c6cb57a04249c6480766f7f7cef5467412af1490f8d1e243141daddada3264f" + [[package]] name = "android-tzdata" version = "0.1.1" @@ -219,6 +225,7 @@ dependencies = [ "bytes", "chrono", "futures-util", + "hashbrown 0.14.5", "heapless", "lazy_static", "libc", @@ -404,6 +411,15 @@ dependencies = [ "byteorder", ] +[[package]] +name = "hashbrown" +version = "0.14.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" +dependencies = [ + "allocator-api2", +] + [[package]] name = "hashbrown" version = "0.15.0" @@ -456,7 +472,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "707907fe3c25f5424cce2cb7e1cbcafee6bdbe735ca90ef77c29e84591e5b9da" dependencies = [ "equivalent", - "hashbrown", + "hashbrown 0.15.0", ] [[package]] @@ -918,9 +934,9 @@ dependencies = [ [[package]] name = "rustls-pki-types" -version = "1.9.0" +version = "1.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0e696e35370c65c9c541198af4543ccd580cf17fc25d8e05c5a242b202488c55" +checksum = "16f1201b3c9a7ee8039bcadc17b7e605e2945b27eee7631788c1bd2b0643674b" [[package]] name = "rustls-webpki" @@ -935,9 +951,9 @@ dependencies = [ [[package]] name = "rustversion" -version = "1.0.17" +version = "1.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "955d28af4278de8121b7ebeb796b6a45735dc01436d898801014aced2773a3d6" +checksum = "0e819f2bc632f285be6d7cd36e25940d45b2391dd6d9b939e79de557f7014248" [[package]] name = "ryu" @@ -1307,9 +1323,9 @@ checksum = "8ecb6da28b8a351d773b68d5825ac39017e680750f980f3a1a85cd8dd28a47c1" [[package]] name = "uuid" -version = "1.10.0" +version = "1.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "81dfa00651efa65069b0b6b651f4aaa31ba9e3c3ce0137aaad053604ee7e0314" +checksum = "f8c5f0a0af699448548ad1a2fbf920fb4bee257eae39953ba95cb84891a0446a" dependencies = [ "getrandom", ] diff --git a/Cargo.toml b/Cargo.toml index 499ce94e6..591d33826 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,6 +28,7 @@ arc-swap = { version = "1.7.0", optional = true } bytes = { version = "1.0", optional = true, default-features = false } chrono = { version = "0.4.35", optional = true, default-features = false } # 0.4.35 deprecates Duration::seconds() futures-util = { version = "0.3", optional = true } +hashbrown = { version = "0.14.2", optional = true, default-features = false, features = ["allocator-api2", "inline-more"] } # 0.14.2 introduces explicit hashing heapless = { version = "0.8", optional = true } libc = { version = "0.2.153", default-features = false, optional = true } # 0.2.79 is the first version that has IP_PMTUDISC_OMIT parking_lot = { version = "0.12", optional = true } @@ -47,6 +48,7 @@ tracing-subscriber = { version = "0.3.18", optional = true, features = ["env-fil [features] default = ["std", "rand"] bytes = ["dep:bytes", "octseq/bytes"] +hashbrown = ["dep:hashbrown", "std"] heapless = ["dep:heapless", "octseq/heapless"] resolv = ["net", "smallvec", "unstable-client-transport"] resolv-sync = ["resolv", "tokio/rt"] diff --git a/src/base/message_builder.rs b/src/base/message_builder.rs index 81d02c716..52040b4bd 100644 --- a/src/base/message_builder.rs +++ b/src/base/message_builder.rs @@ -141,14 +141,20 @@ use super::wire::{Compose, Composer}; use bytes::BytesMut; use core::ops::{Deref, DerefMut}; use core::{fmt, mem}; +#[cfg(feature = "hashbrown")] +use hashbrown::HashTable; #[cfg(feature = "std")] use octseq::array::Array; #[cfg(any(feature = "std", feature = "bytes"))] use octseq::builder::infallible; use octseq::builder::{FreezeBuilder, OctetsBuilder, ShortBuf, Truncate}; use octseq::octets::Octets; +#[cfg(feature = "hashbrown")] +use std::collections::hash_map::RandomState; #[cfg(feature = "std")] use std::collections::HashMap; +#[cfg(feature = "hashbrown")] +use std::hash::{BuildHasher, Hash, Hasher}; #[cfg(feature = "std")] use std::vec::Vec; @@ -2367,6 +2373,247 @@ impl Truncate for TreeCompressor { } } +//------------ HashCompressor ------------------------------------------------ + +/// A domain name compressor that uses a hash table. +/// +/// This type wraps around an octets builder and implements domain name +/// compression for it. It stores the position of any domain name it has seen +/// in a hash table. +/// +/// The position of a domain name is calculated relative to the beginning of +/// the underlying octets builder. This means that this builder must represent +/// the message only. This means that if you are using the [`StreamTarget`], +/// you need to place it inside this type, _not_ the other way around. +/// +/// [`StreamTarget`]: struct.StreamTarget.html +#[cfg(feature = "hashbrown")] +#[derive(Clone, Debug)] +pub struct HashCompressor { + /// The underlying octetsbuilder. + target: Target, + + /// The names inserted into the message. + /// + /// Each entry represents a name with at least one non-root label and its + /// position in the message. The name is divided into a head (the leftmost + /// label) and a tail (the remaining labels). The entry is stored as the + /// position of the head label in the message, and the position of the tail + /// label in the m + /// + /// Each entry is a tuple of head and tail positions. The head position is + /// the position + names: HashTable, + + /// How names in the table are hashed. + hasher: RandomState, +} + +#[cfg(feature = "hashbrown")] +#[derive(Copy, Clone, Debug)] +struct HashEntry { + /// The position of the head label in the name. + head: u16, + + /// The position of the tail name. + tail: u16, +} + +#[cfg(feature = "hashbrown")] +impl HashEntry { + /// Try constructing a [`HashEntry`]. + fn new(head: usize, tail: usize) -> Option { + if head < 0xC000 { + Some(Self { + head: head as u16, + tail: tail as u16, + }) + } else { + None + } + } + + /// Get the head label for this entry. + fn head<'m>(&self, message: &'m [u8]) -> &'m Label { + Label::split_from(&message[self.head as usize..]) + .expect("the message contains valid labels") + .0 + } + + /// Compute the hash of this entry. + fn hash(&self, message: &[u8], hasher: &RandomState) -> u64 { + let mut state = hasher.build_hasher(); + (self.head(message), self.tail).hash(&mut state); + state.finish() + } + + /// Compare this entry to a label. + fn eq(&self, message: &[u8], query: (&Label, u16)) -> bool { + (self.head(message), self.tail) == query + } +} + +#[cfg(feature = "hashbrown")] +impl HashCompressor { + /// Creates a new compressor from an underlying octets builder. + pub fn new(target: Target) -> Self { + HashCompressor { + target, + names: Default::default(), + hasher: Default::default(), + } + } + + /// Returns a reference to the underlying octets builder. + pub fn as_target(&self) -> &Target { + &self.target + } + + /// Converts the compressor into the underlying octets builder. + pub fn into_target(self) -> Target { + self.target + } + + /// Returns an octets slice of the data. + pub fn as_slice(&self) -> &[u8] + where + Target: AsRef<[u8]>, + { + self.target.as_ref() + } + + /// Returns an mutable octets slice of the data. + pub fn as_slice_mut(&mut self) -> &mut [u8] + where + Target: AsMut<[u8]>, + { + self.target.as_mut() + } +} + +//--- AsRef, AsMut, and OctetsBuilder + +#[cfg(feature = "hashbrown")] +impl> AsRef<[u8]> for HashCompressor { + fn as_ref(&self) -> &[u8] { + self.as_slice() + } +} + +#[cfg(feature = "hashbrown")] +impl> AsMut<[u8]> for HashCompressor { + fn as_mut(&mut self) -> &mut [u8] { + self.as_slice_mut() + } +} + +#[cfg(feature = "hashbrown")] +impl OctetsBuilder for HashCompressor { + type AppendError = Target::AppendError; + + fn append_slice( + &mut self, + slice: &[u8], + ) -> Result<(), Self::AppendError> { + self.target.append_slice(slice) + } +} + +#[cfg(feature = "hashbrown")] +impl Composer for HashCompressor { + fn append_compressed_name( + &mut self, + name: &N, + ) -> Result<(), Self::AppendError> { + let mut name = name.iter_labels(); + let message = self.target.as_ref(); + + // Remove the root label -- we know it's there. + assert!( + name.next_back().map_or(false, |l| l.is_root()), + "absolute names must end with a root label" + ); + + // The position of the consumed labels from the end of the name. + let mut position = 0xFFFF; + + // The last label that must be inserted, if any. + let mut last_label = None; + + // Look up each label in reverse order. + while let Some(label) = name.next_back() { + // Look up the labels seen thus far in the hash table. + let query = (label, position); + let hash = { + let mut state = self.hasher.build_hasher(); + query.hash(&mut state); + state.finish() + }; + + let entry = + self.names.find(hash, |&name| name.eq(message, query)); + if let Some(entry) = entry { + // We found a match, so update the position. + position = entry.head; + } else { + // We will have to write this label. + last_label = Some(label); + break; + } + } + + // Write out the remaining labels in the name in regular order. + let mut labels = name.chain(last_label).peekable(); + while let Some(label) = labels.next() { + let head = self.target.as_ref().len(); + let tail = head + label.compose_len() as usize; + + label.compose(self)?; + + // Remember this label for future compression, if possible. + // + // If some labels in this name pass the 0xC000 boundary point, then + // none of its remembered labels can be used (since they are looked + // up from right to left, and the rightmost ones will fail first). + // We could check more thoroughly for this, but it's not worth it. + if let Some(mut entry) = HashEntry::new(head, tail) { + // If there is no following label, use the remaining position. + if labels.peek().is_none() { + entry.tail = position; + } + + let message = self.target.as_ref(); + let hasher = &self.hasher; + let hash = entry.hash(message, hasher); + self.names.insert_unique(hash, entry, |&name| { + name.hash(message, hasher) + }); + } + } + + // Write the compressed pointer or the root label. + if position != 0xFFFF { + (position | 0xC000).compose(self) + } else { + Label::root().compose(self) + } + } + + fn can_compress(&self) -> bool { + true + } +} + +#[cfg(feature = "hashbrown")] +impl Truncate for HashCompressor { + fn truncate(&mut self, len: usize) { + self.target.truncate(len); + if len < 0xC000 { + self.names.retain(|name| name.head < len as u16); + } + } +} + //============ Errors ======================================================== #[derive(Clone, Copy, Debug)] @@ -2634,4 +2881,51 @@ mod test { assert_eq!(45, actual.len(), "unexpected response size"); assert_eq!(expect[..], actual, "unexpected response data"); } + + #[cfg(feature = "hashbrown")] + #[test] + fn hash_compress_positive_response() { + // An example positive response to `A example.com.` that is compressed + // + // ;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 0 + // ;; flags: qr rd ra ad; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 0 + // + // ;; QUESTION SECTION: + // ;example.com. IN A + // + // ;; ANSWER SECTION: + // example.com. 3600 IN A 203.0.113.1 + // + // ;; MSG SIZE rcvd: 45 + let expect = &[ + 0x00, 0x00, 0x81, 0xa0, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, + 0x00, 0x07, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x03, 0x63, + 0x6f, 0x6d, 0x00, 0x00, 0x01, 0x00, 0x01, 0xc0, 0x0c, 0x00, 0x01, + 0x00, 0x01, 0x00, 0x00, 0x0e, 0x10, 0x00, 0x04, 0xcb, 0x00, 0x71, + 0x01, + ]; + + let name = "example.com.".parse::>>().unwrap(); + let mut msg = + MessageBuilder::from_target(HashCompressor::new(Vec::new())) + .unwrap() + .question(); + msg.header_mut().set_rcode(Rcode::NOERROR); + msg.header_mut().set_rd(true); + msg.header_mut().set_ra(true); + msg.header_mut().set_qr(true); + msg.header_mut().set_ad(true); + + // Question + msg.push((name.clone(), Rtype::A)).unwrap(); + + // Answer + let mut msg = msg.answer(); + msg.push((name.clone(), 3600, A::from_octets(203, 0, 113, 1))) + .unwrap(); + + let actual = msg.finish().into_target(); + assert_eq!(45, actual.len(), "unexpected response size"); + assert_eq!(expect[..], actual, "unexpected response data"); + } } From b1028b4c6f634a3d05cb03f3b79553d784a61a56 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Mon, 23 Sep 2024 14:45:31 +0200 Subject: [PATCH 2/4] [base/message_builder] Mention 'HashCompressor' in API docs --- src/base/message_builder.rs | 26 +++++++++++++------------- src/base/mod.rs | 2 ++ 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/base/message_builder.rs b/src/base/message_builder.rs index 52040b4bd..233dd7146 100644 --- a/src/base/message_builder.rs +++ b/src/base/message_builder.rs @@ -43,22 +43,22 @@ //! and makes sure the message doesn’t become longer than what the counter //! can provide for. //! -//! Two further types, [`TreeCompressor`] and [`StaticCompressor`], provide -//! name compression. This is a mechanism to decrease the size of a DNS -//! message by avoiding repeating domain names: Instead of including a domain -//! name or suffix of a domain name that has been mentioned already, a pointer -//! to the position of the original mention is provided. Since this process is -//! somewhat expensive as you have to remember which names have already been -//! used, it isn’t enabled by default and provided via separate octets -//! builders instead which we call compressors. +//! There is also support for name compression. This is a mechanism to decrease +//! the size of a DNS message by avoiding repeating domain names: Instead of +//! including a domain name or suffix of a domain name that has been mentioned +//! already, a pointer to the position of the original mention is provided. +//! Since this process is somewhat expensive as you have to remember which names +//! have already been used, it isn’t enabled by default and is instead provided +//! by separate octets builders which we call compressors. //! -//! Currently, there are two different compressors. [`TreeCompressor`] stores +//! Currently, there are three different compressors. [`TreeCompressor`] stores //! all names it encountered in a binary tree. While it can handle any number //! of names, it does require an allocator and therefore cannot be used in a -//! `no_std` environment. [`StaticCompressor`], meanwhile, has a static table -//! for up to 24 names. It is thus becoming ineffective on large messages -//! with lots of different names. However, 24 should be good enough for most -//! normal messages. +//! `no_std` environment. [`HashCompressor`] also requires allocation, but uses +//! a fast and space efficient hash table (via the `hashbrown` crate) instead. +//! [`StaticCompressor`], meanwhile, has a static table for up to 24 names. It +//! is ineffective on large messages with lots of different names, but this is +//! quite rare anyway. //! //! # Example //! diff --git a/src/base/mod.rs b/src/base/mod.rs index da4edc428..8d3b4bd19 100644 --- a/src/base/mod.rs +++ b/src/base/mod.rs @@ -92,6 +92,8 @@ pub use self::cmp::CanonicalOrd; pub use self::header::{Header, HeaderCounts, HeaderSection}; pub use self::iana::Rtype; pub use self::message::{Message, QuestionSection, RecordSection}; +#[cfg(feature = "hashbrown")] +pub use self::message_builder::HashCompressor; #[cfg(feature = "std")] pub use self::message_builder::TreeCompressor; pub use self::message_builder::{ From c8034cf9d3c2ff95b9257a5519398e048f0136f8 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Thu, 3 Oct 2024 10:20:37 +0200 Subject: [PATCH 3/4] Add internal docs for `HashCompressor` The feature flag to enable it has been changed to be more specific. --- Cargo.toml | 2 +- src/base/message_builder.rs | 71 ++++++++++++++++++++++++++----------- src/base/mod.rs | 2 +- 3 files changed, 53 insertions(+), 22 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 591d33826..b9e8f880e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,7 +48,7 @@ tracing-subscriber = { version = "0.3.18", optional = true, features = ["env-fil [features] default = ["std", "rand"] bytes = ["dep:bytes", "octseq/bytes"] -hashbrown = ["dep:hashbrown", "std"] +hash-name-compressor = ["dep:hashbrown", "std"] heapless = ["dep:heapless", "octseq/heapless"] resolv = ["net", "smallvec", "unstable-client-transport"] resolv-sync = ["resolv", "tokio/rt"] diff --git a/src/base/message_builder.rs b/src/base/message_builder.rs index 233dd7146..e2ec0601a 100644 --- a/src/base/message_builder.rs +++ b/src/base/message_builder.rs @@ -141,7 +141,7 @@ use super::wire::{Compose, Composer}; use bytes::BytesMut; use core::ops::{Deref, DerefMut}; use core::{fmt, mem}; -#[cfg(feature = "hashbrown")] +#[cfg(feature = "hash-name-compressor")] use hashbrown::HashTable; #[cfg(feature = "std")] use octseq::array::Array; @@ -149,11 +149,11 @@ use octseq::array::Array; use octseq::builder::infallible; use octseq::builder::{FreezeBuilder, OctetsBuilder, ShortBuf, Truncate}; use octseq::octets::Octets; -#[cfg(feature = "hashbrown")] +#[cfg(feature = "hash-name-compressor")] use std::collections::hash_map::RandomState; #[cfg(feature = "std")] use std::collections::HashMap; -#[cfg(feature = "hashbrown")] +#[cfg(feature = "hash-name-compressor")] use std::hash::{BuildHasher, Hash, Hasher}; #[cfg(feature = "std")] use std::vec::Vec; @@ -2387,7 +2387,7 @@ impl Truncate for TreeCompressor { /// you need to place it inside this type, _not_ the other way around. /// /// [`StreamTarget`]: struct.StreamTarget.html -#[cfg(feature = "hashbrown")] +#[cfg(feature = "hash-name-compressor")] #[derive(Clone, Debug)] pub struct HashCompressor { /// The underlying octetsbuilder. @@ -2395,21 +2395,52 @@ pub struct HashCompressor { /// The names inserted into the message. /// - /// Each entry represents a name with at least one non-root label and its - /// position in the message. The name is divided into a head (the leftmost - /// label) and a tail (the remaining labels). The entry is stored as the - /// position of the head label in the message, and the position of the tail - /// label in the m + /// Consider a set of names, where the "parent" (i.e. tail) of each name is + /// another name in the set. For example, a set might contain `.`, `org.`, + /// `example.org.`, and `www.example.org.`. Each of these names (except the + /// root, which is handled specially) is stored as a separate entry in this + /// hash table. A name `.` is stored as the index of `` + /// and the index of `` in the built message. Its hash is built from + /// `` (the bytes in the label, not the index into the message) and + /// the index of ``. + /// + /// Lookups are performed by iterating backward through the labels in a name + /// (to go from the root outward). At each step, the canonical position of + /// `` is already known; it is hashed together with the next label and + /// searched for in the hash table. An entry with a matching `` (i.e. + /// where the referenced content in the message matches the searched label) + /// and a matching `` position represents the same name. + /// + /// The root is handled specially. It is never inserted in the hash table, + /// and instead has a fixed fake position in the message of 0xFFFF. That is + /// the initial value that lookups start with. + /// + /// As an example, consider a message `org. example.org. www.example.org.`. + /// In the hash table, the first two entries will look like: + /// + /// ```text + /// - head=0 ("org") tail=0xFFFF + /// - head=5 ("example") tail=0 + /// ``` /// - /// Each entry is a tuple of head and tail positions. The head position is - /// the position + /// Now, to insert `www.example.org.`, the lookup begins from the end. We + /// search for a label "org" with tail 0xFFFF, and find head=0. We set this + /// as the next tail, and search for label "example" to find head=5. Since + /// a label "www" with tail=5 is not already in the hash table, it will be + /// inserted with head=18. + /// + /// This has a space overhead of 4-5 bytes for each entry, accounting for + /// the load factor (which appears to be 87.5%). Technically, storing the + /// labels indirectly means that comparisons are more expensive; but most + /// labels are relatively short and storing them inline as 64-byte arrays + /// would be quite wasteful. names: HashTable, /// How names in the table are hashed. hasher: RandomState, } -#[cfg(feature = "hashbrown")] +#[cfg(feature = "hash-name-compressor")] #[derive(Copy, Clone, Debug)] struct HashEntry { /// The position of the head label in the name. @@ -2419,7 +2450,7 @@ struct HashEntry { tail: u16, } -#[cfg(feature = "hashbrown")] +#[cfg(feature = "hash-name-compressor")] impl HashEntry { /// Try constructing a [`HashEntry`]. fn new(head: usize, tail: usize) -> Option { @@ -2453,7 +2484,7 @@ impl HashEntry { } } -#[cfg(feature = "hashbrown")] +#[cfg(feature = "hash-name-compressor")] impl HashCompressor { /// Creates a new compressor from an underlying octets builder. pub fn new(target: Target) -> Self { @@ -2493,21 +2524,21 @@ impl HashCompressor { //--- AsRef, AsMut, and OctetsBuilder -#[cfg(feature = "hashbrown")] +#[cfg(feature = "hash-name-compressor")] impl> AsRef<[u8]> for HashCompressor { fn as_ref(&self) -> &[u8] { self.as_slice() } } -#[cfg(feature = "hashbrown")] +#[cfg(feature = "hash-name-compressor")] impl> AsMut<[u8]> for HashCompressor { fn as_mut(&mut self) -> &mut [u8] { self.as_slice_mut() } } -#[cfg(feature = "hashbrown")] +#[cfg(feature = "hash-name-compressor")] impl OctetsBuilder for HashCompressor { type AppendError = Target::AppendError; @@ -2519,7 +2550,7 @@ impl OctetsBuilder for HashCompressor { } } -#[cfg(feature = "hashbrown")] +#[cfg(feature = "hash-name-compressor")] impl Composer for HashCompressor { fn append_compressed_name( &mut self, @@ -2604,7 +2635,7 @@ impl Composer for HashCompressor { } } -#[cfg(feature = "hashbrown")] +#[cfg(feature = "hash-name-compressor")] impl Truncate for HashCompressor { fn truncate(&mut self, len: usize) { self.target.truncate(len); @@ -2882,7 +2913,7 @@ mod test { assert_eq!(expect[..], actual, "unexpected response data"); } - #[cfg(feature = "hashbrown")] + #[cfg(feature = "hash-name-compressor")] #[test] fn hash_compress_positive_response() { // An example positive response to `A example.com.` that is compressed diff --git a/src/base/mod.rs b/src/base/mod.rs index 8d3b4bd19..d3fa58820 100644 --- a/src/base/mod.rs +++ b/src/base/mod.rs @@ -92,7 +92,7 @@ pub use self::cmp::CanonicalOrd; pub use self::header::{Header, HeaderCounts, HeaderSection}; pub use self::iana::Rtype; pub use self::message::{Message, QuestionSection, RecordSection}; -#[cfg(feature = "hashbrown")] +#[cfg(feature = "hash-name-compressor")] pub use self::message_builder::HashCompressor; #[cfg(feature = "std")] pub use self::message_builder::TreeCompressor; From 072474a2a650600521ed474b01e47344f2b37d97 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Thu, 3 Oct 2024 14:00:47 +0200 Subject: [PATCH 4/4] Move 'HashCompressor' under the 'std' feature --- Cargo.toml | 3 +-- src/base/message_builder.rs | 28 +++++++++++++--------------- src/base/mod.rs | 4 +--- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b9e8f880e..a9b938811 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,14 +48,13 @@ tracing-subscriber = { version = "0.3.18", optional = true, features = ["env-fil [features] default = ["std", "rand"] bytes = ["dep:bytes", "octseq/bytes"] -hash-name-compressor = ["dep:hashbrown", "std"] heapless = ["dep:heapless", "octseq/heapless"] resolv = ["net", "smallvec", "unstable-client-transport"] resolv-sync = ["resolv", "tokio/rt"] serde = ["dep:serde", "octseq/serde"] sign = ["std"] smallvec = ["dep:smallvec", "octseq/smallvec"] -std = ["bytes?/std", "octseq/std", "time/std"] +std = ["dep:hashbrown", "bytes?/std", "octseq/std", "time/std"] net = ["bytes", "futures-util", "rand", "std", "tokio"] tsig = ["bytes", "ring", "smallvec"] validate = ["bytes", "std", "ring"] diff --git a/src/base/message_builder.rs b/src/base/message_builder.rs index e2ec0601a..723db9867 100644 --- a/src/base/message_builder.rs +++ b/src/base/message_builder.rs @@ -141,7 +141,7 @@ use super::wire::{Compose, Composer}; use bytes::BytesMut; use core::ops::{Deref, DerefMut}; use core::{fmt, mem}; -#[cfg(feature = "hash-name-compressor")] +#[cfg(feature = "std")] use hashbrown::HashTable; #[cfg(feature = "std")] use octseq::array::Array; @@ -149,11 +149,9 @@ use octseq::array::Array; use octseq::builder::infallible; use octseq::builder::{FreezeBuilder, OctetsBuilder, ShortBuf, Truncate}; use octseq::octets::Octets; -#[cfg(feature = "hash-name-compressor")] -use std::collections::hash_map::RandomState; #[cfg(feature = "std")] -use std::collections::HashMap; -#[cfg(feature = "hash-name-compressor")] +use std::collections::{hash_map::RandomState, HashMap}; +#[cfg(feature = "std")] use std::hash::{BuildHasher, Hash, Hasher}; #[cfg(feature = "std")] use std::vec::Vec; @@ -2387,7 +2385,7 @@ impl Truncate for TreeCompressor { /// you need to place it inside this type, _not_ the other way around. /// /// [`StreamTarget`]: struct.StreamTarget.html -#[cfg(feature = "hash-name-compressor")] +#[cfg(feature = "std")] #[derive(Clone, Debug)] pub struct HashCompressor { /// The underlying octetsbuilder. @@ -2440,7 +2438,7 @@ pub struct HashCompressor { hasher: RandomState, } -#[cfg(feature = "hash-name-compressor")] +#[cfg(feature = "std")] #[derive(Copy, Clone, Debug)] struct HashEntry { /// The position of the head label in the name. @@ -2450,7 +2448,7 @@ struct HashEntry { tail: u16, } -#[cfg(feature = "hash-name-compressor")] +#[cfg(feature = "std")] impl HashEntry { /// Try constructing a [`HashEntry`]. fn new(head: usize, tail: usize) -> Option { @@ -2484,7 +2482,7 @@ impl HashEntry { } } -#[cfg(feature = "hash-name-compressor")] +#[cfg(feature = "std")] impl HashCompressor { /// Creates a new compressor from an underlying octets builder. pub fn new(target: Target) -> Self { @@ -2524,21 +2522,21 @@ impl HashCompressor { //--- AsRef, AsMut, and OctetsBuilder -#[cfg(feature = "hash-name-compressor")] +#[cfg(feature = "std")] impl> AsRef<[u8]> for HashCompressor { fn as_ref(&self) -> &[u8] { self.as_slice() } } -#[cfg(feature = "hash-name-compressor")] +#[cfg(feature = "std")] impl> AsMut<[u8]> for HashCompressor { fn as_mut(&mut self) -> &mut [u8] { self.as_slice_mut() } } -#[cfg(feature = "hash-name-compressor")] +#[cfg(feature = "std")] impl OctetsBuilder for HashCompressor { type AppendError = Target::AppendError; @@ -2550,7 +2548,7 @@ impl OctetsBuilder for HashCompressor { } } -#[cfg(feature = "hash-name-compressor")] +#[cfg(feature = "std")] impl Composer for HashCompressor { fn append_compressed_name( &mut self, @@ -2635,7 +2633,7 @@ impl Composer for HashCompressor { } } -#[cfg(feature = "hash-name-compressor")] +#[cfg(feature = "std")] impl Truncate for HashCompressor { fn truncate(&mut self, len: usize) { self.target.truncate(len); @@ -2913,7 +2911,7 @@ mod test { assert_eq!(expect[..], actual, "unexpected response data"); } - #[cfg(feature = "hash-name-compressor")] + #[cfg(feature = "std")] #[test] fn hash_compress_positive_response() { // An example positive response to `A example.com.` that is compressed diff --git a/src/base/mod.rs b/src/base/mod.rs index d3fa58820..4ec565c81 100644 --- a/src/base/mod.rs +++ b/src/base/mod.rs @@ -92,10 +92,8 @@ pub use self::cmp::CanonicalOrd; pub use self::header::{Header, HeaderCounts, HeaderSection}; pub use self::iana::Rtype; pub use self::message::{Message, QuestionSection, RecordSection}; -#[cfg(feature = "hash-name-compressor")] -pub use self::message_builder::HashCompressor; #[cfg(feature = "std")] -pub use self::message_builder::TreeCompressor; +pub use self::message_builder::{HashCompressor, TreeCompressor}; pub use self::message_builder::{ MessageBuilder, RecordSectionBuilder, StaticCompressor, StreamTarget, };