From 083080f1113bcc724ff3e868b5239cd9ebf0880a Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sun, 5 Jul 2026 17:23:18 -0400 Subject: [PATCH] Validate PyO3 buffer dimensions and fix mono OKLab midpoint comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding 1 (behavior/safety): the PyO3 `dither_image` entry point ignored the caller's `height` argument and let `ImageBuffer::new` derive height by integer division. In a release build a caller passing a `width` that does not match the byte-buffer length would silently truncate trailing pixels instead of erroring. The entry point now validates that the pixel buffer length equals width x height x 3 (flat RGB) and raises `PyValueError` on mismatch. A binding-level pytest exercises both the mismatch and the happy path. The wasm binding takes no `height` argument (dimensions are derived from width alone) and so has no equivalent ignored-parameter issue; it is left unchanged. Finding 2 (docs): two comments in algorithms.rs claimed the mono decision midpoint "sits at OKLab L=0.5 ≈ sRGB 188". That conflates linear 0.5 (which sRGB-encodes to ~188) with OKLab L=0.5, whose neutral-gray sRGB value is ~100. Corrected both comments to ≈ sRGB 100. Comments only; no logic changed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR --- packages/python/src/lib.rs | 17 ++++++++++++++++- packages/python/tests/test_dithering.py | 21 +++++++++++++++++++++ packages/rust/core/src/algorithms.rs | 4 ++-- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/packages/python/src/lib.rs b/packages/python/src/lib.rs index 9a4a0bb..b7e9aa2 100644 --- a/packages/python/src/lib.rs +++ b/packages/python/src/lib.rs @@ -56,7 +56,22 @@ fn dither_image( tone: Option, gamut: Option, ) -> PyResult> { - let _ = height; + // Validate the buffer against the caller's dimensions instead of silently + // deriving height and truncating trailing pixels on a width/length mismatch. + // Layout is flat RGB: len = width × height × 3. + let expected = width + .checked_mul(height) + .and_then(|n| n.checked_mul(3)) + .ok_or_else(|| PyValueError::new_err("width × height × 3 overflows usize"))?; + if pixels.len() != expected { + return Err(PyValueError::new_err(format!( + "pixel buffer length ({}) does not match width × height × 3 ({} × {} × 3 = {})", + pixels.len(), + width, + height, + expected, + ))); + } let img = ImageBuffer::new(pixels, width); let config = DitherConfig { mode: parse_mode(mode_id)?, diff --git a/packages/python/tests/test_dithering.py b/packages/python/tests/test_dithering.py index 1a17ba7..b45a706 100644 --- a/packages/python/tests/test_dithering.py +++ b/packages/python/tests/test_dithering.py @@ -63,6 +63,27 @@ def test_output_contains_only_valid_indices(self, scheme): assert max_idx < scheme.color_count, f"{scheme.name}: pixel index {max_idx} >= color count {scheme.color_count}" +class TestBufferValidation: + """Test the low-level `_rs.dither_image` validates buffer dimensions.""" + + def test_mismatched_dimensions_raise(self): + """A pixel buffer whose length != width * height * 3 must raise, not truncate.""" + import epaper_dithering._rs as _rs + + # 10x10 RGB image = 300 bytes; claim a height that does not match. + pixels = bytes(10 * 10 * 3) + with pytest.raises(ValueError): + _rs.dither_image(pixels, 10, 11, scheme_id=ColorScheme.MONO.value) + + def test_matching_dimensions_succeed(self): + """The correct width/height is accepted.""" + import epaper_dithering._rs as _rs + + pixels = bytes(10 * 10 * 3) + indices = _rs.dither_image(pixels, 10, 10, scheme_id=ColorScheme.MONO.value) + assert len(indices) == 10 * 10 + + class TestColorScience: """Test color science improvements.""" diff --git a/packages/rust/core/src/algorithms.rs b/packages/rust/core/src/algorithms.rs index a67e996..19e4d29 100644 --- a/packages/rust/core/src/algorithms.rs +++ b/packages/rust/core/src/algorithms.rs @@ -499,7 +499,7 @@ mod tests { /// in the mid-tone region and assert the max/min ratio stays modest. Empirically the /// linear-space implementation produces ratio ≈ 13.5; sRGB-space ≈ 2.2. /// - /// The mono decision midpoint sits at OKLab L=0.5 ≈ sRGB 188, so the top two bands + /// The mono decision midpoint sits at OKLab L=0.5 ≈ sRGB 100, so the top two bands /// (sRGB ≳ 192) are already in the highlight roll-off where dither activity legitimately /// tapers; we exclude them along with the pure-black first band. #[test] @@ -530,7 +530,7 @@ mod tests { } // Mid-tone bands: skip the pure-black first band and the top two near-white bands - // (past the mono midpoint ≈ sRGB 188) where the threshold clamps and dither + // (past the mono midpoint ≈ sRGB 100) where the threshold clamps and dither // activity legitimately falls off. let mid = &transitions[1..BANDS - 2]; let max = *mid.iter().max().unwrap();