From 015bc0d734ef3737e2f7d8ba2c50aabe82128eb4 Mon Sep 17 00:00:00 2001
From: Martin Hoffmann <martin@nlnetlabs.nl>
Date: Mon, 5 Feb 2024 11:35:08 +0100
Subject: [PATCH] Add OctetsFrom and Debug impls for AllOptData. (#257)

This PR adds implementation for OctetsFrom and Debug to AllOptData. As a
consequence, this also adds these to some of the option data types and
changes Debug for some of them to be available for all octets sequences.
---
 Cargo.toml                |  9 ++++---
 src/base/opt/algsig.rs    | 28 +++++++++++++++++++++-
 src/base/opt/chain.rs     | 12 ++++++++--
 src/base/opt/cookie.rs    |  7 ++++++
 src/base/opt/expire.rs    |  7 ++++++
 src/base/opt/exterr.rs    | 21 +++++++++++++++-
 src/base/opt/keepalive.rs |  9 ++++++-
 src/base/opt/keytag.rs    | 15 +++++++++++-
 src/base/opt/macros.rs    | 50 ++++++++++++++++++++++++++++++++++++++-
 src/base/opt/mod.rs       | 30 +++++++++++++++++++++--
 src/base/opt/nsid.rs      | 15 +++++++++++-
 src/base/opt/padding.rs   | 15 +++++++++++-
 src/base/opt/subnet.rs    |  7 ++++++
 13 files changed, 209 insertions(+), 16 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 36acd187..01da60d6 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -17,10 +17,9 @@ name = "domain"
 path = "src/lib.rs"
 
 [dependencies]
-
-octseq         =  { version = "0.4.0", default-features = false }
+octseq           =  { version = "0.5", default-features = false }
 pin-project-lite = "0.2"
-time           =  { version = "0.3.1", default-features = false }
+time             =  { version = "0.3.1", default-features = false }
 
 rand           = { version = "0.8", optional = true }
 bytes          = { version = "1.0", optional = true, default-features = false }
@@ -31,7 +30,7 @@ heapless       = { version = "0.8", optional = true }
 ring           = { version = "0.17", optional = true }
 serde          = { version = "1.0.130", optional = true, features = ["derive"] }
 siphasher      = { version = "1", optional = true }
-smallvec       = { version = "1", optional = true }
+smallvec       = { version = "1.3", optional = true }
 tokio          = { version = "1.33", optional = true, features = ["io-util", "macros", "net", "time", "sync", "rt-multi-thread" ] }
 tokio-rustls   = { version = "0.24", optional = true, features = [] }
 
@@ -94,4 +93,4 @@ required-features = ["std", "rand"]
 
 [[example]]
 name = "client-transports"
-required-features = ["net"]
\ No newline at end of file
+required-features = ["net"]
diff --git a/src/base/opt/algsig.rs b/src/base/opt/algsig.rs
index 7ba316a9..9caffde8 100644
--- a/src/base/opt/algsig.rs
+++ b/src/base/opt/algsig.rs
@@ -46,7 +46,7 @@ use core::marker::PhantomData;
 /// Once you have a value, you can iterate over the algorithms via the
 /// [`iter`][Understood::iter] method or use the `IntoIterator` implementation
 /// for a reference.
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy)]
 pub struct Understood<Variant, Octs: ?Sized> {
     /// A marker for the variant.
     marker: PhantomData<Variant>,
@@ -392,6 +392,32 @@ where
     }
 }
 
+//--- Debug
+
+impl<Octs: AsRef<[u8]> + ?Sized> fmt::Debug for Understood<DauVariant, Octs> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_tuple("Understood<DauVariant>")
+            .field(&format_args!("{}", self))
+            .finish()
+    }
+}
+
+impl<Octs: AsRef<[u8]> + ?Sized> fmt::Debug for Understood<DhuVariant, Octs> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_tuple("Understood<DhuVariant>")
+            .field(&format_args!("{}", self))
+            .finish()
+    }
+}
+
+impl<Octs: AsRef<[u8]> + ?Sized> fmt::Debug for Understood<N3uVariant, Octs> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_tuple("Understood<N3uVariant>")
+            .field(&format_args!("{}", self))
+            .finish()
+    }
+}
+
 //--- Extended Opt and OptBuilder
 
 impl<Octs: Octets> Opt<Octs> {
diff --git a/src/base/opt/chain.rs b/src/base/opt/chain.rs
index 4ab9f452..011977f1 100644
--- a/src/base/opt/chain.rs
+++ b/src/base/opt/chain.rs
@@ -29,7 +29,7 @@ use core::cmp::Ordering;
 /// point of the included records, i.e., the suffix of the queried name
 /// furthest away from the root to which the requesting resolver already has
 /// all necessary records.
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy)]
 pub struct Chain<Name: ?Sized> {
     /// The start name AKA ‘closest trust point.’
     start: Name
@@ -163,7 +163,7 @@ impl<Name: ToDname> ComposeOptData for Chain<Name> {
     }
 }
 
