From 4f7d898a86c076852c78347dda395271d8a9bd9b Mon Sep 17 00:00:00 2001
From: Markus Pettersson <markus.pettersson@mullvad.net>
Date: Mon, 6 Jan 2025 04:19:04 +0100
Subject: [PATCH] Replace `Ipv{4,6}Network::new_unchecked` with
 `Ipv{4,6}Network::new_checked` (#203)

* Replace `Ipv4Network::new_unchecked` with `Ipv4Network::new_checked`

* Replace `Ipv6Network::new_unchecked` with `Ipv6Network::new_checked`

* Update doc test for `Ipv{4,6}Network::new_checked` to use `should_panic`

* Better demonstrate use of `Ipv{4,6}Network::new_checked` in const contexts

* Add back `unsafe_code` lint at the deny level
---
 src/ipv4.rs | 58 +++++++++++++++++++++++++++++++++++-----------------
 src/ipv6.rs | 59 +++++++++++++++++++++++++++++++++++------------------
 src/lib.rs  |  1 +
 3 files changed, 79 insertions(+), 39 deletions(-)

diff --git a/src/ipv4.rs b/src/ipv4.rs
index e7acdee..58d834c 100644
--- a/src/ipv4.rs
+++ b/src/ipv4.rs
@@ -67,19 +67,17 @@ impl Ipv4Network {
     ///
     /// If the prefix is larger than 32 this will return an `IpNetworkError::InvalidPrefix`.
     pub const fn new(addr: Ipv4Addr, prefix: u8) -> Result<Ipv4Network, IpNetworkError> {
-        if prefix > IPV4_BITS {
-            Err(IpNetworkError::InvalidPrefix)
-        } else {
-            Ok(Ipv4Network { addr, prefix })
+        match Ipv4Network::new_checked(addr, prefix) {
+            Some(a) => Ok(a),
+            None => Err(IpNetworkError::InvalidPrefix),
         }
     }
 
-    /// Constructs without checking prefix a new `Ipv4Network` from any `Ipv4Addr,
-    /// and a prefix denoting the network size.
+    /// Constructs a new `Ipv4Network` from any `Ipv4Addr`, and a prefix denoting the network size.
     ///
-    /// # Safety
-    ///
-    /// The caller must ensure that the prefix is less than or equal to 32.
+    /// If the prefix is larger than 32 this will return `None`. This is useful in const contexts,
+    /// where [`Option::unwrap`] may be called to trigger a compile-time error in case the prefix
+    /// is an unexpected value.
     ///
     /// # Examples
     ///
@@ -87,14 +85,32 @@ impl Ipv4Network {
     /// use std::net::Ipv4Addr;
     /// use ipnetwork::Ipv4Network;
     ///
-    /// let prefix = 24;
-    /// let addr = Ipv4Addr::new(192, 168, 1, 1);
+    /// const PREFIX: u8 = 24;
+    /// const ADDR: Ipv4Addr = Ipv4Addr::new(192, 168, 1, 1);
     ///
-    /// debug_assert!(prefix <= 32);
-    /// let network = unsafe { Ipv4Network::new_unchecked(addr, prefix) };
+    /// // Okay!
+    /// const NETWORK: Ipv4Network = Ipv4Network::new_checked(ADDR, PREFIX).unwrap();
+    /// assert_eq!(NETWORK.prefix(), PREFIX);
     /// ```
-    pub const unsafe fn new_unchecked(addr: Ipv4Addr, prefix: u8) -> Ipv4Network {
-        Ipv4Network { addr, prefix }
+    ///
+    /// ```should_panic
+    /// use std::net::Ipv4Addr;
+    /// use ipnetwork::Ipv4Network;
+    ///
+    /// // Prefix is greater than 32.
+    /// const PREFIX: u8 = 32 + 1;
+    /// const ADDR: Ipv4Addr = Ipv4Addr::new(192, 168, 1, 1);
+    ///
+    /// // This fails!
+    /// const NETWORK: Option<Ipv4Network> = Ipv4Network::new_checked(ADDR, PREFIX);
+    /// assert_eq!(NETWORK.unwrap().prefix(), PREFIX);
+    /// ```
+    pub const fn new_checked(addr: Ipv4Addr, prefix: u8) -> Option<Ipv4Network> {
+        if prefix > IPV4_BITS {
+            None
+        } else {
+            Some(Ipv4Network { addr, prefix })
+        }
     }
 
     /// Constructs a new `Ipv4Network` from a network address and a network mask.
@@ -396,11 +412,15 @@ mod test {
     }
 
     #[test]
-    fn create_unchecked_v4() {
-        let cidr = unsafe { Ipv4Network::new_unchecked(Ipv4Addr::new(77, 88, 21, 11), 24) };
+    fn create_checked_v4() {
+        let cidr = Ipv4Network::new_checked(Ipv4Addr::new(77, 88, 21, 11), 24).unwrap();
         assert_eq!(cidr.prefix(), 24);
-        let cidr = unsafe { Ipv4Network::new_unchecked(Ipv4Addr::new(0, 0, 0, 0), 33) };
-        assert_eq!(cidr.prefix(), 33);
+    }
+
+    #[test]
+    #[should_panic]
+    fn try_create_invalid_checked_v4() {
+        Ipv4Network::new_checked(Ipv4Addr::new(0, 0, 0, 0), 33).unwrap();
     }
 
     #[test]
diff --git a/src/ipv6.rs b/src/ipv6.rs
index ea35cc3..fb12f1a 100644
--- a/src/ipv6.rs
+++ b/src/ipv6.rs
@@ -78,19 +78,17 @@ impl Ipv6Network {
     ///
     /// If the prefix is larger than 128 this will return an `IpNetworkError::InvalidPrefix`.
     pub const fn new(addr: Ipv6Addr, prefix: u8) -> Result<Ipv6Network, IpNetworkError> {
-        if prefix > IPV6_BITS {
-            Err(IpNetworkError::InvalidPrefix)
-        } else {
-            Ok(Ipv6Network { addr, prefix })
+        match Ipv6Network::new_checked(addr, prefix) {
+            Some(a) => Ok(a),
+            None => Err(IpNetworkError::InvalidPrefix),
         }
     }
 
-    /// Constructs without checking prefix a new `Ipv6Network` from any `Ipv6Addr,
-    /// and a prefix denoting the network size.
+    /// Constructs a new `Ipv6Network` from any `Ipv6Addr`, and a prefix denoting the network size.
     ///
-    /// # Safety
-    ///
-    /// The caller must ensure that the prefix is less than or equal to 128.
+    /// If the prefix is larger than 128 this will return `None`. This is useful in const contexts,
+    /// where [`Option::unwrap`] may be called to trigger a compile-time error in case the prefix
+    /// is an unexpected value.
     ///
     /// # Examples
     ///
@@ -98,14 +96,32 @@ impl Ipv6Network {
     /// use std::net::Ipv6Addr;
     /// use ipnetwork::Ipv6Network;
     ///
-    /// let prefix = 64;
-    /// let addr = Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 0);
+    /// const PREFIX: u8 = 64;
+    /// const ADDR: Ipv6Addr = Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 0);
     ///
-    /// debug_assert!(prefix <= 128);
-    /// let net = unsafe { Ipv6Network::new_unchecked(addr, prefix) };
+    /// // Okay!
+    /// const NETWORK: Ipv6Network = Ipv6Network::new_checked(ADDR, PREFIX).unwrap();
+    /// assert_eq!(NETWORK.prefix(), PREFIX);
     /// ```
-    pub const unsafe fn new_unchecked(addr: Ipv6Addr, prefix: u8) -> Ipv6Network {
-        Ipv6Network { addr, prefix }
+    ///
+    /// ```should_panic
+    /// use std::net::Ipv6Addr;
+    /// use ipnetwork::Ipv6Network;
+    ///
+    /// // Prefix is greater than 128.
+    /// const PREFIX: u8 = 128 + 1;
+    /// const ADDR: Ipv6Addr = Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 0);
+    ///
+    /// // This fails!
+    /// const NETWORK: Option<Ipv6Network> = Ipv6Network::new_checked(ADDR, PREFIX);
+    /// assert_eq!(NETWORK.unwrap().prefix(), PREFIX);
+    /// ```
+    pub const fn new_checked(addr: Ipv6Addr, prefix: u8) -> Option<Ipv6Network> {
+        if prefix > IPV6_BITS {
+            None
+        } else {
+            Some(Ipv6Network { addr, prefix })
+        }
     }
 
     /// Constructs a new `Ipv6Network` from a network address and a network mask.
@@ -427,12 +443,15 @@ mod test {
     }
 
     #[test]
-    fn create_unchecked_v6() {
-        let cidr = unsafe { Ipv6Network::new_unchecked(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1), 24) };
+    fn create_checked_v6() {
+        let cidr = Ipv6Network::new_checked(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1), 24).unwrap();
         assert_eq!(cidr.prefix(), 24);
-        let cidr =
-            unsafe { Ipv6Network::new_unchecked(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1), 129) };
-        assert_eq!(cidr.prefix(), 129);
+    }
+
+    #[test]
+    #[should_panic]
+    fn try_create_invalid_checked_v6() {
+        Ipv6Network::new_checked(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1), 129).unwrap();
     }
 
     #[test]
diff --git a/src/lib.rs b/src/lib.rs
index 48df9c0..4931c0a 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -3,6 +3,7 @@
 #![crate_type = "lib"]
 #![deny(
     missing_debug_implementations,
+    unsafe_code,
     unused_extern_crates,
     unused_import_braces
 )]