Merge pull request #99 from hannesm/cidr

store CIDR in Prefix.t (do not mask out non-prefix bits)
mirage · Jun 16, 2020 · 71882ae · 71882ae
2 parents 5dd3a72 + 8d79f78
commit 71882ae
Show file tree

Hide file tree

Showing 5 changed files with 132 additions and 131 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -13,3 +13,6 @@ env:
    - DISTRO=ubuntu OCAML_VERSION=4.05
    - DISTRO=alpine OCAML_VERSION=4.06
    - DISTRO=fedora OCAML_VERSION=4.07
+   - DISTRO=alpine OCAML_VERSION=4.08
+   - DISTRO=alpine OCAML_VERSION=4.09
+   - DISTRO=alpine OCAML_VERSION=4.10
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,11 +1,39 @@
 ## v5.0.0
 
-* Use stdlib-shims to prevent deprecation warnings on OCaml 4.08
-  (@avsm)
+* Do not zero out the non-prefix-length part of the address in
+  `{V4,V6}.Prefix.t` (#99 @hannesm)
+  Removed `{V4,V6}.Prefix.of_address_string{,_exn}` and
+  `{V4,V6}.Prefix.to_address_{string.buffer}`
+
+  To port code:
+  - if you rely on `Prefix.of_string` to zero out the non-prefix-length address
+    bits, call `Prefix.prefix : t -> t` subsequently.
+  - `Prefix.of_address_string{,_exn}` can be replaced by
+    `Prefix.of_string{,_exn}`, to retrieve the address use
+    `Prefix.address : t -> addr`.
+  - The `Prefix.to_address_{string,buffer}` can be replaced by
+    `Prefix.to_{string,buffer}`, where `Prefix.t` already contains the IP
+    address to be printed.
+  - Instead of passing `{V4,V6}.t * {V4,V6}.Prefix.t` for an
+    address and subnet configuration, `{V4,V6}.Prefix.t` is sufficient.
+
+* Implement `{V4,V6,}.succ`, `{V4,V6,}.pred`, `{V4,V6}.Prefix.first`, and
+  `{V4,V6}.Prefix.last` functions (#94 @NightBlues)
+
 * Rename `Prefix.of_netmask` to `Prefix.of_netmask_exn` with labelled
   arguments (~netmask and ~address), provide `Prefix.of_netmask` which returns
   a (t, [> `Msg of string ]) result value (#95 @hannesm)
 
+* Fix undefined behaviour of `V4.Prefix.mem` with a CIDR with prefix length 0
+  (#98 @verbosemode)
+
+* Use stdlib-shims to prevent deprecation warnings on OCaml 4.08
+  (@avsm)
+
+* Remove unnecessary "sexplib0" dependency (#95 @hannesm)
+
+* Remove "{build}" directive from "dune" dependency (#93 @CraigFe)
+
 ## v4.0.0 (2019-07-12)
 
 * Rename the `to/from_bytes` functions to refer to `octets`

diff --git a/lib/ipaddr.ml b/lib/ipaddr.ml
@@ -308,10 +308,12 @@ module V4 = struct
       else if sz >= 32 then 0x0_FF_FF_FF_FF_l
       else 0x0_FF_FF_FF_FF_l <|< (32 - sz)
 
-    let make sz pre = (pre &&& (mask sz),sz)
+    let prefix (pre,sz) = (pre &&& (mask sz), sz)
+
+    let make sz pre = (pre,sz)
 
     let network_address (pre,sz) addr =
-      pre ||| (addr &&& Int32.lognot (mask sz))
+      (pre &&& (mask sz)) ||| (addr &&& Int32.lognot (mask sz))
 
     (* string conversion *)
 
@@ -337,11 +339,6 @@ module V4 = struct
 
     let of_string s = try_with_result of_string_exn s
 
-    let of_address_string_exn s =
-      let (p,quad) = _of_string_exn s in (make p quad, quad)
-
-    let of_address_string s = try_with_result of_address_string_exn s
-
     let _of_netmask_exn ~netmask address =
       let rec find_greatest_one bits i =
         if bits = 0_l then i-1 else find_greatest_one (bits >|> 1) (i+1)
@@ -367,14 +364,6 @@ module V4 = struct
     let pp ppf i =
       Format.fprintf ppf "%s" (to_string i)
 
-    let to_address_buffer buf ((_,sz) as subnet) addr =
-      to_buffer buf (network_address subnet addr,sz)
-
-    let to_address_string subnet addr =
-      let b = Buffer.create 18 in
-      to_address_buffer b subnet addr;
-      Buffer.contents b
-
     let mem ip (pre,sz) =
       let m = mask sz in
       (ip &&& m) = (pre &&& m)
@@ -403,21 +392,22 @@ module V4 = struct
     ]
 
     let broadcast (pre,sz) = pre ||| (0x0_FF_FF_FF_FF_l >|> sz)
-    let network (pre,_) = pre
+    let network (pre,sz) = pre &&& mask sz
+    let address (addr,_) = addr
     let bits (_,sz) = sz
     let netmask subnet = mask (bits subnet)
 
-    let first (pre,sz) =
+    let first (_,sz as cidr) =
       if sz > 30 then
-        pre
+        network cidr
       else
-        succ pre |> failwith_msg
+        network cidr |> succ |> failwith_msg
 
-    let last (_,sz as t) =
+    let last (_,sz as cidr) =
       if sz > 30 then
-        broadcast t
+        broadcast cidr
       else
-        broadcast t |> pred |> failwith_msg
+        broadcast cidr |> pred |> failwith_msg
   end
 
   (* TODO: this could be optimized with something trie-like *)
@@ -854,10 +844,12 @@ module V6 = struct
       mask (sz - 64),
       mask (sz - 96))
 
-    let make sz pre = (logand pre (mask sz),sz)
+    let prefix (pre,sz) = (logand pre (mask sz),sz)
+
+    let make sz pre = (pre,sz)
 
     let network_address (pre,sz) addr =
-      logor pre (logand addr (lognot (mask sz)))
+      logor (logand pre (mask sz)) (logand addr (lognot (mask sz)))
 
     let _of_string_raw s i =
       let v6 = of_string_raw s i in
@@ -881,11 +873,6 @@ module V6 = struct
 
     let of_string s = try_with_result of_string_exn s
 
-    let of_address_string_exn s =
-      let (p,v6) = _of_string_exn s in (make p v6, v6)
-
-    let of_address_string s = try_with_result of_address_string_exn s
-
     let _of_netmask_exn ~netmask address =
       let nm =
         let bits netmask =
@@ -917,14 +904,6 @@ module V6 = struct
     let pp ppf i =
       Format.fprintf ppf "%s" (to_string i)
 
-    let to_address_buffer buf ((_,sz) as subnet) addr =
-      to_buffer buf (network_address subnet addr,sz)
-
-    let to_address_string subnet addr =
-      let b = Buffer.create 43 in
-      to_address_buffer b subnet addr;
-      Buffer.contents b
-
     let mem ip (pre,sz) =
       let m = mask sz in
       logand ip m = logand pre m
@@ -942,20 +921,21 @@ module V6 = struct
     let noneui64_interface  = make   3 (ip 0x0000 0 0 0 0 0 0 0)
     let solicited_node      = make 104 (ip 0xff02 0 0 0 0 1 0xff00 0)
 
-    let network (pre,_) = pre
+    let network (pre,sz) = logand pre (mask sz)
+    let address (addr,_) = addr
     let bits (_,sz) = sz
     let netmask subnet = mask (bits subnet)
 
-    let first (pre,sz) =
+    let first (_,sz as cidr) =
       if sz > 126 then
-        pre
+        network cidr
       else
-        succ pre |> failwith_msg
+        network cidr |> succ |> failwith_msg
 
-    let last (pre,sz) =
+    let last (_,sz as cidr) =
       let ffff = ip 0xffff 0xffff 0xffff 0xffff
           0xffff 0xffff 0xffff 0xffff in
-      logor pre (shift_right ffff sz |> failwith_msg)
+      logor (network cidr) (shift_right ffff sz |> failwith_msg)
   end
 
   (* TODO: This could be optimized with something trie-like *)

diff --git a/lib/ipaddr.mli b/lib/ipaddr.mli
@@ -178,20 +178,24 @@ module V4 : sig
   (** [localhost] is 127.0.0.1. *)
   val localhost : t
 
-  (** A module for manipulating IPv4 network prefixes. *)
+  (** A module for manipulating IPv4 network prefixes (CIDR). *)
   module Prefix : sig
     type addr = t
 
-    (** Type of a internet protocol subnet *)
+    (** Type of a internet protocol subnet: an address and prefix length. *)
     type t
 
     (** [mask n] is the pseudo-address of an [n] bit subnet mask. *)
     val mask : int -> addr
 
-    (** [make n addr] is the [n] bit subnet prefix to which [addr] belongs. *)
+    (** [make n addr] is the cidr of [addr] with [n] bits prefix. *)
     val make : int -> addr -> t
 
-    (** [network_address prefix addr] is the address with prefix [prefix]
+    (** [prefix cidr] is the subnet prefix of [cidr] where all non-prefix bits
+        set to 0. *)
+    val prefix : t -> t
+
+    (** [network_address cidr addr] is the address with prefix [cidr]
         and suffix from [addr].
         See <http://tools.ietf.org/html/rfc4291#section-2.3>. *)
     val network_address : t -> addr -> addr
@@ -211,40 +215,23 @@ module V4 : sig
         into the string for reading. *)
     val of_string_raw : string -> int ref -> t
 
-    (** [to_string prefix] is the CIDR notation string representation
-        of [prefix], i.e. [XXX.XX.X.XXX/XX]. *)
+    (** [to_string cidr] is the CIDR notation string representation
+        of [cidr], i.e. [XXX.XX.X.XXX/XX]. *)
     val to_string : t -> string
 
-    (** [pp f prefix] outputs a human-readable representation of [prefix]
+    (** [pp f cidr] outputs a human-readable representation of [cidr]
         to the formatter [f]. *)
     val pp : Format.formatter -> t -> unit [@@ocaml.toplevel_printer]
 
-    (** [of_address_string_exn cidr_addr] is the address and prefix
-        represented by [cidr_addr]. Raises {!Parse_error} if [cidr_addr] is not
-        a valid representation of a CIDR-scoped address. *)
-    val of_address_string_exn : string -> t * addr
-
-    (** Same as {!of_address_string_exn} but returns a result type instead of
-        raising an exception. *)
-    val of_address_string : string -> (t * addr, [> `Msg of string ]) result
-
-    (** [to_address_string prefix addr] is the network address
-        constructed from [prefix] and [addr]. *)
-    val to_address_string : t -> addr -> string
-
-    (** [to_buffer buf prefix] writes the string representation
-        of [prefix] into the buffer [buf]. *)
+    (** [to_buffer buf cidr] writes the string representation
+        of [cidr] into the buffer [buf]. *)
     val to_buffer : Buffer.t -> t -> unit
 
-    (** [to_address_buffer buf prefix addr] writes string representation of the
-        network address representing [addr] in [prefix] to the buffer [buf]. *)
-    val to_address_buffer : Buffer.t -> t -> addr -> unit
-
     (** [of_netmask_exn ~netmask ~address] is the subnet prefix of [address]
         with netmask [netmask]. *)
     val of_netmask_exn : netmask:addr -> address:addr -> t
 
-    (** [of_netmask ~netmask ~address] is the subnet prefix of [address] with
+    (** [of_netmask ~netmask ~address] is the cidr of [address] with
         netmask [netmask]. *)
     val of_netmask : netmask:addr -> address:addr ->
       (t, [> `Msg of string ]) result
@@ -297,13 +284,16 @@ module V4 : sig
     (** [netmask subnet] is the netmask for [subnet]. *)
     val netmask : t -> addr
 
-    (** [bits subnet] is the bit size of the [subnet] prefix. *)
+    (** [address cidr] is the address for [cidr]. *)
+    val address : t -> addr
+
+    (** [bits cidr] is the bit size of the [cidr] prefix. *)
     val bits : t -> int
 
-    (** [first subnet] is first valid unicast address in this [subnet]. *)
+    (** [first cidr] is first valid unicast address in this [cidr]. *)
     val first : t -> addr
 
-    (** [last subnet] is last valid unicast address in this [subnet]. *)
+    (** [last cidr] is last valid unicast address in this [cidr]. *)
     val last : t -> addr
 
     include Map.OrderedType with type t := t
@@ -386,7 +376,7 @@ module V6 : sig
       from offset [off] within [b].  [b] must be at least [off+16] bytes long or
       an error is returned. *)
   val write_octets : ?off:int -> t -> bytes -> (unit, [> `Msg of string ]) result
- 
+
   (** [to_octets ipv6] returns the 16 bytes representing the [ipv6] octets. *)
   val to_octets : t -> string
 
@@ -461,20 +451,24 @@ module V6 : sig
   (** [site_routers] is ff05::02. *)
   val site_routers : t
 
-  (** A module for manipulating IPv6 network prefixes. *)
+  (** A module for manipulating IPv6 network prefixes (CIDR). *)
   module Prefix : sig
     type addr = t
 
-    (** Type of a internet protocol subnet *)
+    (** Type of a internet protocol subnet: an address and a prefix length. *)
     type t
 
     (** [mask n] is the pseudo-address of an [n] bit subnet mask. *)
     val mask : int -> addr
 
-    (** [make n addr] is the [n] bit subnet prefix to which [addr] belongs. *)
+    (** [make n addr] is the cidr of [addr] with [n] bit prefix. *)
     val make : int -> addr -> t
 
-    (** [network_address prefix addr] is the address with prefix [prefix]
+    (** [prefix cidr] is the subnet prefix of [cidr] where all non-prefix bits
+        set to 0. *)
+    val prefix : t -> t
+
+    (** [network_address cidr addr] is the address with prefix [cidr]
         and suffix from [addr].
         See <http://tools.ietf.org/html/rfc4291#section-2.3>. *)
     val network_address : t -> addr -> addr
@@ -492,40 +486,23 @@ module V6 : sig
         into the string for reading. *)
     val of_string_raw : string -> int ref -> t
 
-    (** [to_string prefix] is the CIDR notation string representation
-        of [prefix], i.e. XXX:XX:X::XXX/XX. *)
+    (** [to_string cidr] is the CIDR notation string representation
+        of [cidr], i.e. XXX:XX:X::XXX/XX. *)
     val to_string : t -> string
 
-    (** [pp f prefix] outputs a human-readable representation of [prefix]
+    (** [pp f cidr] outputs a human-readable representation of [cidr]
         to the formatter [f]. *)
     val pp : Format.formatter -> t -> unit [@@ocaml.toplevel_printer]
 
-    (** [of_address_string_exn cidr_addr] is the address and prefix
-        represented by [cidr_addr]. Raises {!Parse_error} if [cidr_addr] is not
-        a valid representation of a CIDR-scoped address. *)
-    val of_address_string_exn : string -> t * addr
-
-    (** Same as {!of_address_string_exn} but returns an option type instead of
-        raising an exception. *)
-    val of_address_string : string -> ((t * addr), [> `Msg of string]) result
-
-    (** [to_address_string prefix addr] is the network address
-        constructed from [prefix] and [addr]. *)
-    val to_address_string : t -> addr -> string
-
-    (** [to_buffer buf prefix] writes the string representation
-        of [prefix] to the buffer [buf]. *)
+    (** [to_buffer buf cidr] writes the string representation
+        of [cidr] to the buffer [buf]. *)
     val to_buffer : Buffer.t -> t -> unit
 
-    (** [to_address_buffer buf prefix addr] writes string representation of the
-        network address representing [addr] in [prefix] to the buffer [buf]. *)
-    val to_address_buffer : Buffer.t -> t -> addr -> unit
-
     (** [of_netmask_exn ~netmask ~address] is the subnet prefix of [address]
         with netmask [netmask]. *)
     val of_netmask_exn : netmask:addr -> address:addr -> t
 
-    (** [of_netmask ~netmask ~address] is the subnet prefix of [address] with
+    (** [of_netmask ~netmask ~address] is the cidr of [address] with
         netmask [netmask]. *)
     val of_netmask : netmask:addr -> address:addr ->
       (t, [> `Msg of string ]) result
@@ -569,6 +546,9 @@ module V6 : sig
     (** [netmask subnet] is the netmask for [subnet]. *)
     val netmask : t -> addr
 
+    (** [address cidr] is the address for [cidr]. *)
+    val address : t -> addr
+
     (** [bits subnet] is the bit size of the [subnet] prefix. *)
     val bits : t -> int