-//--- Display
+//--- Display and Debug
 
 impl<Name: fmt::Display> fmt::Display for Chain<Name> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
@@ -171,6 +171,14 @@ impl<Name: fmt::Display> fmt::Display for Chain<Name> {
     }
 }
 
+impl<Name: fmt::Display> fmt::Debug for Chain<Name> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_struct("Chain")
+            .field("start", &format_args!("{}", self.start))
+            .finish()
+    }
+}
+
 //--- Extended Opt and OptBuilder
 
 impl<Octs: Octets> Opt<Octs> {
diff --git a/src/base/opt/cookie.rs b/src/base/opt/cookie.rs
index 320b333c..d6d63d26 100644
--- a/src/base/opt/cookie.rs
+++ b/src/base/opt/cookie.rs
@@ -156,6 +156,13 @@ impl Cookie {
             )
         )
     }
+
+    /// Placeholder for unnecessary octets conversion.
+    ///
+    /// This method only exists for the `AllOptData` macro.
+    pub(super) fn try_octets_from<E>(src: Self) -> Result<Self, E> {
+        Ok(src)
+    }
 }
 
 
diff --git a/src/base/opt/expire.rs b/src/base/opt/expire.rs
index ec38739a..8055b3b1 100644
--- a/src/base/opt/expire.rs
+++ b/src/base/opt/expire.rs
@@ -55,6 +55,13 @@ impl Expire {
             u32::parse(parser).map(|res| Expire::new(Some(res)))
         }
     }
+
+    /// Placeholder for unnecessary octets conversion.
+    ///
+    /// This method only exists for the `AllOptData` macro.
+    pub(super) fn try_octets_from<E>(src: Self) -> Result<Self, E> {
+        Ok(src)
+    }
 }
 
 //--- OptData
diff --git a/src/base/opt/exterr.rs b/src/base/opt/exterr.rs
index 77ef10b1..a6e46b38 100644
--- a/src/base/opt/exterr.rs
+++ b/src/base/opt/exterr.rs
@@ -14,7 +14,7 @@ use super::{
     BuildDataError, LongOptData, Opt, OptData, ComposeOptData, ParseOptData
 };
 use octseq::builder::OctetsBuilder;
-use octseq::octets::Octets;
+use octseq::octets::{Octets, OctetsFrom};
 use octseq::parse::Parser;
 use octseq::str::Str;
 use core::{fmt, hash, str};
@@ -146,6 +146,25 @@ where Octs: AsRef<[u8]> {
     }
 }
 
+//--- OctetsFrom
+
+impl<Octs, SrcOcts> OctetsFrom<ExtendedError<SrcOcts>> for ExtendedError<Octs>
+where
+    Octs: OctetsFrom<SrcOcts>
+{
+    type Error = Octs::Error;
+
+    fn try_octets_from(
+        source: ExtendedError<SrcOcts>
+    ) -> Result<Self, Self::Error> {
+        let text = match source.text {
+            Some(Ok(text)) => Some(Ok(Str::try_octets_from(text)?)),
+            Some(Err(octs)) => Some(Err(Octs::try_octets_from(octs)?)),
+            None => None,
+        };
+        Ok(Self { code: source.code, text })
+    }
+}
 //--- OptData, ParseOptData, and ComposeOptData
 
 impl<Octs> OptData for ExtendedError<Octs> {
diff --git a/src/base/opt/keepalive.rs b/src/base/opt/keepalive.rs
index bf7cc7c3..c827f7a7 100644
--- a/src/base/opt/keepalive.rs
+++ b/src/base/opt/keepalive.rs
@@ -55,6 +55,13 @@ impl TcpKeepalive {
             IdleTimeout::parse(parser).map(|v| Self::new(Some(v)))
         }
     }
+
+    /// Placeholder for unnecessary octets conversion.
+    ///
+    /// This method only exists for the `AllOptData` macro.
+    pub(super) fn try_octets_from<E>(src: Self) -> Result<Self, E> {
+        Ok(src)
+    }
 }
 
 //--- OptData
