[pdm-devel] [PATCH proxmox v2 02/14] network-types: move `Fqdn` type from proxmox-installer-common

[Christoph Heiss <c.heiss@proxmox.com>]

This introduces an `Fqdn` type for safely representing (valid) FQDNs on
Debian, following all relevant RFCs as well as restrictions given by
Debian.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
  * no changes

 proxmox-network-types/Cargo.toml     |   3 +-
 proxmox-network-types/debian/control |   2 +
 proxmox-network-types/src/fqdn.rs    | 248 +++++++++++++++++++++++++++
 proxmox-network-types/src/lib.rs     |   1 +
 4 files changed, 253 insertions(+), 1 deletion(-)
 create mode 100644 proxmox-network-types/src/fqdn.rs

diff --git a/proxmox-network-types/Cargo.toml b/proxmox-network-types/Cargo.toml
index 6333a37f..25c4bcf2 100644
--- a/proxmox-network-types/Cargo.toml
+++ b/proxmox-network-types/Cargo.toml
@@ -10,9 +10,10 @@ exclude.workspace = true
 rust-version.workspace = true
 
 [dependencies]
-regex = { workspace = true, optional = true}
+regex = { workspace = true, optional = true }
 serde = { workspace = true, features = [ "derive", "std" ] }
 serde_with = "3.8.1"
+serde_plain.workspace = true
 thiserror.workspace = true
 
 proxmox-schema = { workspace = true, features = [ "api-macro", "api-types" ], optional = true}
diff --git a/proxmox-network-types/debian/control b/proxmox-network-types/debian/control
index 8b68deb1..08df0f9f 100644
--- a/proxmox-network-types/debian/control
+++ b/proxmox-network-types/debian/control
@@ -9,6 +9,7 @@ Build-Depends-Arch: cargo:native <!nocheck>,
  librust-serde-1+default-dev <!nocheck>,
  librust-serde-1+derive-dev <!nocheck>,
  librust-serde-1+std-dev <!nocheck>,
+ librust-serde-plain-1+default-dev <!nocheck>,
  librust-serde-with-3+default-dev (>= 3.8.1-~~) <!nocheck>,
  librust-thiserror-2+default-dev <!nocheck>
 Maintainer: Proxmox Support Team <support@proxmox.com>
@@ -26,6 +27,7 @@ Depends:
  librust-serde-1+default-dev,
  librust-serde-1+derive-dev,
  librust-serde-1+std-dev,
+ librust-serde-plain-1+default-dev,
  librust-serde-with-3+default-dev (>= 3.8.1-~~),
  librust-thiserror-2+default-dev
 Suggests:
