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
33 changes: 23 additions & 10 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ quick-xml = { version = "0.42", features = ["escape-html"] }
# pulling every codec (bzip2/zstd/lzma/aes-crypto chains incl. zstd-sys's C
# build) and zip's optional `time`. The zlib-rs backend keeps flate2 pure-Rust
# (no C toolchain) and matches the backend the old default features already used.
zip = { version = "8.6", default-features = false, features = ["deflate-flate2-zlib-rs"] }
zip = { version = "9.0", default-features = false, features = ["deflate-flate2-zlib-rs"] }
# Held below 0.40 deliberately: 0.40 pulls a libsqlite3-sys that uses the
# `cfg_select!` macro, which does not exist in our MSRV (see docs/msrv-audit.md
# for why the floor is 1.88 and what it buys). Raising past 0.39 means raising
Expand Down
2 changes: 1 addition & 1 deletion crates/bookforge-audio/src/atomic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub fn replace_file(staged: &Path, destination: &Path) -> io::Result<()> {
Err(_) => {}
}
}
return replace_file_windows(staged, destination);
replace_file_windows(staged, destination)
}

#[cfg(not(windows))]
Expand Down
23 changes: 17 additions & 6 deletions crates/bookforge-epub/src/archive_limits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

use std::{
fs::File,
io::{Read, Seek, SeekFrom},
io::{ErrorKind, Read, Seek, SeekFrom},
path::Path,
};