@@ -144,7 +151,7 @@ impl IdleTimeout {
     const COMPOSE_LEN: u16 = 2;
 
     /// Parses a value from its wire format.
-    fn parse<Octs: AsRef<[u8]>>(
+    fn parse<Octs: AsRef<[u8]> + ?Sized>(
         parser: &mut Parser<Octs>
     ) -> Result<Self, ParseError> {
         u16::parse(parser).map(Self)
diff --git a/src/base/opt/keytag.rs b/src/base/opt/keytag.rs
index 8bb35d87..ba769342 100644
--- a/src/base/opt/keytag.rs
+++ b/src/base/opt/keytag.rs
@@ -13,7 +13,7 @@ use super::super::message_builder::OptBuilder;
 use super::super::wire::{Composer, ParseError};
 use super::{Opt, OptData, ComposeOptData, ParseOptData};
 use octseq::builder::OctetsBuilder;
-use octseq::octets::Octets;
+use octseq::octets::{Octets, OctetsFrom};
 use octseq::parse::Parser;
 use core::{borrow, fmt, hash};
 use core::cmp::Ordering;
@@ -149,6 +149,19 @@ impl<Octs: ?Sized> KeyTag<Octs> {
     }
 }
 
+//--- OctetsFrom
+
+impl<Octs, SrcOcts> OctetsFrom<KeyTag<SrcOcts>> for KeyTag<Octs>
+where Octs: OctetsFrom<SrcOcts> {
+    type Error = Octs::Error;
+
+    fn try_octets_from(src: KeyTag<SrcOcts>) -> Result<Self, Self::Error> {
+        Octs::try_octets_from(src.octets).map(|octets| unsafe {
+            Self::from_octets_unchecked(octets)
+        })
+    }
+}
+
 //--- AsRef, AsMut, Borrow, BorrowMut
 
 impl<Octs: AsRef<[u8]> + ?Sized> AsRef<[u8]> for KeyTag<Octs> {
diff --git a/src/base/opt/macros.rs b/src/base/opt/macros.rs
index 373f9707..7bc3a40d 100644
--- a/src/base/opt/macros.rs
+++ b/src/base/opt/macros.rs
@@ -16,7 +16,6 @@ macro_rules! opt_types {
 
         //------------ AllOptData --------------------------------------------
 
-        // TODO Impl Debug.
         #[derive(Clone)]
         #[non_exhaustive]
         pub enum AllOptData<Octs, Name> {
@@ -26,6 +25,37 @@ macro_rules! opt_types {
             Other(UnknownOptData<Octs>),
         }
 
+        //--- OctetsFrom
+
+        impl<Octs, Name, SrcOcts, SrcName>
+        OctetsFrom<AllOptData<SrcOcts, SrcName>>
+        for AllOptData<Octs, Name>
+        where
+            Octs: OctetsFrom<SrcOcts>,
+            Name: OctetsFrom<SrcName, Error = Octs::Error>,
+        {
+            type Error = Octs::Error;
+
+            fn try_octets_from(
+                source: AllOptData<SrcOcts, SrcName>,
+            ) -> Result<Self, Self::Error> {
+                match source {
+                    $( $(
+                        AllOptData::$opt(opt) => {
+                            Ok(AllOptData::$opt(
+                                $module::$opt::try_octets_from(opt)?
+                            ))
+                        },
+                    )* )*
+                    AllOptData::Other(opt) => {
+                        Ok(AllOptData::Other(
+                            UnknownOptData::try_octets_from(opt)?
+                        ))
+                    }
+                }
+            }
+        }
+
         //--- From
 
         $( $(
@@ -101,5 +131,23 @@ macro_rules! opt_types {
                 }
             }
         }
+
+        //--- Debug
+
+        impl<Octs, Name> fmt::Debug for AllOptData<Octs, Name>
+        where Octs: AsRef<[u8]>, Name: fmt::Display {
+            fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+                match *self {
+                    $( $(
+                        AllOptData::$opt(ref inner) => {
+                            fmt::Debug::fmt(inner, f)
+                        }
+                    )* )*
+                    AllOptData::Other(ref inner) => {
+                            fmt::Debug::fmt(inner, f)
+                    }
+                }
+            }
+        }
     }
 }
diff --git a/src/base/opt/mod.rs b/src/base/opt/mod.rs
index 5557ce94..27fcbab1 100644
--- a/src/base/opt/mod.rs
+++ b/src/base/opt/mod.rs
@@ -830,7 +830,7 @@ pub trait ComposeOptData: OptData {
 /// An OPT option in its raw form.
 ///
 /// This type accepts any option type via its option code and raw data.
-#[derive(Clone, Debug)]
+#[derive(Clone)]
 pub struct UnknownOptData<Octs> {
     /// The option code for the option.
     code: OptionCode,
@@ -888,6 +888,23 @@ impl<Octs> UnknownOptData<Octs> {
     }
 }
 
+//--- OctetsFrom
+
+impl<Octs, SrcOcts> OctetsFrom<UnknownOptData<SrcOcts>>
+    for UnknownOptData<Octs>
+where
+    Octs: OctetsFrom<SrcOcts>,
+{
+    type Error = Octs::Error;
+
+    fn try_octets_from(
+        src: UnknownOptData<SrcOcts>,
+    ) -> Result<Self, Self::Error> {
+        Ok(unsafe {
+            Self::new_unchecked(src.code, Octs::try_octets_from(src.data)?)
+        })
+    }
+}
 //--- AsRef and AsMut
 
 impl<Octs> AsRef<Octs> for UnknownOptData<Octs> {
@@ -947,7 +964,7 @@ impl<Octs: AsRef<[u8]>> ComposeOptData for UnknownOptData<Octs> {
     }
 }
 
-//--- Display
+//--- Display and Debug
 
 impl<Octs: AsRef<[u8]>> fmt::Display for UnknownOptData<Octs> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
@@ -955,6 +972,15 @@ impl<Octs: AsRef<[u8]>> fmt::Display for UnknownOptData<Octs> {
     }
 }
 
+impl<Octs: AsRef<[u8]>> fmt::Debug for UnknownOptData<Octs> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_struct("UnknownOptData")
+            .field("code", &self.code)
+            .field("data", &format_args!("{}", self))
+            .finish()
+    }
+}
+
 //============ Error Types ===================================================
 
 //------------ LongOptData ---------------------------------------------------
