Skip to content

Commit a75c852

Browse files
CopilotByron
andcommitted
feat!: Replace libz-rs-sys with zlib-rs for safe zlib usage
Co-authored-by: Byron <63622+Byron@users.noreply.github.com>
1 parent d207075 commit a75c852

File tree

4 files changed

+90
-59
lines changed

4 files changed

+90
-59
lines changed

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gix-features/Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ crc32 = ["dep:crc32fast"]
5757

5858
## Enable the usage of zlib-related utilities to compress or decompress data.
5959
## This enables and uses the high-performance `zlib-rs` backend.
60-
zlib = ["dep:libz-rs-sys", "dep:thiserror"]
60+
zlib = ["dep:zlib-rs", "dep:thiserror"]
6161

6262
#! ### Other
6363

@@ -108,7 +108,7 @@ bytesize = { version = "2.3.1", optional = true }
108108
bytes = { version = "1.0.0", optional = true }
109109

110110
# zlib module
111-
libz-rs-sys = { version = "0.5.2", optional = true }
111+
zlib-rs = { version = "0.5.2", optional = true }
112112
thiserror = { version = "2.0.17", optional = true }
113113

114114
# Note: once_cell is kept for OnceCell type because std::sync::OnceLock::get_or_try_init() is not yet stable.

gix-features/src/zlib/mod.rs

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
use std::ffi::c_int;
22

33
/// A type to hold all state needed for decompressing a ZLIB encoded stream.
4-
pub struct Decompress(libz_rs_sys::z_stream);
4+
pub struct Decompress(zlib_rs::c_api::z_stream);
55

6-
unsafe impl Sync for Decompress {}
6+
// SAFETY: The z_stream contains raw pointers but they are only used through safe zlib-rs APIs
7+
// and the state is fully owned by this struct, making it safe to send across threads.
78
unsafe impl Send for Decompress {}
9+
unsafe impl Sync for Decompress {}
810

