refactor

- avoid all unsafe by avoiding the C-api completely

Author: Byron

Cargo.lock

@@ -108,7 +108,7 @@ bytesize = { version = "2.3.1", optional = true }
 bytes = { version = "1.0.0", optional = true }
 
 # zlib module
-zlib-rs = { version = "0.5.2", optional = true }
+zlib-rs = { version = "0.5.4", optional = true }
 thiserror = { version = "2.0.17", optional = true }
 
 # Note: once_cell is kept for OnceCell type because std::sync::OnceLock::get_or_try_init() is not yet stable.

gix-features/Cargo.toml

@@ -1,12 +1,7 @@
-use std::ffi::c_int;
+use zlib_rs::InflateError;
 
 /// A type to hold all state needed for decompressing a ZLIB encoded stream.
-pub struct Decompress(zlib_rs::c_api::z_stream);
-
-// SAFETY: The z_stream contains raw pointers but they are only used through safe zlib-rs APIs
-// and the state is fully owned by this struct, making it safe to send across threads.
-unsafe impl Send for Decompress {}
-unsafe impl Sync for Decompress {}
+pub struct Decompress(zlib_rs::Inflate);
 
 impl Default for Decompress {
     fn default() -> Self {
@@ -17,28 +12,23 @@ impl Default for Decompress {
 impl Decompress {
     /// The amount of bytes consumed from the input so far.
     pub fn total_in(&self) -> u64 {
-        self.0.total_in as _
+        self.0.total_in()
     }
 
     /// The amount of decompressed bytes that have been written to the output thus far.
     pub fn total_out(&self) -> u64 {
-        self.0.total_out as _
+        self.0.total_out()
     }
 
     /// Create a new instance. Note that it allocates in various ways and thus should be re-used.
     pub fn new() -> Self {
-        let mut stream = zlib_rs::c_api::z_stream::default();
-        let config = zlib_rs::inflate::InflateConfig::default();
-        zlib_rs::inflate::init(&mut stream, config);
-        Self(stream)
+        let inner = zlib_rs::Inflate::new(true, zlib_rs::MAX_WBITS as u8);
+        Self(inner)
     }
 
     /// Reset the state to allow handling a new stream.
     pub fn reset(&mut self) {
-        // SAFETY: Converting from z_stream to InflateStream is safe here as we maintain ownership
-        if let Some(stream) = unsafe { zlib_rs::inflate::InflateStream::from_stream_mut(&mut self.0) } {
-            zlib_rs::inflate::reset(stream);
-        }
+        self.0.reset(true);
     }
 
     /// Decompress `input` and write all decompressed bytes into `output`, with `flush` defining some details about this.
@@ -48,43 +38,17 @@ impl Decompress {
         output: &mut [u8],
         flush: FlushDecompress,
     ) -> Result<Status, DecompressError> {
-        self.0.avail_in = input.len() as _;
-        self.0.avail_out = output.len() as _;
-
-        self.0.next_in = input.as_ptr();
-        self.0.next_out = output.as_mut_ptr();
-
-        // SAFETY: We're converting from the C-compatible z_stream to the typed InflateStream.
-        // This is safe because we initialized the stream with `inflate::init` and maintain ownership.
-        let stream = unsafe { zlib_rs::inflate::InflateStream::from_stream_mut(&mut self.0) }
-            .ok_or(DecompressError::StreamError)?;
-        
         let inflate_flush = match flush {
             FlushDecompress::None => zlib_rs::InflateFlush::NoFlush,
             FlushDecompress::Sync => zlib_rs::InflateFlush::SyncFlush,
             FlushDecompress::Finish => zlib_rs::InflateFlush::Finish,
         };
-        
-        // SAFETY: The zlib_rs::inflate::inflate function is marked unsafe because it operates on
-        // raw pointers internally. However, we've properly set up the stream with valid input/output
-        // buffers and the actual decompression is done in safe Rust code within zlib-rs.
-        match unsafe { zlib_rs::inflate::inflate(stream, inflate_flush) } {
-            zlib_rs::ReturnCode::Ok => Ok(Status::Ok),
-            zlib_rs::ReturnCode::BufError => Ok(Status::BufError),
-            zlib_rs::ReturnCode::StreamEnd => Ok(Status::StreamEnd),
-            zlib_rs::ReturnCode::StreamError => Err(DecompressError::StreamError),
-            zlib_rs::ReturnCode::DataError => Err(DecompressError::DataError),
-            zlib_rs::ReturnCode::MemError => Err(DecompressError::InsufficientMemory),
-            err => Err(DecompressError::Unknown { err: err as c_int }),
-        }
-    }
-}
 
-impl Drop for Decompress {
-    fn drop(&mut self) {
-        // SAFETY: Converting from z_stream to InflateStream is safe here as we maintain ownership
-        if let Some(stream) = unsafe { zlib_rs::inflate::InflateStream::from_stream_mut(&mut self.0) } {
-            zlib_rs::inflate::end(stream);
+        let status = self.0.decompress(input, output, inflate_flush)?;
+        match status {
+            zlib_rs::Status::Ok => Ok(Status::Ok),
+            zlib_rs::Status::BufError => Ok(Status::BufError),
+            zlib_rs::Status::StreamEnd => Ok(Status::StreamEnd),
         }
     }
 }
@@ -99,8 +63,19 @@ pub enum DecompressError {
     InsufficientMemory,
     #[error("Invalid input data")]
     DataError,
-    #[error("An unknown error occurred: {err}")]
-    Unknown { err: c_int },
+    #[error("Decompressing this input requires a dictionary")]
+    NeedDict,
+}
+
+impl From<zlib_rs::InflateError> for DecompressError {
+    fn from(value: InflateError) -> Self {
+        match value {
+            InflateError::NeedDict { .. } => DecompressError::NeedDict,
+            InflateError::StreamError => DecompressError::StreamError,
+            InflateError::DataError => DecompressError::DataError,
+            InflateError::MemError => DecompressError::InsufficientMemory,
+        }
+    }
 }
 
 /// The status returned by [`Decompress::decompress()`].

gix-features/src/zlib/mod.rs

@@ -1,5 +1,5 @@
 use crate::zlib::Status;
-use std::ffi::c_int;
+use zlib_rs::DeflateError;
 
 const BUF_SIZE: usize = 4096 * 8;
 
@@ -26,12 +26,7 @@ where
 }
 
 /// Hold all state needed for compressing data.
-pub struct Compress(zlib_rs::c_api::z_stream);
-
-// SAFETY: The z_stream contains raw pointers but they are only used through safe zlib-rs APIs
-// and the state is fully owned by this struct, making it safe to send across threads.
-unsafe impl Send for Compress {}
-unsafe impl Sync for Compress {}
+pub struct Compress(zlib_rs::Deflate);
 
 impl Default for Compress {
     fn default() -> Self {
@@ -42,72 +37,41 @@ impl Default for Compress {
 impl Compress {
     /// The number of bytes that were read from the input.
     pub fn total_in(&self) -> u64 {
-        self.0.total_in as _
+        self.0.total_in()
     }
 
     /// The number of compressed bytes that were written to the output.
     pub fn total_out(&self) -> u64 {
-        self.0.total_out as _
+        self.0.total_out()
     }
 
     /// Create a new instance - this allocates so should be done with care.
     pub fn new() -> Self {
-        let mut stream = zlib_rs::c_api::z_stream::default();
-        let config = zlib_rs::deflate::DeflateConfig {
-            level: 1, // Z_BEST_SPEED
-            ..Default::default()
-        };
-        zlib_rs::deflate::init(&mut stream, config);
-        Self(stream)
+        let inner = zlib_rs::Deflate::new(zlib_rs::c_api::Z_BEST_SPEED, true, zlib_rs::MAX_WBITS as u8);
+        Self(inner)
     }
 
     /// Prepare the instance for a new stream.
     pub fn reset(&mut self) {
-        // SAFETY: Converting from z_stream to DeflateStream is safe here as we maintain ownership
-        if let Some(stream) = unsafe { zlib_rs::deflate::DeflateStream::from_stream_mut(&mut self.0) } {
-            zlib_rs::deflate::reset(stream);
-        }
+        self.0.reset();
     }
 
     /// Compress `input` and write compressed bytes to `output`, with `flush` controlling additional characteristics.
     pub fn compress(&mut self, input: &[u8], output: &mut [u8], flush: FlushCompress) -> Result<Status, CompressError> {
-        self.0.avail_in = input.len() as _;
-        self.0.avail_out = output.len() as _;
-
-        self.0.next_in = input.as_ptr();
-        self.0.next_out = output.as_mut_ptr();
-
-        // SAFETY: We're converting from the C-compatible z_stream to the typed DeflateStream.
-        // This is safe because we initialized the stream with `deflate::init` and maintain ownership.
-        let stream = unsafe { zlib_rs::deflate::DeflateStream::from_stream_mut(&mut self.0) }
-            .ok_or(CompressError::StreamError)?;
-        
-        let deflate_flush = match flush {
+        let flush = match flush {
             FlushCompress::None => zlib_rs::DeflateFlush::NoFlush,
             FlushCompress::Partial => zlib_rs::DeflateFlush::PartialFlush,
             FlushCompress::Sync => zlib_rs::DeflateFlush::SyncFlush,
             FlushCompress::Full => zlib_rs::DeflateFlush::FullFlush,
             FlushCompress::Finish => zlib_rs::DeflateFlush::Finish,
         };
-        
+        let status = self.0.compress(input, output, flush)?;
         // Note: zlib_rs::deflate::deflate is a safe function (unlike inflate which is unsafe)
         // because deflate doesn't have the same safety concerns as inflate regarding output buffer handling
-        match zlib_rs::deflate::deflate(stream, deflate_flush) {
-            zlib_rs::ReturnCode::Ok => Ok(Status::Ok),
-            zlib_rs::ReturnCode::BufError => Ok(Status::BufError),
-            zlib_rs::ReturnCode::StreamEnd => Ok(Status::StreamEnd),
-            zlib_rs::ReturnCode::StreamError => Err(CompressError::StreamError),
-            zlib_rs::ReturnCode::MemError => Err(CompressError::InsufficientMemory),
-            err => Err(CompressError::Unknown { err: err as c_int }),
-        }
-    }
-}
-
-impl Drop for Compress {
-    fn drop(&mut self) {
-        // SAFETY: Converting from z_stream to DeflateStream is safe here as we maintain ownership
-        if let Some(stream) = unsafe { zlib_rs::deflate::DeflateStream::from_stream_mut(&mut self.0) } {
-            let _ = zlib_rs::deflate::end(stream);
+        match status {
+            zlib_rs::Status::Ok => Ok(Status::Ok),
+            zlib_rs::Status::BufError => Ok(Status::BufError),
+            zlib_rs::Status::StreamEnd => Ok(Status::StreamEnd),
         }
     }
 }
@@ -119,10 +83,20 @@ impl Drop for Compress {
 pub enum CompressError {
     #[error("stream error")]
     StreamError,
+    #[error("The input is not a valid deflate stream.")]
+    DataError,
     #[error("Not enough memory")]
     InsufficientMemory,
-    #[error("An unknown error occurred: {err}")]
-    Unknown { err: c_int },
+}
+
+impl From<zlib_rs::DeflateError> for CompressError {
+    fn from(value: zlib_rs::DeflateError) -> Self {
+        match value {
+            DeflateError::StreamError => CompressError::StreamError,
+            DeflateError::DataError => CompressError::DataError,
+            DeflateError::MemError => CompressError::InsufficientMemory,
+        }
+    }
 }
 
 /// Values which indicate the form of flushing to be used when compressing