Skip to content

Commit ced5f4b

Browse files
committed
common: address compression engine reviews
1 parent 360bfca commit ced5f4b

7 files changed

Lines changed: 259 additions & 176 deletions

File tree

‎Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ xz = "0.4.6-rc.0"
240240
xz-sys = "0.4.6-rc.0"
241241
libsqlite3-sys = "0.38"
242242
libz-rs-sys = "0.6"
243-
zlib-rs = { version = "0.6", default-features = false, features = ["rust-allocator", "__internal-api"] }
243+
zlib-rs = { version = "=0.6.7", default-features = false, features = ["rust-allocator", "__internal-api"] }
244244
lock_api = "0.4"
245245
log = "0.4.30"
246246
lz4_flex = "0.13"

‎crates/common/src/compression/lzma.rs‎

Lines changed: 84 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ use xz::stream::{
1111
TELL_ANY_CHECK, TELL_NO_CHECK,
1212
};
1313

14+
use super::{CHUNKSIZE, Chunker};
15+
1416
pub const BUFSIZ: usize = 8192;
15-
const CHUNKSIZE: usize = u32::MAX as usize;
1617
const DEF_BUF_SIZE: usize = 16 * 1024;
1718
const USE_AFTER_FINISH_ERR: &str = "Error -2: inconsistent stream state";
1819
const LZMA_FILTERS_MAX: usize = 4;
@@ -250,6 +251,9 @@ pub fn encode_filter_properties(spec: &FilterSpec) -> Result<Vec<u8>, Error> {
250251
let lc = spec.lc.unwrap_or(DEFAULT_LC);
251252
let lp = spec.lp.unwrap_or(DEFAULT_LP);
252253
let pb = spec.pb.unwrap_or(DEFAULT_PB);
254+
if lc > 4 || lp > 4 || lc + lp > 4 || pb > 4 {
255+
return Err(Error::Lzma("Invalid or unsupported options".to_owned()));
256+
}
253257
let dict_size = spec.dict_size.unwrap_or_else(|| preset_dict_size(preset));
254258
let mut result = vec![0u8; 5];
255259
result[0] = ((pb * 5 + lp) * 9 + lc) as u8;
@@ -370,42 +374,6 @@ impl LzmaStream {
370374
}
371375
}
372376

373-
struct Chunker<'a> {
374-
data1: &'a [u8],
375-
data2: &'a [u8],
376-
}
377-
378-
impl<'a> Chunker<'a> {
379-
fn chain(data1: &'a [u8], data2: &'a [u8]) -> Self {
380-
if data1.is_empty() {
381-
Self {
382-
data1: data2,
383-
data2: &[],
384-
}
385-
} else {
386-
Self { data1, data2 }
387-
}
388-
}
389-
fn len(&self) -> usize {
390-
self.data1.len() + self.data2.len()
391-
}
392-
fn is_empty(&self) -> bool {
393-
self.data1.is_empty()
394-
}
395-
fn chunk(&self) -> &'a [u8] {
396-
self.data1.get(..CHUNKSIZE).unwrap_or(self.data1)
397-
}
398-
fn advance(&mut self, consumed: usize) {
399-
self.data1 = &self.data1[consumed..];
400-
if self.data1.is_empty() {
401-
self.data1 = core::mem::take(&mut self.data2);
402-
}
403-
}
404-
fn to_vec(&self) -> Vec<u8> {
405-
[self.data1, self.data2].concat()
406-
}
407-
}
408-
409377
fn decompress_chunks(
410378
chunks: &mut Chunker<'_>,
411379
stream: &mut LzmaStream,
@@ -580,8 +548,20 @@ impl Compressor {
580548
}
581549
}
582550
FORMAT_ALONE => {
583-
let options = LzmaOptions::new_preset(preset)
584-
.map_err(|_| Error::Lzma(format!("Invalid compression preset: {preset}")))?;
551+
let options = match filters {
552+
None => LzmaOptions::new_preset(preset).map_err(|_| {
553+
Error::Lzma(format!("Invalid compression preset: {preset}"))
554+
})?,
555+
Some(specs) => match specs.as_slice() {
556+
[spec] if spec.id == FILTER_LZMA1 => lzma_options(spec)?,
557+
_ => {
558+
return Err(Error::Value(
559+
"Invalid filter chain for FORMAT_ALONE - must be a single LZMA1 filter"
560+
.to_owned(),
561+
));
562+
}
563+
},
564+
};
585565
Stream::new_lzma_encoder(&options)?
586566
}
587567
FORMAT_RAW => {
@@ -638,3 +618,68 @@ impl Compressor {
638618
Ok(output)
639619
}
640620
}
621+
622+
#[cfg(test)]
623+
mod tests {
624+
use super::*;
625+
626+
#[test]
627+
fn encode_lzma1_properties_rejects_invalid_lclppb() {
628+
for (lc, lp, pb) in [(5, 0, 0), (0, 5, 0), (4, 1, 0), (0, 0, 5)] {
629+
let spec = FilterSpec {
630+
id: FILTER_LZMA1,
631+
lc: Some(lc),
632+
lp: Some(lp),
633+
pb: Some(pb),
634+
..FilterSpec::default()
635+
};
636+
assert!(matches!(
637+
encode_filter_properties(&spec),
638+
Err(Error::Lzma(message)) if message == "Invalid or unsupported options"
639+
));
640+
}
641+
}
642+
643+
#[test]
644+
fn format_alone_uses_the_single_lzma1_filter() {
645+
let dict_size = 1 << 20;
646+
let spec = FilterSpec {
647+
id: FILTER_LZMA1,
648+
dict_size: Some(dict_size),
649+
..FilterSpec::default()
650+
};
651+
let mut compressor =
652+
Compressor::new(FORMAT_ALONE, CHECK_NONE, PRESET_DEFAULT, Some(vec![spec])).unwrap();
653+
let mut encoded = compressor.compress(b"hello").unwrap();
654+
encoded.extend(compressor.flush().unwrap());
655+
assert_eq!(&encoded[1..5], &dict_size.to_le_bytes());
656+
}
657+
658+
#[test]
659+
fn format_alone_rejects_other_filter_chains() {
660+
for filters in [
661+
vec![],
662+
vec![FilterSpec {
663+
id: FILTER_LZMA2,
664+
..FilterSpec::default()
665+
}],
666+
vec![
667+
FilterSpec {
668+
id: FILTER_LZMA1,
669+
..FilterSpec::default()
670+
},
671+
FilterSpec {
672+
id: FILTER_LZMA1,
673+
..FilterSpec::default()
674+
},
675+
],
676+
] {
677+
assert!(matches!(
678+
Compressor::new(FORMAT_ALONE, CHECK_NONE, PRESET_DEFAULT, Some(filters)),
679+
Err(Error::Value(message))
680+
if message
681+
== "Invalid filter chain for FORMAT_ALONE - must be a single LZMA1 filter"
682+
));
683+
}
684+
}
685+
}

‎crates/common/src/compression/mod.rs‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,74 @@
1+
// spell-checker:ignore chunker
2+
13
//! VM-independent compression engines shared by RustPython components.
24
5+
const CHUNKSIZE: usize = u32::MAX as usize;
6+
7+
/// A two-slice cursor used by streaming decompressors to consume buffered and
8+
/// newly supplied input without joining them first.
9+
#[doc(hidden)]
10+
#[derive(Clone)]
11+
pub struct Chunker<'a> {
12+
data1: &'a [u8],
13+
data2: &'a [u8],
14+
}
15+
16+
impl<'a> Chunker<'a> {
17+
/// Start a cursor over one input slice.
18+
#[must_use]
19+
pub const fn new(data: &'a [u8]) -> Self {
20+
Self {
21+
data1: data,
22+
data2: &[],
23+
}
24+
}
25+
26+
/// Chain previously buffered input in front of newly supplied input.
27+
#[must_use]
28+
pub const fn chain(data1: &'a [u8], data2: &'a [u8]) -> Self {
29+
if data1.is_empty() {
30+
Self {
31+
data1: data2,
32+
data2: &[],
33+
}
34+
} else {
35+
Self { data1, data2 }
36+
}
37+
}
38+
39+
/// Return the number of bytes that have not been consumed.
40+
#[must_use]
41+
pub const fn len(&self) -> usize {
42+
self.data1.len() + self.data2.len()
43+
}
44+
45+
/// Return whether all input has been consumed.
46+
#[must_use]
47+
pub const fn is_empty(&self) -> bool {
48+
self.data1.is_empty()
49+
}
50+
51+
/// Copy the remaining input into a contiguous vector.
52+
#[must_use]
53+
pub fn to_vec(&self) -> Vec<u8> {
54+
[self.data1, self.data2].concat()
55+
}
56+
57+
/// Return the next native-codec-sized input chunk.
58+
#[must_use]
59+
pub fn chunk(&self) -> &'a [u8] {
60+
self.data1.get(..CHUNKSIZE).unwrap_or(self.data1)
61+
}
62+
63+
/// Advance the cursor by `consumed` bytes.
64+
pub fn advance(&mut self, consumed: usize) {
65+
self.data1 = &self.data1[consumed..];
66+
if self.data1.is_empty() {
67+
self.data1 = core::mem::take(&mut self.data2);
68+
}
69+
}
70+
}
71+
372
#[cfg(all(
473
feature = "lzma",
574
not(any(target_os = "android", target_arch = "wasm32"))

0 commit comments

Comments
 (0)