Expand Down Expand Up @@ -256,7 +256,16 @@ impl ArchiveReadBudget {
// without allowing the decompressor or Vec to consume the rest of an attacker stream.
let mut bounded = reader.take(read_limit.saturating_add(1));
let mut bytes = Vec::with_capacity(read_limit.min(64 * 1024) as usize);
bounded.read_to_end(&mut bytes)?;
bounded.read_to_end(&mut bytes).map_err(|error| {
// zip itself refuses to inflate an entry past its declared
// uncompressed size, reporting InvalidData before this budget
// trips. Name the entry, as the budget's own limit errors do.
if error.kind() == ErrorKind::InvalidData {
limit_error(format!("failed for '{name}': {error}"))
} else {
error.into()
}
})?;
let bytes_read = bytes.len() as u64;

if bytes_read > read_limit {
Expand Down Expand Up @@ -304,7 +313,7 @@ pub fn validate_archive_metadata<R: Read + Seek>(
let mut total_compressed = 0u64;
for index in 0..archive.len() {
let file = archive.by_index_raw(index)?;
let name = file.name().to_string();
let name = file.name()?.into_owned();
let uncompressed = file.size();
let compressed = file.compressed_size();
if uncompressed > limits.max_entry_uncompressed_size {
Expand Down Expand Up @@ -434,10 +443,12 @@ mod tests {
.read_entry(&mut file, "liar.xhtml", compressed_size)
.expect_err("bounded read must catch actual expansion");

// zip stops at the declared 1 KiB, before the per-entry limit.
let message = error.to_string();
assert!(
error
.to_string()
.contains("per-entry uncompressed size limit exceeded for 'liar.xhtml'")
message.contains("EPUB decompression failed for 'liar.xhtml'")
&& message.contains("declared uncompressed size"),
"{message}"
);
}

Expand Down
4 changes: 2 additions & 2 deletions crates/bookforge-epub/src/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,8 @@ pub fn inspect_epub(path: &Path) -> Result<EpubInspection> {
// covers hostile archives).
let archive_names = archive
.file_names()
.map(|name| name.to_string())
.collect::<HashSet<_>>();
.map(|name| name.map(String::from))
.collect::<std::result::Result<HashSet<_>, _>>()?;

let mut xhtml_spine_count = 0;
for item in &package.spine {
Expand Down
2 changes: 1 addition & 1 deletion crates/bookforge-epub/src/reflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ fn write_archive_entries(
) -> Result<()> {
for index in 0..archive.len() {
let mut file = archive.by_index(index)?;
let name = file.name().to_string();
let name = file.name()?.into_owned();

if name == "mimetype" {
continue;
Expand Down
4 changes: 3 additions & 1 deletion crates/bookforge-epub/src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,9 @@ pub fn validate_translated_epub(
let Ok(mut entry) = archive.by_index(index) else {
continue;
};
let name = entry.name().to_string();
let Ok(name) = entry.name().map(|name| name.into_owned()) else {
continue;
};
if !is_validatable_resource_name(&name) {
file_names.push(name);
continue;
Expand Down
4 changes: 2 additions & 2 deletions crates/bookforge-epub/src/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ fn write_rebuilt_epub(
// A raw (metadata-only) handle answers cheap questions — name,
// directory-ness, declared compression — before any bytes move.
let raw = archive.by_index_raw(index)?;
let name = raw.name().to_string();
let name = raw.name()?.into_owned();

if name == "mimetype" {
continue;
Expand Down Expand Up @@ -324,7 +324,7 @@ fn normalized_entry_options(compression_method: CompressionMethod) -> SimpleFile
fn archive_entry_names(archive: &mut ZipArchive<File>) -> Result<HashSet<String>> {
let mut names = HashSet::new();
for index in 0..archive.len() {
names.insert(archive.by_index_raw(index)?.name().to_string());
names.insert(archive.by_index_raw(index)?.name()?.into_owned());
}
Ok(names)
}
Expand Down
33 changes: 18 additions & 15 deletions crates/bookforge-epub/tests/hostile_corpus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ use std::{
use bookforge_core::{config::BilingualMode, segment::BlockTranslation};
use bookforge_epub::{
RebuildOptions,
archive_limits::{ArchiveLimits, read_archive_text, validate_archive_metadata},
archive_limits::{
ArchiveLimits, ArchiveReadBudget, read_archive_text, validate_archive_metadata,
},
inspect_epub, read_epub, rebuild_epub, rebuild_epub_with_options, text_coverage,
};
use quick_xml::{Reader, events::Event};
Expand Down Expand Up @@ -236,8 +238,9 @@ fn injectable_limits_catch_declared_size_and_ratio_lies() {
fn bounded_read_catches_declared_sizes_that_lie_about_expansion() {
let dir = tmp_dir("bomb-read");
let epub = dir.join("lie.epub");
// Stored payload expands far beyond what the central directory claims;
// with an injected 1 KiB cap the bounded read stops after 1 KiB + 1.
// Stored payload expands far beyond what the central directory claims.
// zip refuses to inflate past the declared 16 bytes, before the injected
// 1 KiB cap; the rejection must still name the entry.
let big = vec![b'x'; 64 * 1024];
write_deflated_fixture(&dir.join("liar-src.zip"), "liar.bin", &big);
relabel_declared_sizes(&dir.join("liar-src.zip"), &epub);
Expand All @@ -255,9 +258,11 @@ fn bounded_read_catches_declared_sizes_that_lie_about_expansion() {
let error = budget
.read_entry(&mut file, "liar.bin", compressed)
.expect_err("actual 64 KiB expansion must be caught");
let message = error.to_string();
assert!(
error.to_string().contains("expanded data exceeds"),
"unexpected error: {error}"
message.contains("EPUB decompression failed for 'liar.bin'")
&& message.contains("declared uncompressed size"),
"unexpected error: {message}"
);
}

Expand Down Expand Up @@ -293,10 +298,11 @@ fn relabel_declared_sizes(source: &Path, target: &Path) {

#[test]
fn bounded_budget_tracks_total_expansion_across_entries() {
// Two deflated entries whose central declarations say 16 bytes but that
// actually expand to 48 KiB each. The metadata stage passes (declared
// totals fit the tiny caps), so the running total is enforced at READ
// time: entry one fits, entry two crosses the remaining budget.
// Two deflated entries that expand to 48 KiB each against an 80 KiB
// running total, enforced at READ time: entry one fits, entry two crosses
// the remaining budget. zip rejects entries that inflate past their
// declared size, so these are honest and the budget is built directly;
// validate_archive_metadata would reject the declared 96 KiB upfront.
let half: Vec<u8> = vec![b'z'; 48 * 1024];
let dir = tmp_dir("bomb-total");
let source = dir.join("half-src.zip");
Expand All @@ -308,10 +314,8 @@ fn bounded_budget_tracks_total_expansion_across_entries() {
zip.start_file("b.bin", deflated).expect("start");
zip.write_all(&half).expect("write");
zip.finish().expect("finish");
let epub = dir.join("liars.epub");
relabel_declared_sizes(&source, &epub);

let mut archive = ZipArchive::new(File::open(&epub).expect("open")).expect("zip");
let mut archive = ZipArchive::new(File::open(&source).expect("open")).expect("zip");
let limits = ArchiveLimits {
max_entry_uncompressed_size: 64 * 1024,
max_total_uncompressed_size: 80 * 1024,
Expand All @@ -321,8 +325,7 @@ fn bounded_budget_tracks_total_expansion_across_entries() {
..tiny_limits(true)
}
};
let mut budget =
validate_archive_metadata(&mut archive, limits).expect("16-byte declarations fit");
let mut budget = ArchiveReadBudget::new(limits);

for name in ["a.bin", "b.bin"] {
let mut file = archive.by_name(name).expect("entry exists");
Expand Down Expand Up @@ -1215,7 +1218,7 @@ fn xml_string_api_preserves_unicode_entities_namespaces_and_archive_members() {
assert_eq!(entry_bytes(&output, "mimetype"), MIMETYPE);
let mut archive = ZipArchive::new(File::open(&output).unwrap()).unwrap();
let first = archive.by_index(0).unwrap();
assert_eq!(first.name(), "mimetype");
assert_eq!(first.name().unwrap(), "mimetype");
assert_eq!(first.compression(), CompressionMethod::Stored);
}

Expand Down
58 changes: 55 additions & 3 deletions crates/bookforge-epub/tests/inspect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,50 @@ fn rebuilds_epub_with_patched_xhtml_and_preserved_resources() {
assert!(chapter.contains("Ciao da un EPUB minimo."));
}

#[test]
fn non_ascii_entry_names_resolve_through_inspect_read_and_rebuild() {
// zip 9 decodes entry names on access instead of at parse time; a UTF-8
// name must still match its manifest href on every archive path.
let href = "capítulo-東京.xhtml";
let fixture =
create_epub_fixture_with_chapter("non-ascii-name", href, "<p>Hello from chapter 1.</p>");

let inspection = inspect_epub(&fixture).expect("non-ASCII spine entry should be found");
assert_eq!(inspection.xhtml_spine_count, 1);

let book = read_epub(&fixture).expect("non-ASCII entry should parse into IR");
let body_block = book
.blocks
.iter()
.find(|block| block_text(block) == "Hello from chapter 1.")
.expect("body paragraph should be extracted");

let output = std::env::temp_dir().join(format!(
"bookforge-rebuilt-non-ascii-{}.epub",
std::process::id()
));
let _ = std::fs::remove_file(&output);
rebuild_epub(
&book,
&[BlockTranslation {
block_id: body_block.id.clone(),
text: "Ciao da un capitolo non ASCII.".to_string(),
}],
&output,
)
.expect("EPUB with a non-ASCII entry should rebuild");

let mut archive = ZipArchive::new(File::open(&output).expect("rebuilt EPUB should exist"))
.expect("rebuilt EPUB should be a zip");
let mut chapter = String::new();
archive
.by_name(&format!("OEBPS/{href}"))
.expect("non-ASCII entry name should survive the rebuild")
.read_to_string(&mut chapter)
.expect("chapter should be UTF-8");
assert!(chapter.contains("Ciao da un capitolo non ASCII."));
}

#[test]
fn packed_notes_extract_and_rebuild_at_matching_dom_paths() {
let fixture = create_epub_fixture(
Expand Down Expand Up @@ -434,6 +478,10 @@ fn block_text(block: &bookforge_core::ir::Block) -> String {
}

fn create_epub_fixture(name: &str, body: &str) -> PathBuf {
create_epub_fixture_with_chapter(name, "chapter1.xhtml", body)
}

fn create_epub_fixture_with_chapter(name: &str, chapter_href: &str, body: &str) -> PathBuf {
let nanos = SystemTime::now()
.duration_since(UNIX_EPOCH)
.expect("clock should be after epoch")
Expand Down Expand Up @@ -463,16 +511,20 @@ fn create_epub_fixture(name: &str, body: &str) -> PathBuf {
.start_file("OEBPS/content.opf", deflated)
.expect("package entry should start");
writer
.write_all(CONTENT_OPF.as_bytes())
.write_all(
CONTENT_OPF
.replace("chapter1.xhtml", chapter_href)
.as_bytes(),
)
.expect("package should write");
writer
.start_file("OEBPS/nav.xhtml", deflated)
.expect("nav entry should start");
writer
.write_all(NAV_XHTML.as_bytes())
.write_all(NAV_XHTML.replace("chapter1.xhtml", chapter_href).as_bytes())
.expect("nav should write");
writer
.start_file("OEBPS/chapter1.xhtml", deflated)
.start_file(format!("OEBPS/{chapter_href}"), deflated)
.expect("chapter entry should start");
writer
.write_all(chapter_xhtml(body).as_bytes())
Expand Down
Loading
Loading