From a10d9c6f89e95472291a5168596a1928b90b1d17 Mon Sep 17 00:00:00 2001 From: Afonsop Date: Fri, 18 Sep 2026 05:15:37 +0100 Subject: [PATCH] fix: trust the probe for over-budget images; TIFF allowance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the image wave (with empirical reproduction against the vendored image 0.25) found the budget contract did not hold across formats: the decoder's Limits reserve checks NATIVE bytes-per-pixel, so a grayscale/RGB JPEG/PNG/BMP over the RGBA budget decoded anyway and into_rgba8() then ballooned to up to 4x it — the tab death the budgets exist to prevent. The probed dimensions already prove an OverBudget image cannot fit, so it is now refused without decoding, removing all dependence on per-decoder limit fidelity. Conversely, image's TIFF codec stacks ~3x scratch buffers on top of the reserve, so an in-budget TIFF (the flagship GIS scenario) was refused with a self-contradictory note ('400 MB, over the 512 MB limit'); the decoder allowance for TIFF is now 3x while tier math stays on the real budget. Also: the refusal message saturates instead of overflowing on crafted headers, the image pick only matches the refusal note naming ITS file (peek, not drain — other references' notes survive for their own drains), and the Georgian Supplement range ends at U+2D2F (U+2D30.. is Tifinagh, which ships no font). (cherry picked from commit 58d3789bcf72271c7a8db30cafe3122a052466ff) Ported onto main (#1717 review): the build-web.sh wrapper and its pages/README/Trunk wiring stay out — it works around a local Windows trunk dist lock; the Linux Pages build is unaffected. --- src/io/mod.rs | 9 ++++++-- src/scene/model/image_model.rs | 41 +++++++++++++++++++++++++++++----- src/scene/text/web_font.rs | 2 +- web/fonts/generate.sh | 2 +- 4 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/io/mod.rs b/src/io/mod.rs index b1c5cd0fb..d4a836592 100644 --- a/src/io/mod.rs +++ b/src/io/mod.rs @@ -2040,8 +2040,13 @@ pub async fn pick_image_file() -> Result<(PathBuf, u32, u32), String> { let decoded = crate::scene::model::image_model::resolve_image(&reference).ok_or_else(|| { // An over-budget decode refusal explains itself (dims + limit); // anything else stays the generic unsupported/corrupt message. - crate::scene::model::image_model::take_decode_refusals() - .pop() + // Only a note naming THIS pick explains its failure; the queue may + // also hold refusals queued by concurrent re-resolution of other + // references, which must survive for their own drains. + crate::scene::model::image_model::decode_refusals() + .iter() + .find(|note| note.contains(short.as_str())) + .cloned() .unwrap_or_else(|| format!("Unsupported or corrupt image \"{short}\"")) })?; // Entity sizing truth: the file's ORIGINAL pixel dimensions, not the diff --git a/src/scene/model/image_model.rs b/src/scene/model/image_model.rs index 56e4f80aa..7d186e45f 100644 --- a/src/scene/model/image_model.rs +++ b/src/scene/model/image_model.rs @@ -775,16 +775,34 @@ fn decode_with_budgets( tier_for(width, height, max_decode_bytes, max_display_pixels) }); + // Trust the probe, not the decoder limits: `Limits::reserve` checks the + // decoder's NATIVE bytes-per-pixel, so a grayscale/RGB JPEG/PNG/BMP over + // the RGBA budget decodes fine and then `into_rgba8()` balloons to up to + // 4x it — exactly the tab death this exists to prevent. The probed + // dimensions already prove it cannot fit; refuse without decoding. + if tier == DecodeTier::OverBudget { + return DecodeAttempt::Refused { + width: probed.map_or(0, |(width, _)| width), + height: probed.map_or(0, |(_, height)| height), + }; + } + let mut reader = match open().and_then(|reader| reader.with_guessed_format()) { Ok(reader) => reader, Err(_) => return DecodeAttempt::Failed, }; - // The decoder enforces the decode budget itself (`Limits::reserve` - // checks the output allocation up front), so a too-large image errors - // here instead of allocating first. This also RAISES the image crate's - // 512 MiB default on native, where the budget is 4 GB. + // For formats that FIT the budget, the limits still guard runaway + // allocations mid-decode. TIFF additionally stacks its own intermediate + // scratch buffers on top of the reserve (empirically ~3x the frame size + // in image 0.25), so refusing an in-budget TIFF at face value would + // produce a note claiming e.g. "400 MB, over the 512 MB limit". + let tiff = matches!(reader.format(), Some(image::ImageFormat::Tiff)); let mut limits = image::Limits::default(); - limits.max_alloc = Some(max_decode_bytes); + limits.max_alloc = Some(if tiff { + max_decode_bytes.saturating_mul(3) + } else { + max_decode_bytes + }); reader.limits(limits); let img = match reader.decode() { Ok(img) => img, @@ -855,7 +873,7 @@ fn refusal_message(path: &str, width: u32, height: u32) -> String { .file_name() .map(|name| name.to_string_lossy().into_owned()) .unwrap_or_else(|| path.to_string()); - let decoded_mb = width as u64 * height as u64 * 4 / (1024 * 1024); + let decoded_mb = (width as u64).saturating_mul(height as u64).saturating_mul(4) / (1024 * 1024); let limit_mb = MAX_DECODE_BYTES / (1024 * 1024); crate::tf!( "\"{name}\" is {width}×{height} px and would decode to {decoded} MB, over the {limit} MB image limit of this build — it cannot be attached. Images within the limit are attached at a reduced display resolution when needed.", @@ -876,6 +894,17 @@ fn refusal_notes() -> &'static Mutex> { } /// Take and clear every recorded decode refusal (most recent last). +/// Peek without draining — callers that only want THEIR note (the pick +/// path) must not consume refusals queued for other references. +pub fn decode_refusals() -> Vec { + refusal_notes() + .lock() + .unwrap_or_else(|error| error.into_inner()) + .clone() +} + +/// Take and clear — for the drains that own every queued note (the poll and +/// the open-settle paths). pub fn take_decode_refusals() -> Vec { let mut notes = refusal_notes().lock().unwrap_or_else(|error| error.into_inner()); std::mem::take(&mut *notes) diff --git a/src/scene/text/web_font.rs b/src/scene/text/web_font.rs index 51eb44fce..7fa1a78dd 100644 --- a/src/scene/text/web_font.rs +++ b/src/scene/text/web_font.rs @@ -253,7 +253,7 @@ pub fn script_of(ch: char) -> Option