fix!: use Range<u64> instead of RangeInclusive<u64> and support zero-sized files

Change ContentRange::range() from RangeInclusive<u64> to Range<u64>
(exclusive end) and file_range() from NonZero<u64> to plain u64.

RangeInclusive cannot represent an empty range, which forced the API to
require NonZero<u64> and reject zero-sized files entirely. With
Range<u64>, a zero-sized file naturally produces 0..0 when no Range
header is present. Any Range header against a zero-sized file correctly
returns 416.

The HTTP header types (OrderedRange, HttpContentRange) keep their
inclusive semantics since they model the wire format.

Author: paolobarbolini

src/headers/tests.rs

@@ -400,8 +400,6 @@ mod content_range_parsing_errors {
 
 #[cfg(test)]
 mod file_range {
-    use std::num::NonZeroU64;
-
     use crate::{
         file_range,
         headers::{
@@ -411,147 +409,176 @@ mod file_range {
         },
     };
 
-    fn size(n: u64) -> NonZeroU64 {
-        NonZeroU64::new(n).unwrap()
-    }
-
     #[test]
     fn no_range_returns_full_file() {
-        let result = file_range(size(10), None).unwrap();
+        let result = file_range(10, None).unwrap();
         assert!(result.header().is_none());
-        assert_eq!(result.range(), &(0..=9));
+        assert_eq!(result.range(), &(0..10));
     }
 
     #[test]
     fn starting_point_zero() {
-        let result = file_range(size(10), Some(HttpRange::StartingPoint(0))).unwrap();
+        let result = file_range(10, Some(HttpRange::StartingPoint(0))).unwrap();
         assert!(result.header().is_some());
-        assert_eq!(result.range(), &(0..=9));
+        assert_eq!(result.range(), &(0..10));
     }
 
     #[test]
     fn starting_point_middle() {
-        let result = file_range(size(10), Some(HttpRange::StartingPoint(5))).unwrap();
-        assert_eq!(result.range(), &(5..=9));
+        let result = file_range(10, Some(HttpRange::StartingPoint(5))).unwrap();
+        assert_eq!(result.range(), &(5..10));
     }
 
     #[test]
     fn starting_point_last_byte() {
-        let result = file_range(size(10), Some(HttpRange::StartingPoint(9))).unwrap();
-        assert_eq!(result.range(), &(9..=9));
+        let result = file_range(10, Some(HttpRange::StartingPoint(9))).unwrap();
+        assert_eq!(result.range(), &(9..10));
     }
 
     #[test]
     fn starting_point_at_size() {
-        let result = file_range(size(10), Some(HttpRange::StartingPoint(10)));
+        let result = file_range(10, Some(HttpRange::StartingPoint(10)));
         assert!(result.is_err());
     }
 
     #[test]
     fn starting_point_beyond_size() {
-        let result = file_range(size(10), Some(HttpRange::StartingPoint(20)));
+        let result = file_range(10, Some(HttpRange::StartingPoint(20)));
         assert!(result.is_err());
     }
 
     #[test]
     fn range_single_byte() {
         let range = HttpRange::Range(OrderedRange::new(0..=0).unwrap());
-        let result = file_range(size(10), Some(range)).unwrap();
-        assert_eq!(result.range(), &(0..=0));
+        let result = file_range(10, Some(range)).unwrap();
+        assert_eq!(result.range(), &(0..1));
     }
 
     #[test]
     fn range_full_file() {
         let range = HttpRange::Range(OrderedRange::new(0..=9).unwrap());
-        let result = file_range(size(10), Some(range)).unwrap();
-        assert_eq!(result.range(), &(0..=9));
+        let result = file_range(10, Some(range)).unwrap();
+        assert_eq!(result.range(), &(0..10));
     }
 
     #[test]
     fn range_end_at_size_is_clamped() {
         let range = HttpRange::Range(OrderedRange::new(0..=10).unwrap());
-        let result = file_range(size(10), Some(range)).unwrap();
-        assert_eq!(result.range(), &(0..=9));
+        let result = file_range(10, Some(range)).unwrap();
+        assert_eq!(result.range(), &(0..10));
     }
 
     #[test]
     fn range_beyond_size_is_clamped() {
         let range = HttpRange::Range(OrderedRange::new(0..=50).unwrap());
-        let result = file_range(size(10), Some(range)).unwrap();
-        assert_eq!(result.range(), &(0..=9));
+        let result = file_range(10, Some(range)).unwrap();
+        assert_eq!(result.range(), &(0..10));
+    }
+
+    #[test]
+    fn range_end_at_u64_max_is_clamped() {
+        let range = HttpRange::Range(OrderedRange::new(0..=u64::MAX).unwrap());
+        let result = file_range(10, Some(range)).unwrap();
+        assert_eq!(result.range(), &(0..10));
     }
 
     #[test]
     fn range_start_at_size_is_unsatisfiable() {
         let range = HttpRange::Range(OrderedRange::new(10..=20).unwrap());
-        let result = file_range(size(10), Some(range));
+        let result = file_range(10, Some(range));
         assert!(result.is_err());
     }
 
     #[test]
     fn range_start_beyond_size_is_unsatisfiable() {
         let range = HttpRange::Range(OrderedRange::new(50..=100).unwrap());
-        let result = file_range(size(10), Some(range));
+        let result = file_range(10, Some(range));
         assert!(result.is_err());
     }
 
     #[test]
     fn suffix_entire_file() {
-        let result = file_range(size(10), Some(HttpRange::Suffix(10))).unwrap();
-        assert_eq!(result.range(), &(0..=9));
+        let result = file_range(10, Some(HttpRange::Suffix(10))).unwrap();
+        assert_eq!(result.range(), &(0..10));
     }
 
     #[test]
     fn suffix_last_byte() {
-        let result = file_range(size(10), Some(HttpRange::Suffix(1))).unwrap();
-        assert_eq!(result.range(), &(9..=9));
+        let result = file_range(10, Some(HttpRange::Suffix(1))).unwrap();
+        assert_eq!(result.range(), &(9..10));
     }
 
     #[test]
     fn suffix_middle() {
-        let result = file_range(size(10), Some(HttpRange::Suffix(5))).unwrap();
-        assert_eq!(result.range(), &(5..=9));
+        let result = file_range(10, Some(HttpRange::Suffix(5))).unwrap();
+        assert_eq!(result.range(), &(5..10));
     }
 
     #[test]
     fn suffix_zero_is_unsatisfiable() {
-        let result = file_range(size(10), Some(HttpRange::Suffix(0)));
+        let result = file_range(10, Some(HttpRange::Suffix(0)));
         assert!(result.is_err());
     }
 
     #[test]
     fn suffix_exceeds_size_is_clamped() {
-        let result = file_range(size(10), Some(HttpRange::Suffix(11))).unwrap();
-        assert_eq!(result.range(), &(0..=9));
+        let result = file_range(10, Some(HttpRange::Suffix(11))).unwrap();
+        assert_eq!(result.range(), &(0..10));
     }
 
     #[test]
     fn size_one_no_range() {
-        let result = file_range(size(1), None).unwrap();
-        assert_eq!(result.range(), &(0..=0));
+        let result = file_range(1, None).unwrap();
+        assert_eq!(result.range(), &(0..1));
     }
 
     #[test]
     fn size_one_starting_point_zero() {
-        let result = file_range(size(1), Some(HttpRange::StartingPoint(0))).unwrap();
-        assert_eq!(result.range(), &(0..=0));
+        let result = file_range(1, Some(HttpRange::StartingPoint(0))).unwrap();
+        assert_eq!(result.range(), &(0..1));
     }
 
     #[test]
     fn size_one_suffix_one() {
-        let result = file_range(size(1), Some(HttpRange::Suffix(1))).unwrap();
-        assert_eq!(result.range(), &(0..=0));
+        let result = file_range(1, Some(HttpRange::Suffix(1))).unwrap();
+        assert_eq!(result.range(), &(0..1));
     }
 
     #[test]
     fn content_range_header_present_for_range_request() {
-        let result = file_range(size(10), Some(HttpRange::StartingPoint(0))).unwrap();
+        let result = file_range(10, Some(HttpRange::StartingPoint(0))).unwrap();
         let header = result.header().unwrap();
         assert_eq!(
             header,
             HttpContentRange::Bound(Bound::new(0..=9, Some(10)).unwrap())
         );
     }
+
+    #[test]
+    fn size_zero_no_range() {
+        let result = file_range(0, None).unwrap();
+        assert!(result.header().is_none());
+        assert_eq!(result.range(), &(0..0));
+    }
+
+    #[test]
+    fn size_zero_starting_point_is_unsatisfiable() {
+        let result = file_range(0, Some(HttpRange::StartingPoint(0)));
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn size_zero_range_is_unsatisfiable() {
+        let range = HttpRange::Range(OrderedRange::new(0..=0).unwrap());
+        let result = file_range(0, Some(range));
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn size_zero_suffix_is_unsatisfiable() {
+        let result = file_range(0, Some(HttpRange::Suffix(1)));
+        assert!(result.is_err());
+    }
 }
 
 #[cfg(test)]
@@ -586,10 +613,11 @@ mod serve_file {
     }
 
     #[test]
-    fn empty_body_is_unsatisfiable() {
+    fn empty_body_returns_empty_response() {
         let body = Bytes::new();
-        let result = serve_file_with_http_range(body, None);
-        assert!(result.is_err());
+        let result = serve_file_with_http_range(body, None).unwrap();
+        assert!(result.body().is_empty());
+        assert!(result.header().is_none());
     }
 
     #[test]

src/lib.rs

@@ -1,9 +1,6 @@
 #![cfg_attr(docsrs, feature(doc_cfg))]
 
-use std::{
-    num::{NonZero, NonZeroU64},
-    ops::RangeInclusive,
-};
+use std::ops::Range;
 
 use bytes::Bytes;
 
@@ -22,17 +19,14 @@ pub fn serve_file_with_http_range(
     http_range: Option<HttpRange>,
 ) -> Result<BodyRange<Bytes>, UnsatisfiableRange> {
     let size = u64::try_from(body.len()).expect("we do not support 128bit usize");
-    let size = NonZeroU64::try_from(size).map_err(|_| {
-        UnsatisfiableRange(HttpContentRange::Unsatisfiable(Unsatisfiable::new(size)))
-    })?;
 
     let content_range = file_range(size, http_range)?;
 
-    let start = usize::try_from(*content_range.range.start()).expect("u64 doesn't fit usize");
-    let end = usize::try_from(*content_range.range.end()).expect("u64 doesn't fit usize");
+    let start = usize::try_from(content_range.range.start).expect("u64 doesn't fit usize");
+    let end = usize::try_from(content_range.range.end).expect("u64 doesn't fit usize");
 
     Ok(BodyRange {
-        body: body.slice(start..=end),
+        body: body.slice(start..end),
         header: content_range.header,
     })
 }
@@ -41,31 +35,30 @@ pub fn serve_file_with_http_range(
 ///
 /// [`HttpRange`]: crate::headers::range::HttpRange
 pub fn file_range(
-    size: NonZero<u64>,
+    size: u64,
     http_range: Option<HttpRange>,
 ) -> Result<ContentRange, UnsatisfiableRange> {
-    let size = size.get();
-
     let Some(http_range) = http_range else {
         return Ok(ContentRange {
             header: None,
-            range: 0..=size - 1,
+            range: 0..size,
         });
     };
 
     let range = match http_range {
-        HttpRange::StartingPoint(start) if start < size => start..=size - 1,
+        HttpRange::StartingPoint(start) if start < size => start..size,
         HttpRange::Range(range) if range.start() < size => {
-            range.start()..=range.end().min(size - 1)
+            range.start()..range.end().saturating_add(1).min(size)
         }
-        HttpRange::Suffix(suffix) if suffix > 0 => size.saturating_sub(suffix)..=size - 1,
+        HttpRange::Suffix(suffix) if suffix > 0 && size > 0 => size.saturating_sub(suffix)..size,
         _ => {
             let content_range = HttpContentRange::Unsatisfiable(Unsatisfiable::new(size));
             return Err(UnsatisfiableRange(content_range));
         }
     };
 
-    let content_range = HttpContentRange::Bound(Bound::new(range.clone(), Some(size)).unwrap());
+    let content_range =
+        HttpContentRange::Bound(Bound::new(range.start..=range.end - 1, Some(size)).unwrap());
 
     Ok(ContentRange {
         header: Some(content_range),
@@ -107,7 +100,7 @@ impl<T> BodyRange<T> {
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct ContentRange {
     header: Option<HttpContentRange>,
-    range: RangeInclusive<u64>,
+    range: Range<u64>,
 }
 
 impl ContentRange {
@@ -117,8 +110,8 @@ impl ContentRange {
         self.header
     }
 
-    /// Returns a [`RangeInclusive`] of `u64` useful to manually slice the response body.
-    pub fn range(&self) -> &RangeInclusive<u64> {
+    /// Returns a [`Range`] of `u64` useful to manually slice the response body.
+    pub fn range(&self) -> &Range<u64> {
         &self.range
     }
 }