From 3b427bf03c9b4b5bd325a359ca8d4bbcab84f629 Mon Sep 17 00:00:00 2001 From: Mingshen Sun Date: Thu, 5 Jul 2018 16:12:30 -0700 Subject: [PATCH 1/5] Making inflate_state consistent with tdefl_compressor to work around type confusion caused by C API --- src/lib_oxide.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/lib_oxide.rs b/src/lib_oxide.rs index 18d4741c..c46740fb 100644 --- a/src/lib_oxide.rs +++ b/src/lib_oxide.rs @@ -12,6 +12,7 @@ use miniz_oxide::inflate::TINFLStatus; use miniz_oxide::inflate::core::{TINFL_LZ_DICT_SIZE, inflate_flags, DecompressorOxide}; use miniz_oxide::*; +use miniz_oxide::deflate::core::{CompressorOxide}; const MZ_DEFLATED: c_int = 8; const MZ_DEFAULT_WINDOW_BITS: c_int = 15; @@ -475,6 +476,17 @@ pub struct inflate_state { pub m_window_bits: c_int, pub m_dict: [u8; TINFL_LZ_DICT_SIZE], pub m_last_status: TINFLStatus, + // consistent with tdefl_compressor in case there is a type confusion + pub inner: Option, +} + +// There could be a type confusion problem when calling deflateEnd with inflate +// stream buffer. Making inflate_state consistent with tdefl_compressor can +// workaround this issue. +impl inflate_state { + pub fn drop_inner(&mut self) { + self.inner = None; + } } pub fn mz_inflate_init_oxide(stream_oxide: &mut StreamOxide) -> MZResult { From ed1c479f2e9a8bba3e02bafa17dddfcf1242119a Mon Sep 17 00:00:00 2001 From: Mingshen Sun Date: Tue, 24 Jul 2018 19:11:31 -0700 Subject: [PATCH 2/5] Move Option to the first field --- src/lib_oxide.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib_oxide.rs b/src/lib_oxide.rs index c46740fb..7f837db7 100644 --- a/src/lib_oxide.rs +++ b/src/lib_oxide.rs @@ -466,6 +466,8 @@ pub fn mz_deflate_reset_oxide(stream_oxide: &mut StreamOxide) #[repr(C)] #[allow(bad_style)] pub struct inflate_state { + // consistent with tdefl_compressor in case there is a type confusion + pub inner: Option, pub m_decomp: DecompressorOxide, pub m_dict_ofs: c_uint, @@ -476,8 +478,6 @@ pub struct inflate_state { pub m_window_bits: c_int, pub m_dict: [u8; TINFL_LZ_DICT_SIZE], pub m_last_status: TINFLStatus, - // consistent with tdefl_compressor in case there is a type confusion - pub inner: Option, } // There could be a type confusion problem when calling deflateEnd with inflate From de2f8031d7ceaca418d80230738cf74189fbe79e Mon Sep 17 00:00:00 2001 From: Mingshen Sun Date: Fri, 27 Jul 2018 11:15:10 +0800 Subject: [PATCH 3/5] Gracefully handle type confusion --- src/lib.rs | 6 +++--- src/lib_oxide.rs | 47 +++++++++++++++++++++++++++++++---------------- src/tdef.rs | 5 +++++ 3 files changed, 39 insertions(+), 19 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index cf6b9c76..a3c81114 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -155,7 +155,7 @@ macro_rules! oxidize { // Make sure we catch a potential panic, as // this is called from C. match catch_unwind(AssertUnwindSafe(|| { - let mut stream_oxide = StreamOxide::new(&mut *stream); + let mut stream_oxide = StreamOxide::new(&mut *stream).unwrap(); let status = $mz_func_oxide(&mut stream_oxide, $($arg_name),*); *stream = stream_oxide.into_mz_stream(); as_c_return_code(status) @@ -234,7 +234,7 @@ pub unsafe extern "C" fn mz_compress2( ..Default::default() }; - let mut stream_oxide = StreamOxide::new(&mut stream); + let mut stream_oxide = StreamOxide::new(&mut stream).unwrap(); as_c_return_code(mz_compress2_oxide(&mut stream_oxide, level, dest_len)) }, ) @@ -272,7 +272,7 @@ pub unsafe extern "C" fn mz_uncompress( ..Default::default() }; - let mut stream_oxide = StreamOxide::new(&mut stream); + let mut stream_oxide = StreamOxide::new(&mut stream).unwrap(); as_c_return_code(mz_uncompress2_oxide(&mut stream_oxide, dest_len)) }, ) diff --git a/src/lib_oxide.rs b/src/lib_oxide.rs index 7f837db7..b9387998 100644 --- a/src/lib_oxide.rs +++ b/src/lib_oxide.rs @@ -12,7 +12,6 @@ use miniz_oxide::inflate::TINFLStatus; use miniz_oxide::inflate::core::{TINFL_LZ_DICT_SIZE, inflate_flags, DecompressorOxide}; use miniz_oxide::*; -use miniz_oxide::deflate::core::{CompressorOxide}; const MZ_DEFLATED: c_int = 8; const MZ_DEFAULT_WINDOW_BITS: c_int = 15; @@ -140,14 +139,23 @@ pub unsafe extern "C" fn def_free_func(_opaque: *mut c_void, address: *mut c_voi /// Trait used for states that can be carried by BoxedState. pub trait StateType { fn drop_state(&mut self); + fn get_stream_type() -> StreamType; } impl StateType for tdefl_compressor { fn drop_state(&mut self) { self.drop_inner(); } + + fn get_stream_type() -> StreamType { + StreamType::Deflate + } } impl StateType for inflate_state { fn drop_state(&mut self) {} + + fn get_stream_type() -> StreamType { + StreamType::Inflate + } } /// Wrapper for a heap-allocated compressor/decompressor that frees the stucture on drop. @@ -222,7 +230,18 @@ pub struct StreamOxide<'io, ST: StateType> { impl<'io, ST: StateType> StreamOxide<'io, ST> { - pub unsafe fn new(stream: &mut mz_stream) -> Self { + pub unsafe fn new(stream: &mut mz_stream) -> Result { + // XXX: When calling init function, the state is null. We cannot check it here. + // if stream.state.is_null() { + // return Err(MZError::Stream); + // } + + if !stream.state.is_null() { + let stream_type_slice: StreamType = ptr::read(stream.state as *const StreamType); + if stream_type_slice != ST::get_stream_type() { + return Err(MZError::Stream); + } + } let in_slice = stream.next_in.as_ref().map(|ptr| { slice::from_raw_parts(ptr, stream.avail_in as usize) }); @@ -231,14 +250,14 @@ impl<'io, ST: StateType> StreamOxide<'io, ST> { slice::from_raw_parts_mut(ptr, stream.avail_out as usize) }); - StreamOxide { + Ok(StreamOxide { next_in: in_slice, total_in: stream.total_in, next_out: out_slice, total_out: stream.total_out, state: BoxedState::new(stream), adler: stream.adler, - } + }) } pub fn into_mz_stream(mut self) -> mz_stream { @@ -461,13 +480,17 @@ pub fn mz_deflate_reset_oxide(stream_oxide: &mut StreamOxide) Ok(MZStatus::Ok) } - +#[allow(bad_style)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +pub enum StreamType { + Inflate, + Deflate +} #[repr(C)] #[allow(bad_style)] pub struct inflate_state { - // consistent with tdefl_compressor in case there is a type confusion - pub inner: Option, + pub stream_type: StreamType, pub m_decomp: DecompressorOxide, pub m_dict_ofs: c_uint, @@ -480,15 +503,6 @@ pub struct inflate_state { pub m_last_status: TINFLStatus, } -// There could be a type confusion problem when calling deflateEnd with inflate -// stream buffer. Making inflate_state consistent with tdefl_compressor can -// workaround this issue. -impl inflate_state { - pub fn drop_inner(&mut self) { - self.inner = None; - } -} - pub fn mz_inflate_init_oxide(stream_oxide: &mut StreamOxide) -> MZResult { mz_inflate_init2_oxide(stream_oxide, MZ_DEFAULT_WINDOW_BITS) } @@ -507,6 +521,7 @@ pub fn mz_inflate_init2_oxide( stream_oxide.state.alloc_state::()?; let state = stream_oxide.state.as_mut().ok_or(MZError::Mem)?; + state.stream_type = StreamType::Inflate; state.m_decomp.init(); state.m_dict_ofs = 0; state.m_dict_avail = 0; diff --git a/src/tdef.rs b/src/tdef.rs index 33aec49b..c451c5f3 100644 --- a/src/tdef.rs +++ b/src/tdef.rs @@ -4,6 +4,7 @@ use std::panic::{catch_unwind, AssertUnwindSafe}; use miniz_oxide::deflate::core::{compress, compress_to_output, create_comp_flags_from_zip_params, CompressorOxide, TDEFLFlush, TDEFLStatus}; +use lib_oxide::StreamType; /// Compression callback function type. pub type PutBufFuncPtrNotNull = unsafe extern "C" fn( @@ -44,6 +45,7 @@ pub mod strategy { #[repr(C)] #[allow(bad_style)] pub struct tdefl_compressor { + pub stream_type: StreamType, pub inner: Option, pub callback: Option, } @@ -51,6 +53,7 @@ pub struct tdefl_compressor { impl tdefl_compressor { pub(crate) fn new(flags: u32) -> Self { tdefl_compressor { + stream_type: StreamType::Deflate, inner: Some(CompressorOxide::new(flags)), callback: None, } @@ -58,6 +61,7 @@ impl tdefl_compressor { pub(crate) fn new_with_callback(flags: u32, func: CallbackFunc) -> Self { tdefl_compressor { + stream_type: StreamType::Deflate, inner: Some(CompressorOxide::new(flags)), callback: Some(func), } @@ -178,6 +182,7 @@ pub unsafe extern "C" fn tdefl_compress_buffer( pub unsafe extern "C" fn tdefl_allocate() -> *mut tdefl_compressor { Box::into_raw(Box::::new( tdefl_compressor { + stream_type: StreamType::Deflate, inner: None, callback: None, } From 585a287ed308a142c84025d42577666f7e21e9ba Mon Sep 17 00:00:00 2001 From: Mingshen Sun Date: Fri, 27 Jul 2018 15:22:41 +0800 Subject: [PATCH 4/5] Remove comment on checking null --- src/lib_oxide.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/lib_oxide.rs b/src/lib_oxide.rs index b9387998..6c59548f 100644 --- a/src/lib_oxide.rs +++ b/src/lib_oxide.rs @@ -231,11 +231,6 @@ pub struct StreamOxide<'io, ST: StateType> { impl<'io, ST: StateType> StreamOxide<'io, ST> { pub unsafe fn new(stream: &mut mz_stream) -> Result { - // XXX: When calling init function, the state is null. We cannot check it here. - // if stream.state.is_null() { - // return Err(MZError::Stream); - // } - if !stream.state.is_null() { let stream_type_slice: StreamType = ptr::read(stream.state as *const StreamType); if stream_type_slice != ST::get_stream_type() { From de137d668579283946641d085d2349bb67ffbee9 Mon Sep 17 00:00:00 2001 From: Mingshen Sun Date: Fri, 27 Jul 2018 15:23:18 +0800 Subject: [PATCH 5/5] Gracefully handle Result of OxideStream::new --- src/lib.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a3c81114..b75f3f64 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -155,7 +155,10 @@ macro_rules! oxidize { // Make sure we catch a potential panic, as // this is called from C. match catch_unwind(AssertUnwindSafe(|| { - let mut stream_oxide = StreamOxide::new(&mut *stream).unwrap(); + let mut stream_oxide = match StreamOxide::new(&mut *stream) { + Ok(stream_oxide) => stream_oxide, + Err(e) => return e as c_int + }; let status = $mz_func_oxide(&mut stream_oxide, $($arg_name),*); *stream = stream_oxide.into_mz_stream(); as_c_return_code(status) @@ -234,7 +237,11 @@ pub unsafe extern "C" fn mz_compress2( ..Default::default() }; - let mut stream_oxide = StreamOxide::new(&mut stream).unwrap(); + let mut stream_oxide = match StreamOxide::new(&mut stream) { + Ok(stream_oxide) => stream_oxide, + Err(e) => return e as c_int + }; + as_c_return_code(mz_compress2_oxide(&mut stream_oxide, level, dest_len)) }, ) @@ -272,7 +279,11 @@ pub unsafe extern "C" fn mz_uncompress( ..Default::default() }; - let mut stream_oxide = StreamOxide::new(&mut stream).unwrap(); + let mut stream_oxide = match StreamOxide::new(&mut stream) { + Ok(stream_oxide) => stream_oxide, + Err(e) => return e as c_int + }; + as_c_return_code(mz_uncompress2_oxide(&mut stream_oxide, dest_len)) }, )