911
impl Default for Decompress {
1012
fn default() -> Self {
@@ -25,22 +27,18 @@ impl Decompress {
2527

2628
/// Create a new instance. Note that it allocates in various ways and thus should be re-used.
2729
pub fn new() -> Self {
28-
let mut this = libz_rs_sys::z_stream::default();
29-
30-
unsafe {
31-
libz_rs_sys::inflateInit_(
32-
&mut this,
33-
libz_rs_sys::zlibVersion(),
34-
core::mem::size_of::<libz_rs_sys::z_stream>() as core::ffi::c_int,
35-
);
36-
}
37-
38-
Self(this)
30+
let mut stream = zlib_rs::c_api::z_stream::default();
31+
let config = zlib_rs::inflate::InflateConfig::default();
32+
zlib_rs::inflate::init(&mut stream, config);
33+
Self(stream)
3934
}
4035

4136
/// Reset the state to allow handling a new stream.
4237
pub fn reset(&mut self) {
43-
unsafe { libz_rs_sys::inflateReset(&mut self.0) };
38+
// SAFETY: Converting from z_stream to InflateStream is safe here as we maintain ownership
39+
if let Some(stream) = unsafe { zlib_rs::inflate::InflateStream::from_stream_mut(&mut self.0) } {
40+
zlib_rs::inflate::reset(stream);
41+
}
4442
}
4543

4644
/// Decompress `input` and write all decompressed bytes into `output`, with `flush` defining some details about this.
@@ -56,22 +54,38 @@ impl Decompress {
5654
self.0.next_in = input.as_ptr();
5755
self.0.next_out = output.as_mut_ptr();
5856

59-
match unsafe { libz_rs_sys::inflate(&mut self.0, flush as _) } {
60-
libz_rs_sys::Z_OK => Ok(Status::Ok),
61-
libz_rs_sys::Z_BUF_ERROR => Ok(Status::BufError),
62-
libz_rs_sys::Z_STREAM_END => Ok(Status::StreamEnd),
63-
64-
libz_rs_sys::Z_STREAM_ERROR => Err(DecompressError::StreamError),
65-
libz_rs_sys::Z_DATA_ERROR => Err(DecompressError::DataError),
66-
libz_rs_sys::Z_MEM_ERROR => Err(DecompressError::InsufficientMemory),
67-
err => Err(DecompressError::Unknown { err }),
57+
// SAFETY: We're converting from the C-compatible z_stream to the typed InflateStream.
58+
// This is safe because we initialized the stream with `inflate::init` and maintain ownership.
59+
let stream = unsafe { zlib_rs::inflate::InflateStream::from_stream_mut(&mut self.0) }
60+
.ok_or(DecompressError::StreamError)?;
61+
62+
let inflate_flush = match flush {
63+
FlushDecompress::None => zlib_rs::InflateFlush::NoFlush,
64+
FlushDecompress::Sync => zlib_rs::InflateFlush::SyncFlush,
65+
FlushDecompress::Finish => zlib_rs::InflateFlush::Finish,
66+
};
67+
68+
// SAFETY: The zlib_rs::inflate::inflate function is marked unsafe because it operates on
69+
// raw pointers internally. However, we've properly set up the stream with valid input/output
70+
// buffers and the actual decompression is done in safe Rust code within zlib-rs.
71+
match unsafe { zlib_rs::inflate::inflate(stream, inflate_flush) } {
72+
zlib_rs::ReturnCode::Ok => Ok(Status::Ok),
73+
zlib_rs::ReturnCode::BufError => Ok(Status::BufError),
74+
zlib_rs::ReturnCode::StreamEnd => Ok(Status::StreamEnd),
75+
zlib_rs::ReturnCode::StreamError => Err(DecompressError::StreamError),
76+
zlib_rs::ReturnCode::DataError => Err(DecompressError::DataError),
77+
zlib_rs::ReturnCode::MemError => Err(DecompressError::InsufficientMemory),
78+
err => Err(DecompressError::Unknown { err: err as c_int }),
6879
}
6980
}
7081
}
7182

7283
impl Drop for Decompress {
7384
fn drop(&mut self) {
74-
unsafe { libz_rs_sys::inflateEnd(&mut self.0) };
85+
// SAFETY: Converting from z_stream to InflateStream is safe here as we maintain ownership
86+
if let Some(stream) = unsafe { zlib_rs::inflate::InflateStream::from_stream_mut(&mut self.0) } {
87+
zlib_rs::inflate::end(stream);
88+
}
7589
}
7690
}
7791

@@ -110,7 +124,7 @@ pub enum FlushDecompress {
110124
/// A typical parameter for passing to compression/decompression functions,
111125
/// this indicates that the underlying stream to decide how much data to
112126
/// accumulate before producing output in order to maximize compression.
113-
None = libz_rs_sys::Z_NO_FLUSH as isize,
127+
None = 0,
114128

115129
/// All pending output is flushed to the output buffer and the output is
116130
/// aligned on a byte boundary so that the decompressor can get all input
@@ -119,13 +133,13 @@ pub enum FlushDecompress {
119133
/// Flushing may degrade compression for some compression algorithms and so
120134
/// it should only be used when necessary. This will complete the current
121135
/// deflate block and follow it with an empty stored block.
122-
Sync = libz_rs_sys::Z_SYNC_FLUSH as isize,
136+
Sync = 2,
123137

124138
/// Pending input is processed and pending output is flushed.
125139
///
126140
/// The return value may indicate that the stream is not yet done and more
127141
/// data has yet to be processed.
128-
Finish = libz_rs_sys::Z_FINISH as isize,
142+
Finish = 4,
129143
}
130144

131145
/// non-streaming interfaces for decompression

gix-features/src/zlib/stream/deflate/mod.rs

Lines changed: 46 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@ where
2626
}
2727

2828
/// Hold all state needed for compressing data.
29-
pub struct Compress(libz_rs_sys::z_stream);
29+
pub struct Compress(zlib_rs::c_api::z_stream);
3030

31-
unsafe impl Sync for Compress {}
31+
// SAFETY: The z_stream contains raw pointers but they are only used through safe zlib-rs APIs
32+
// and the state is fully owned by this struct, making it safe to send across threads.
3233
unsafe impl Send for Compress {}
34+
unsafe impl Sync for Compress {}
3335

3436
impl Default for Compress {
3537
fn default() -> Self {
@@ -50,23 +52,21 @@ impl Compress {
5052

5153
/// Create a new instance - this allocates so should be done with care.
5254
pub fn new() -> Self {
53-
let mut this = libz_rs_sys::z_stream::default();
54-
55-
unsafe {
56-
libz_rs_sys::deflateInit_(
57-
&mut this,
58-
libz_rs_sys::Z_BEST_SPEED,
59-
libz_rs_sys::zlibVersion(),
60-
core::mem::size_of::<libz_rs_sys::z_stream>() as core::ffi::c_int,
61-
);
62-
}
63-
64-
Self(this)
55+
let mut stream = zlib_rs::c_api::z_stream::default();
56+
let config = zlib_rs::deflate::DeflateConfig {
57+
level: 1, // Z_BEST_SPEED
58+
..Default::default()
59+
};
60+
zlib_rs::deflate::init(&mut stream, config);
61+
Self(stream)
6562
}
6663

6764
/// Prepare the instance for a new stream.
6865
pub fn reset(&mut self) {
69-
unsafe { libz_rs_sys::deflateReset(&mut self.0) };
66+
// SAFETY: Converting from z_stream to DeflateStream is safe here as we maintain ownership
67+
if let Some(stream) = unsafe { zlib_rs::deflate::DeflateStream::from_stream_mut(&mut self.0) } {
68+
zlib_rs::deflate::reset(stream);
69+
}
7070
}
7171

7272
/// Compress `input` and write compressed bytes to `output`, with `flush` controlling additional characteristics.
@@ -77,21 +77,38 @@ impl Compress {
7777
self.0.next_in = input.as_ptr();
7878
self.0.next_out = output.as_mut_ptr();
7979

80-
match unsafe { libz_rs_sys::deflate(&mut self.0, flush as _) } {
81-
libz_rs_sys::Z_OK => Ok(Status::Ok),
82-
libz_rs_sys::Z_BUF_ERROR => Ok(Status::BufError),
83-
libz_rs_sys::Z_STREAM_END => Ok(Status::StreamEnd),
84-
85-
libz_rs_sys::Z_STREAM_ERROR => Err(CompressError::StreamError),
86-
libz_rs_sys::Z_MEM_ERROR => Err(CompressError::InsufficientMemory),
87-
err => Err(CompressError::Unknown { err }),
80+
// SAFETY: We're converting from the C-compatible z_stream to the typed DeflateStream.
81+
// This is safe because we initialized the stream with `deflate::init` and maintain ownership.
82+
let stream = unsafe { zlib_rs::deflate::DeflateStream::from_stream_mut(&mut self.0) }
83+
.ok_or(CompressError::StreamError)?;
84+
85+
let deflate_flush = match flush {
86+
FlushCompress::None => zlib_rs::DeflateFlush::NoFlush,
87+
FlushCompress::Partial => zlib_rs::DeflateFlush::PartialFlush,
88+
FlushCompress::Sync => zlib_rs::DeflateFlush::SyncFlush,
89+
FlushCompress::Full => zlib_rs::DeflateFlush::FullFlush,
90+
FlushCompress::Finish => zlib_rs::DeflateFlush::Finish,
91+
};
92+
93+
// Note: zlib_rs::deflate::deflate is a safe function (unlike inflate which is unsafe)
94+
// because deflate doesn't have the same safety concerns as inflate regarding output buffer handling
95+
match zlib_rs::deflate::deflate(stream, deflate_flush) {
96+
zlib_rs::ReturnCode::Ok => Ok(Status::Ok),
97+
zlib_rs::ReturnCode::BufError => Ok(Status::BufError),
98+
zlib_rs::ReturnCode::StreamEnd => Ok(Status::StreamEnd),
99+
zlib_rs::ReturnCode::StreamError => Err(CompressError::StreamError),
100+
zlib_rs::ReturnCode::MemError => Err(CompressError::InsufficientMemory),
101+
err => Err(CompressError::Unknown { err: err as c_int }),
88102
}
89103
}
90104
}
91105

92106
impl Drop for Compress {
93107
fn drop(&mut self) {
94-
unsafe { libz_rs_sys::deflateEnd(&mut self.0) };
108+
// SAFETY: Converting from z_stream to DeflateStream is safe here as we maintain ownership
109+
if let Some(stream) = unsafe { zlib_rs::deflate::DeflateStream::from_stream_mut(&mut self.0) } {
110+
let _ = zlib_rs::deflate::end(stream);
111+
}
95112
}
96113
}
97114

@@ -117,7 +134,7 @@ pub enum FlushCompress {
117134
/// A typical parameter for passing to compression/decompression functions,
118135
/// this indicates that the underlying stream to decide how much data to
119136
/// accumulate before producing output in order to maximize compression.
120-
None = libz_rs_sys::Z_NO_FLUSH as isize,
137+
None = 0,
121138

122139
/// All pending output is flushed to the output buffer, but the output is
123140
/// not aligned to a byte boundary.
@@ -127,7 +144,7 @@ pub enum FlushCompress {
127144
/// with an empty fixed codes block that is 10 bits long, and it assures
128145
/// that enough bytes are output in order for the decompressor to finish the
129146
/// block before the empty fixed code block.
130-
Partial = libz_rs_sys::Z_PARTIAL_FLUSH as isize,
147+
Partial = 1,
131148

132149
/// All pending output is flushed to the output buffer and the output is
133150
/// aligned on a byte boundary so that the decompressor can get all input
@@ -136,20 +153,20 @@ pub enum FlushCompress {
136153
/// Flushing may degrade compression for some compression algorithms and so
137154
/// it should only be used when necessary. This will complete the current
138155
/// deflate block and follow it with an empty stored block.
139-
Sync = libz_rs_sys::Z_SYNC_FLUSH as isize,
156+
Sync = 2,
140157

141158
/// All output is flushed as with `Flush::Sync` and the compression state is
142159
/// reset so decompression can restart from this point if previous
143160
/// compressed data has been damaged or if random access is desired.
144161
///
145162
/// Using this option too often can seriously degrade compression.
146-
Full = libz_rs_sys::Z_FULL_FLUSH as isize,
163+
Full = 3,
147164

148165
/// Pending input is processed and pending output is flushed.
149166
///
150167
/// The return value may indicate that the stream is not yet done and more
151168
/// data has yet to be processed.
152-
Finish = libz_rs_sys::Z_FINISH as isize,
169+
Finish = 4,
153170
}
154171

155172
mod impls {

0 commit comments

Comments
 (0)