diff --git a/src/base/opt/nsid.rs b/src/base/opt/nsid.rs
index 45207270..50399c31 100644
--- a/src/base/opt/nsid.rs
+++ b/src/base/opt/nsid.rs
@@ -14,7 +14,7 @@ use super::{
     BuildDataError, LongOptData, Opt, OptData, ComposeOptData, ParseOptData
 };
 use octseq::builder::OctetsBuilder;
-use octseq::octets::Octets;
+use octseq::octets::{Octets, OctetsFrom};
 use octseq::parse::Parser;
 use core::{borrow, fmt, hash, str};
 use core::cmp::Ordering;
@@ -129,6 +129,19 @@ impl<Octs: ?Sized> Nsid<Octs> {
     }
 }
 
+//--- OctetsFrom
+
+impl<Octs, SrcOcts> OctetsFrom<Nsid<SrcOcts>> for Nsid<Octs>
+where Octs: OctetsFrom<SrcOcts> {
+    type Error = Octs::Error;
+
+    fn try_octets_from(src: Nsid<SrcOcts>) -> Result<Self, Self::Error> {
+        Octs::try_octets_from(src.octets).map(|octets| unsafe {
+            Self::from_octets_unchecked(octets)
+        })
+    }
+}
+
 //--- AsRef and Borrow
 
 impl<Octs: AsRef<[u8]> + ?Sized> AsRef<[u8]> for Nsid<Octs> {
diff --git a/src/base/opt/padding.rs b/src/base/opt/padding.rs
index 086a70ad..33b7446d 100644
--- a/src/base/opt/padding.rs
+++ b/src/base/opt/padding.rs
@@ -14,7 +14,7 @@ use super::super::message_builder::OptBuilder;
 use super::super::wire::{Compose, Composer, ParseError};
 use super::{LongOptData, OptData, ComposeOptData, ParseOptData};
 use octseq::builder::OctetsBuilder;
-use octseq::octets::Octets;
+use octseq::octets::{Octets, OctetsFrom};
 use octseq::parse::Parser;
 
 
@@ -91,6 +91,19 @@ impl<Octs: ?Sized> Padding<Octs> {
     }
 }
 
+//--- OctetsFrom
+
+impl<Octs, SrcOcts> OctetsFrom<Padding<SrcOcts>> for Padding<Octs>
+where Octs: OctetsFrom<SrcOcts> {
+    type Error = Octs::Error;
+
+    fn try_octets_from(src: Padding<SrcOcts>) -> Result<Self, Self::Error> {
+        Octs::try_octets_from(src.octets).map(|octets| unsafe {
+            Self::from_octets_unchecked(octets)
+        })
+    }
+}
+
 //--- AsRef and Borrow
 
 impl<Octs: AsRef<[u8]> + ?Sized> AsRef<[u8]> for Padding<Octs> {
diff --git a/src/base/opt/subnet.rs b/src/base/opt/subnet.rs
index 7a6f3efc..eae4f9ac 100644
--- a/src/base/opt/subnet.rs
+++ b/src/base/opt/subnet.rs
@@ -169,6 +169,13 @@ impl ClientSubnet {
             addr,
         })
     }
+
+    /// Placeholder for unnecessary octets conversion.
+    ///
+    /// This method only exists for the `AllOptData` macro.
+    pub(super) fn try_octets_from<E>(src: Self) -> Result<Self, E> {
+        Ok(src)
+    }
 }
 
 //--- OptData