diff --git a/proxmox-network-types/src/fqdn.rs b/proxmox-network-types/src/fqdn.rs
new file mode 100644
index 00000000..9582639d
--- /dev/null
+++ b/proxmox-network-types/src/fqdn.rs
@@ -0,0 +1,248 @@
+//! A type for safely representing fully-qualified domain names (FQDNs).
+
+use std::{fmt, str::FromStr};
+
+use serde::Deserialize;
+
+/// Possible errors that might occur when parsing FQDNs.
+#[derive(Debug, Eq, PartialEq)]
+pub enum FqdnParseError {
+    MissingHostname,
+    NumericHostname,
+    InvalidPart(String),
+    TooLong(usize),
+}
+
+impl fmt::Display for FqdnParseError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        use FqdnParseError::*;
+        match self {
+            MissingHostname => write!(f, "missing hostname part"),
+            NumericHostname => write!(f, "hostname cannot be purely numeric"),
+            InvalidPart(part) => write!(
+                f,
+                "FQDN must only consist of alphanumeric characters and dashes. Invalid part: '{part}'",
+            ),
+            TooLong(len) => write!(f, "FQDN too long: {len} > {}", Fqdn::MAX_LENGTH),
+        }
+    }
+}
+
+/// A type for safely representing fully-qualified domain names (FQDNs).
+///
+/// It considers following RFCs:
+/// - [RFC952] (sec. "ASSUMPTIONS", 1.)
+/// - [RFC1035] (sec. 2.3. "Conventions")
+/// - [RFC1123] (sec. 2.1. "Host Names and Numbers")
+/// - [RFC3492]
+/// - [RFC4343]
+///
+/// .. and applies some restriction given by Debian, e.g. 253 instead of 255
+/// maximum total length and maximum 63 characters per label, per the
+/// [hostname(7)].
+///
+/// Additionally:
+/// - It enforces the restriction as per Bugzilla #1054, in that
+///   purely numeric hostnames are not allowed - against RFC1123 sec. 2.1.
+///
+/// Some terminology:
+/// - "label" - a single part of a FQDN, e.g. {label}.{label}.{tld}
+///
+/// [RFC952]: <https://www.ietf.org/rfc/rfc952.txt>
+/// [RFC1035]: <https://www.ietf.org/rfc/rfc1035.txt>
+/// [RFC1123]: <https://www.ietf.org/rfc/rfc1123.txt>
+/// [RFC3492]: <https://www.ietf.org/rfc/rfc3492.txt>
+/// [RFC4343]: <https://www.ietf.org/rfc/rfc4343.txt>
+/// [hostname(7)]: <https://manpages.debian.org/stable/manpages/hostname.7.en.html>
+#[derive(Clone, Debug, Eq)]
+pub struct Fqdn {
+    parts: Vec<String>,
+}
+
+impl Fqdn {
+    /// Maximum length of a single label of the FQDN
+    const MAX_LABEL_LENGTH: usize = 63;
+    /// Maximum total length of the FQDN
+    const MAX_LENGTH: usize = 253;
+
+    pub fn from(fqdn: &str) -> Result<Self, FqdnParseError> {
+        if fqdn.len() > Self::MAX_LENGTH {
+            return Err(FqdnParseError::TooLong(fqdn.len()));
+        }
+
+        let parts = fqdn
+            .split('.')
+            .map(ToOwned::to_owned)
+            .collect::<Vec<String>>();
+
+        for part in &parts {
+            if !Self::validate_single(part) {
+                return Err(FqdnParseError::InvalidPart(part.clone()));
+            }
+        }
+
+        if parts.len() < 2 {
+            Err(FqdnParseError::MissingHostname)
+        } else if parts[0].chars().all(|c| c.is_ascii_digit()) {
+            // Do not allow a purely numeric hostname, see:
+            // https://bugzilla.proxmox.com/show_bug.cgi?id=1054
+            Err(FqdnParseError::NumericHostname)
+        } else {
+            Ok(Self { parts })
+        }
+    }
+
+    pub fn host(&self) -> Option<&str> {
+        self.has_host().then_some(&self.parts[0])
+    }
+
+    pub fn domain(&self) -> String {
+        let parts = if self.has_host() {
+            &self.parts[1..]
+        } else {
+            &self.parts
+        };
+
+        parts.join(".")
+    }
+
+    /// Checks whether the FQDN has a hostname associated with it, i.e. is has more than 1 part.
+    fn has_host(&self) -> bool {
+        self.parts.len() > 1
+    }
+
+    fn validate_single(s: &str) -> bool {
+        !s.is_empty()
+            && s.len() <= Self::MAX_LABEL_LENGTH
+            // First character must be alphanumeric
+            && s.chars()
+                .next()
+                .map(|c| c.is_ascii_alphanumeric())
+                .unwrap_or_default()
+            // .. last character as well,
+            && s.chars()
+                .last()
+                .map(|c| c.is_ascii_alphanumeric())
+                .unwrap_or_default()
+            // and anything between must be alphanumeric or -
+            && s.chars()
+                .skip(1)
+                .take(s.len().saturating_sub(2))
+                .all(|c| c.is_ascii_alphanumeric() || c == '-')
+    }
+}
+
+impl FromStr for Fqdn {
+    type Err = FqdnParseError;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        Self::from(value)
+    }
+}
+
+impl fmt::Display for Fqdn {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{}", self.parts.join("."))
+    }
+}
+
+serde_plain::derive_serialize_from_display!(Fqdn);
+
+impl<'de> Deserialize<'de> for Fqdn {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        let s: String = Deserialize::deserialize(deserializer)?;
+        s.parse()
+            .map_err(|_| serde::de::Error::custom("invalid FQDN"))
+    }
+}
+
+impl PartialEq for Fqdn {
+    // Case-insensitive comparison, as per RFC 952 "ASSUMPTIONS", RFC 1035 sec. 2.3.3. "Character
+    // Case" and RFC 4343 as a whole
+    fn eq(&self, other: &Self) -> bool {
+        if self.parts.len() != other.parts.len() {
+            return false;
+        }
+
+        self.parts
+            .iter()
+            .zip(other.parts.iter())
+            .all(|(a, b)| a.to_lowercase() == b.to_lowercase())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn fqdn_construct() {
+        use FqdnParseError::*;
+        assert!(Fqdn::from("foo.example.com").is_ok());
+        assert!(Fqdn::from("foo-bar.com").is_ok());
+        assert!(Fqdn::from("a-b.com").is_ok());
+
+        assert_eq!(Fqdn::from("foo"), Err(MissingHostname));
+
+        assert_eq!(Fqdn::from("-foo.com"), Err(InvalidPart("-foo".to_owned())));
+        assert_eq!(Fqdn::from("foo-.com"), Err(InvalidPart("foo-".to_owned())));
+        assert_eq!(Fqdn::from("foo.com-"), Err(InvalidPart("com-".to_owned())));
+        assert_eq!(Fqdn::from("-o-.com"), Err(InvalidPart("-o-".to_owned())));
+
+        // https://bugzilla.proxmox.com/show_bug.cgi?id=1054
+        assert_eq!(Fqdn::from("123.com"), Err(NumericHostname));
+        assert!(Fqdn::from("foo123.com").is_ok());
+        assert!(Fqdn::from("123foo.com").is_ok());
+
+        assert!(Fqdn::from(&format!("{}.com", "a".repeat(63))).is_ok());
+        assert_eq!(
+            Fqdn::from(&format!("{}.com", "a".repeat(250))),
+            Err(TooLong(254)),
+        );
+        assert_eq!(
+            Fqdn::from(&format!("{}.com", "a".repeat(64))),
+            Err(InvalidPart("a".repeat(64))),
+        );
+
+        // https://bugzilla.proxmox.com/show_bug.cgi?id=5230
+        assert_eq!(
+            Fqdn::from("123@foo.com"),
+            Err(InvalidPart("123@foo".to_owned()))
+        );
+    }
+
+    #[test]
+    fn fqdn_parts() {
+        let fqdn = Fqdn::from("pve.example.com").unwrap();
+        assert_eq!(fqdn.host().unwrap(), "pve");
+        assert_eq!(fqdn.domain(), "example.com");
+        assert_eq!(
+            fqdn.parts,
+            &["pve".to_owned(), "example".to_owned(), "com".to_owned()]
+        );
+    }
+
+    #[test]
+    fn fqdn_display() {
+        assert_eq!(
+            Fqdn::from("foo.example.com").unwrap().to_string(),
+            "foo.example.com"
+        );
+    }
+
+    #[test]
+    fn fqdn_compare() {
+        assert_eq!(Fqdn::from("example.com"), Fqdn::from("example.com"));
+        assert_eq!(Fqdn::from("example.com"), Fqdn::from("ExAmPle.Com"));
+        assert_eq!(Fqdn::from("ExAmPle.Com"), Fqdn::from("example.com"));
+        assert_ne!(
+            Fqdn::from("subdomain.ExAmPle.Com"),
+            Fqdn::from("example.com")
+        );
+        assert_ne!(Fqdn::from("foo.com"), Fqdn::from("bar.com"));
+        assert_ne!(Fqdn::from("example.com"), Fqdn::from("example.net"));
+    }
+}
diff --git a/proxmox-network-types/src/lib.rs b/proxmox-network-types/src/lib.rs
index ee26b1c1..e5d31285 100644
--- a/proxmox-network-types/src/lib.rs
+++ b/proxmox-network-types/src/lib.rs
@@ -1,5 +1,6 @@
 #![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
 #![deny(unsafe_op_in_unsafe_fn)]
 
+pub mod fqdn;
 pub mod ip_address;
 pub mod mac_address;
-- 
2.51.2



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel