Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions crates/peryx-archive/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,8 @@ fn text_chunk(member: &str, mut chunk: MemberChunk) -> Result<MemberChunk, Archi
Ok(_) => Ok(chunk),
Err(err) if err.error_len().is_none() && chunk.next_offset.is_some() && err.valid_up_to() > 0 => {
chunk.bytes.truncate(err.valid_up_to());
let next = chunk.offset + u64::try_from(chunk.bytes.len()).unwrap_or_default();
chunk.next_offset = (next < chunk.size).then_some(next);
// The trimmed tail lies inside the member, so a further chunk always follows.
chunk.next_offset = Some(chunk.offset + u64::try_from(chunk.bytes.len()).unwrap_or_default());
Ok(chunk)
}
Err(_) => Err(ArchiveError::BinaryMember(member.to_owned())),
Expand Down Expand Up @@ -200,9 +200,7 @@ fn read_zip_member(
let member = safe_member_name(member)?;
let mut archive = zip::ZipArchive::new(reader).map_err(read_error)?;
let position = zip_member_position(&mut archive, &member)?.ok_or(ArchiveError::MemberNotFound)?;
if offset > 0
&& let Ok(mut entry) = archive.by_index_seek(position)
{
if let Ok(mut entry) = archive.by_index_seek(position) {
let size = entry.get_metadata().uncompressed_size;
if offset > size {
return Err(ArchiveError::InvalidRange { offset, size });
Expand Down
15 changes: 9 additions & 6 deletions crates/peryx-archive/src/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,14 +198,17 @@ impl Read for FileRangeReader {
impl Seek for FileRangeReader {
fn seek(&mut self, pos: SeekFrom) -> std::io::Result<u64> {
let position = match pos {
SeekFrom::Start(offset) => offset.min(self.len),
SeekFrom::Current(offset) if offset < 0 => self.position.saturating_sub(offset.unsigned_abs()),
SeekFrom::Current(offset) => self.position.saturating_add(offset.unsigned_abs()).min(self.len),
SeekFrom::End(offset) if offset < 0 => self.len.saturating_sub(offset.unsigned_abs()),
SeekFrom::End(_) => self.len,
};
SeekFrom::Start(offset) => offset,
SeekFrom::Current(offset) => self.position.saturating_add_signed(offset),
SeekFrom::End(offset) => self.len.saturating_add_signed(offset),
}
.min(self.len);
self.file.seek(SeekFrom::Start(self.start + position))?;
self.position = position;
Ok(position)
}
}

#[cfg(test)]
#[path = "../tests/unit/source_tests.rs"]
mod tests;
4 changes: 2 additions & 2 deletions crates/peryx-archive/src/zip_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ const COMPRESSION_DEFLATED: u16 = 8;
/// fields carry no claim to cross-check.
const FLAG_DATA_DESCRIPTOR: u16 = 1 << 3;

/// General-purpose bits this reader can honour. Bits 1 and 2 only hint at the deflate level, bit 3
/// General-purpose bits this reader can honour: bits 1 and 2 only hint at the deflate level, bit 3
/// defers the CRC and sizes to a data descriptor, and bit 11 declares the name UTF-8. Bits 0 and 6
/// encrypt the member, bit 13 masks the very fields cross-checked here, and the rest are reserved.
const READABLE_FLAGS: u16 = (1 << 1) | (1 << 2) | FLAG_DATA_DESCRIPTOR | (1 << 11);
const READABLE_FLAGS: u16 = 0b1000_0000_1110;

/// The span the central directory occupies inside the archive.
#[derive(Debug, PartialEq, Eq)]
Expand Down
127 changes: 127 additions & 0 deletions crates/peryx-archive/tests/unit/source_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
use std::io::{Cursor, Read as _, Seek as _, SeekFrom};

use rstest::rstest;

use super::{ArchiveSource, resolve_container_stack};
use crate::ArchiveError;
use crate::tests::{BODY, PROFILE, write_archive, zip};

const CONTAINER: &str = "inner.zip";

fn inner() -> Vec<u8> {
zip(&[("text.txt", BODY)], zip::CompressionMethod::Stored)
}

/// Offset of the outer archive's local header for its first member.
fn local_header(outer: &[u8]) -> usize {
outer.windows(4).position(|window| window == b"PK\x03\x04").unwrap()
}

/// Offset of the outer archive's central-directory entry; a stored inner zip carries its own
/// entry earlier in the bytes, so the outer's is the last one.
fn central_entry(outer: &[u8]) -> usize {
outer.windows(4).rposition(|window| window == b"PK\x01\x02").unwrap()
}

#[test]
fn test_stored_container_is_read_in_place() {
let inner = inner();
let outer = zip(&[(CONTAINER, &inner)], zip::CompressionMethod::Stored);
let (_dir, path) = write_archive(&outer);
let data_start = zip::ZipArchive::new(Cursor::new(&outer))
.unwrap()
.by_index(0)
.unwrap()
.data_start();

let resolved = resolve_container_stack(&PROFILE, "outer.zip", &path, &[CONTAINER.to_owned()]).unwrap();

assert_eq!(
(resolved.source.path, resolved.source.start, resolved.source.len),
(path, data_start.unwrap(), Some(inner.len() as u64))
);
}

#[test]
fn test_compressed_container_is_spilled_to_its_own_file() {
let outer = zip(&[(CONTAINER, &inner())], zip::CompressionMethod::Deflated);
let (_dir, path) = write_archive(&outer);

let resolved = resolve_container_stack(&PROFILE, "outer.zip", &path, &[CONTAINER.to_owned()]).unwrap();

assert_ne!(resolved.source.path, path);
assert_eq!((resolved.source.start, resolved.source.len), (0, None));
}

#[test]
fn test_stored_container_declaring_another_size_is_rejected_as_truncated() {
let inner = inner();
let mut outer = zip(&[(CONTAINER, &inner)], zip::CompressionMethod::Stored);
let declared = (u32::try_from(inner.len()).unwrap() + 1).to_le_bytes();
let local = local_header(&outer);
outer[local + 22..local + 26].copy_from_slice(&declared);
let central = central_entry(&outer);
outer[central + 24..central + 28].copy_from_slice(&declared);
let (_dir, path) = write_archive(&outer);

assert!(matches!(
resolve_container_stack(&PROFILE, "outer.zip", &path, &[CONTAINER.to_owned()]),
Err(ArchiveError::TruncatedMember { expected, actual })
if expected == inner.len() as u64 + 1 && actual == inner.len() as u64
));
}

#[test]
fn test_encrypted_container_is_rejected_before_it_is_read() {
let mut outer = zip(&[(CONTAINER, &inner())], zip::CompressionMethod::Stored);
let local = local_header(&outer);
outer[local + 6] |= 1;
let central = central_entry(&outer);
outer[central + 8] |= 1;
let (_dir, path) = write_archive(&outer);

assert!(matches!(
resolve_container_stack(&PROFILE, "outer.zip", &path, &[CONTAINER.to_owned()]),
Err(ArchiveError::Read(message)) if message.contains("assword")
));
}

#[test]
fn test_slice_reads_stop_at_the_slice_end() {
let (_dir, path) = write_archive(b"0123456789");
let mut reader = ArchiveSource::new(path).slice(2, 3).open().unwrap();
let mut bytes = Vec::new();

reader.read_to_end(&mut bytes).unwrap();

assert_eq!(bytes, b"234");
}

#[rstest]
#[case::start_within(SeekFrom::Start(1), SeekFrom::Start(3), 3, b"56")]
#[case::start_past_the_end(SeekFrom::Start(1), SeekFrom::Start(9), 5, b"")]
#[case::current_forward(SeekFrom::Start(1), SeekFrom::Current(1), 2, b"456")]
#[case::current_in_place(SeekFrom::Start(1), SeekFrom::Current(0), 1, b"3456")]
#[case::current_backward(SeekFrom::Start(3), SeekFrom::Current(-1), 2, b"456")]
#[case::current_before_the_start(SeekFrom::Start(1), SeekFrom::Current(-5), 0, b"23456")]
#[case::current_past_the_end(SeekFrom::Start(1), SeekFrom::Current(10), 5, b"")]
#[case::end_backward(SeekFrom::Start(0), SeekFrom::End(-2), 3, b"56")]
#[case::end_before_the_start(SeekFrom::Start(0), SeekFrom::End(-9), 0, b"23456")]
#[case::end_in_place(SeekFrom::Start(0), SeekFrom::End(0), 5, b"")]
#[case::end_forward(SeekFrom::Start(0), SeekFrom::End(3), 5, b"")]
fn test_slice_seeks_stay_within_the_slice(
#[case] first: SeekFrom,
#[case] second: SeekFrom,
#[case] position: u64,
#[case] rest: &[u8],
) {
let (_dir, path) = write_archive(b"0123456789");
let mut reader = ArchiveSource::new(path).slice(2, 5).open().unwrap();
reader.seek(first).unwrap();

assert_eq!(reader.seek(second).unwrap(), position);

let mut bytes = Vec::new();
reader.read_to_end(&mut bytes).unwrap();
assert_eq!(bytes, rest);
}
107 changes: 86 additions & 21 deletions crates/peryx-archive/tests/unit/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@ use flate2::{Compression, write::GzEncoder};
use rstest::rstest;

use crate::{
ArchiveError, ArchiveFormat, ArchiveProfile, MAX_DECOMPRESSED_INSPECT_BYTES, Member, MemberKind, generic_format,
generic_member_kind, list_members, list_members_nested_path, list_members_path, read_error, read_member,
read_member_chunk, read_member_chunk_path, read_text_member_chunk_nested_path, safe_member_name,
strip_ascii_suffix_ignore_case,
ArchiveError, ArchiveFormat, ArchiveProfile, DEFAULT_MEMBER_CHUNK, MAX_CONTAINER_DEPTH,
MAX_DECOMPRESSED_INSPECT_BYTES, MAX_MEMBER_CHUNK, MAX_NESTED_ARCHIVE_SIZE, MAX_ZIP_CENTRAL_DIRECTORY_BYTES, Member,
MemberChunk, MemberKind, generic_format, generic_member_kind, list_members, list_members_nested_path,
list_members_path, read_error, read_member, read_member_chunk, read_member_chunk_path,
read_text_member_chunk_nested_path, safe_member_name, strip_ascii_suffix_ignore_case,
};

const BODY: &[u8] = b"body\n";
pub const BODY: &[u8] = b"body\n";

struct TestProfile;
pub struct TestProfile;

impl ArchiveProfile for TestProfile {
fn format(&self, name: &str) -> Option<ArchiveFormat> {
Expand All @@ -24,9 +25,9 @@ impl ArchiveProfile for TestProfile {
}
}

const PROFILE: TestProfile = TestProfile;
pub const PROFILE: TestProfile = TestProfile;

fn zip(entries: &[(&str, &[u8])], method: zip::CompressionMethod) -> Vec<u8> {
pub fn zip(entries: &[(&str, &[u8])], method: zip::CompressionMethod) -> Vec<u8> {
let mut buf = Vec::new();
{
let mut archive = zip::ZipWriter::new(std::io::Cursor::new(&mut buf));
Expand Down Expand Up @@ -111,11 +112,11 @@ fn truncated_tar(path: &str, size: u64, body: &[u8]) -> Vec<u8> {
bytes
}

fn oversized_nested_tar() -> Vec<u8> {
fn nested_tar_declaring(size: u64) -> Vec<u8> {
let mut header = tar::Header::new_gnu();
header.set_path("inner.zip").unwrap();
header.set_mode(0o644);
header.set_size((128 << 20) + 1);
header.set_size(size);
header.set_cksum();
let mut bytes = header.as_bytes().to_vec();
bytes.extend_from_slice(&[0; 1024]);
Expand All @@ -129,7 +130,7 @@ fn zip_with_declared_size(size: u32) -> Vec<u8> {
bytes
}

fn write_archive(bytes: &[u8]) -> (tempfile::TempDir, std::path::PathBuf) {
pub fn write_archive(bytes: &[u8]) -> (tempfile::TempDir, std::path::PathBuf) {
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("archive");
std::fs::write(&path, bytes).unwrap();
Expand Down Expand Up @@ -252,6 +253,15 @@ fn listed_text_member(path: &str) -> Member {
}
}

#[rstest]
#[case::default_member_chunk(DEFAULT_MEMBER_CHUNK, 262_144)]
#[case::max_member_chunk(MAX_MEMBER_CHUNK, 1_048_576)]
#[case::max_nested_archive(MAX_NESTED_ARCHIVE_SIZE, 134_217_728)]
#[case::max_zip_central_directory(MAX_ZIP_CENTRAL_DIRECTORY_BYTES, 16_777_216)]
fn test_byte_limits_are_the_documented_sizes(#[case] limit: u64, #[case] bytes: u64) {
assert_eq!(limit, bytes);
}

#[test]
fn test_read_error_preserves_the_source_message() {
assert!(matches!(
Expand Down Expand Up @@ -334,9 +344,26 @@ fn test_file_backed_archive_readers_share_listing_and_range_behavior() {
(chunk.bytes, chunk.size, chunk.offset, chunk.next_offset),
(b"cka".to_vec(), 7, 2, Some(5))
);
let tail = read_member_chunk_path(&PROFILE, name, &path, "file.txt", 2, u64::MAX).unwrap();
assert_eq!((tail.bytes, tail.next_offset), (b"ckage".to_vec(), None));
}
}

#[rstest]
#[case("bundle.zip", zip_with("file.txt"))]
#[case("bundle.tar", tar(&[("file.txt", BODY)]))]
fn test_member_chunk_at_the_member_end_is_empty(#[case] filename: &str, #[case] bytes: Vec<u8>) {
assert_eq!(
read_member_chunk(&PROFILE, filename, &bytes, "file.txt", 5, 1).unwrap(),
MemberChunk {
bytes: Vec::new(),
size: 5,
offset: 5,
next_offset: None,
}
);
}

#[rstest]
#[case(zip::CompressionMethod::Stored)]
#[case(zip::CompressionMethod::Deflated)]
Expand Down Expand Up @@ -366,6 +393,16 @@ fn test_text_chunks_trim_an_incomplete_trailing_character() {
assert_eq!((chunk.bytes, chunk.next_offset), (b"ab".to_vec(), Some(2)));
}

#[test]
fn test_text_chunks_reject_a_window_shorter_than_its_first_character() {
let bytes = zip(&[("text.txt", "茅a".as_bytes())], zip::CompressionMethod::Deflated);
let (_dir, path) = write_archive(&bytes);
assert!(matches!(
read_text_member_chunk_nested_path(&PROFILE, "bundle.zip", &path, &[], "text.txt", 0, 1),
Err(ArchiveError::BinaryMember(name)) if name == "text.txt"
));
}

#[rstest]
#[case::invalid_first_byte("text.txt", &[0xff])]
#[case::invalid_after_ascii("text.txt", b"a\xff")]
Expand Down Expand Up @@ -498,19 +535,24 @@ fn test_tar_member_read_rejects_the_inspection_boundary() {
));
}

#[test]
fn test_tar_member_read_allows_the_exact_inspection_boundary() {
#[rstest]
#[case::empty_window(MAX_DECOMPRESSED_INSPECT_BYTES, MAX_DECOMPRESSED_INSPECT_BYTES - 512, 0)]
#[case::window_to_the_member_end(MAX_DECOMPRESSED_INSPECT_BYTES - 512, MAX_DECOMPRESSED_INSPECT_BYTES - 1024, u64::MAX)]
fn test_tar_member_read_allows_the_exact_inspection_boundary(
#[case] size: u64,
#[case] offset: u64,
#[case] limit: u64,
) {
assert!(matches!(
read_member_chunk(
&PROFILE,
"bundle.tar",
&truncated_tar("file.txt", MAX_DECOMPRESSED_INSPECT_BYTES, BODY),
&truncated_tar("file.txt", size, BODY),
"file.txt",
MAX_DECOMPRESSED_INSPECT_BYTES - 512,
0,
offset,
limit,
),
Err(ArchiveError::TruncatedMember { expected, actual })
if expected == MAX_DECOMPRESSED_INSPECT_BYTES && actual == BODY.len() as u64
Err(ArchiveError::TruncatedMember { expected, actual }) if expected == size && actual == BODY.len() as u64
));
}

Expand Down Expand Up @@ -584,9 +626,23 @@ fn test_nested_archive_reader_rejects_unsafe_and_excessive_paths() {
Err(ArchiveError::UnsafeMember(_))
));
assert!(matches!(
list_members_nested_path(&PROFILE, "outer.zip", &path, &vec!["inner.zip".to_owned(); 9]),
list_members_nested_path(
&PROFILE,
"outer.zip",
&path,
&vec!["inner.zip".to_owned(); MAX_CONTAINER_DEPTH + 1]
),
Err(ArchiveError::NestingTooDeep { .. })
));
assert!(matches!(
list_members_nested_path(
&PROFILE,
"outer.zip",
&path,
&vec!["inner.zip".to_owned(); MAX_CONTAINER_DEPTH]
),
Err(ArchiveError::MemberNotFound)
));
assert!(matches!(
list_members_nested_path(&PROFILE, "outer.zip", &path, &["inner.bin".to_owned()]),
Err(ArchiveError::UnsupportedNestedArchive(name)) if name == "inner.bin"
Expand All @@ -600,10 +656,19 @@ fn test_nested_archive_reader_rejects_unsafe_and_excessive_paths() {
list_members_nested_path(&PROFILE, "outer.tar", &path, &["inner.zip".to_owned()]),
Err(ArchiveError::MemberNotFound)
));
let (_dir, path) = write_archive(&oversized_nested_tar());
let (_dir, path) = write_archive(&nested_tar_declaring(MAX_NESTED_ARCHIVE_SIZE + 1));
assert!(matches!(
list_members_nested_path(&PROFILE, "outer.tar", &path, &["inner.zip".to_owned()]),
Err(ArchiveError::NestedArchiveTooLarge { size, .. }) if size == MAX_NESTED_ARCHIVE_SIZE + 1
));
}

#[test]
fn test_nested_archive_reader_allows_a_container_at_the_size_limit() {
let (_dir, path) = write_archive(&nested_tar_declaring(MAX_NESTED_ARCHIVE_SIZE));
assert!(matches!(
list_members_nested_path(&PROFILE, "outer.tar", &path, &["inner.zip".to_owned()]),
Err(ArchiveError::NestedArchiveTooLarge { size, .. }) if size == (128 << 20) + 1
Err(ArchiveError::TruncatedMember { expected, actual: 1024 }) if expected == MAX_NESTED_ARCHIVE_SIZE
));
}

Expand Down
Loading