fix(redact): rotated and edge-clipped marks fail closed, gray JPEGs decode

Review findings, each reproduced before the fix:
- row_hits built each transformed row's bounding box and skipped it on a
  strict two-axis overlap; a 90-degree cm makes every row vertical (zero
  X extent), so every row was skipped and the original JPEG survived
  the mark untouched behind the overlay. A degenerate row now falls
  through to the per-pixel test; regression: a rotated scan is cleared.
- a mark that clips an image's edge without covering a pixel centre
  left the image as-is; both the JPEG and raw paths now remove such an
  image whole instead (the mark touches it, fail closed). Regression:
  an edge-sliver mark removes the image.
- identity_decode was checked against three components unconditionally,
  so a DeviceGray scan with an explicit /Decode [0 1] was removed whole.
  The check now uses the image's own component count. Regressions: gray
  with identity Decode is cleared; inverted Decode still fails closed.
- the JPEG header's dimensions are now read before the sample
  allocation, so a dictionary that lies small cannot decode a huge JPEG
  past MAX_PIXELS; mismatches fail closed as before.

46 redact tests green; clippy/fmt clean; wasm32 check passes.
This commit is contained in:
skylar taylor-barrick
2026-10-14 16:18:01 -06:00
parent 685258d369
commit 7dc94ce431
2 changed files with 138 additions and 6 deletions
+51 -5
View File
@@ -69,11 +69,20 @@ fn filter_chain(s: &Stream) -> Option<Vec<Vec<u8>>> {
/// Is a pixel's sample row, at row `j` of `h`, anywhere near a region? The image is walked row by
/// row, so the whole rows outside every region are skipped without touching a pixel.
///
/// A degenerate row (zero extent on either axis — a 90° rotation makes every row vertical, a
/// flip makes them horizontal, skew can collapse one) must *not* be skipped: `overlaps` is
/// strict in both axes, so a zero-width box would fail it and the mark would be silently ignored
/// while the image survived untouched. When either axis of the transformed row is degenerate the
/// conservative answer is "run the per-pixel test".
fn row_hits(ctm: &Matrix, h: usize, j: usize, rects: &[[f64; 4]]) -> bool {
let v = 1.0 - (j as f64 + 0.5) / h as f64;
let (ax, ay) = ctm.apply(0.0, v);
let (bx, by) = ctm.apply(1.0, v);
let row = [ax.min(bx), ay.min(by), ax.max(bx), ay.max(by)];
if (row[2] - row[0]).abs() < f64::EPSILON || (row[3] - row[1]).abs() < f64::EPSILON {
return true;
}
rects.iter().any(|r| overlaps(*r, [row[0], row[1] - 0.01, row[2], row[3] + 0.01], 0.0))
}
@@ -111,6 +120,22 @@ struct Jpeg {
ncomp: usize,
}
/// Does any region's rectangle intersect the image's transformed unit square? (Conservative:
/// a mark that only clips the square's edge still counts — the per-pixel centre test may find
/// no sample centre inside such a sliver, and an image the mark touches must not survive as-is.)
fn image_touched(ctm: &Matrix, rects: &[[f64; 4]]) -> bool {
let corners = [(0.0, 0.0), (1.0, 0.0), (0.0, 1.0), (1.0, 1.0)];
let mut box_ = [f64::INFINITY, f64::INFINITY, f64::NEG_INFINITY, f64::NEG_INFINITY];
for (u, v) in corners {
let (x, y) = ctm.apply(u, v);
box_[0] = box_[0].min(x);
box_[1] = box_[1].min(y);
box_[2] = box_[2].max(x);
box_[3] = box_[3].max(y);
}
rects.iter().any(|r| overlaps(*r, box_, 0.0))
}
fn jpeg_clear(doc: &Document, s: &Stream, ctm: &Matrix, rects: &[[f64; 4]], w: usize, h: usize) -> Option<Option<Stream>> {
let Jpeg { mut px, space, ncomp } = decode_jpeg(doc, s, w, h)?;
let mut cleared = 0usize;
@@ -130,6 +155,14 @@ fn jpeg_clear(doc: &Document, s: &Stream, ctm: &Matrix, rects: &[[f64; 4]], w: u
}
}
if cleared == 0 {
// A mark that intersects the image (its unit square) but clears no pixel centre — an
// edge-only overlap, or a mark thinner than one pixel footprint — must not leave the
// original untouched: the overlay hides it visually, but the samples survive. Fail
// closed; the caller removes the image whole. (A mark entirely outside the image is
// the legitimate `Ok(None)` case.)
if image_touched(ctm, rects) {
return None;
}
return Some(None);
}
let encoded = encode_jpeg(&px, w, h, space)?;
@@ -159,15 +192,23 @@ fn decode_jpeg(doc: &Document, s: &Stream, w: usize, h: usize) -> Option<Jpeg> {
3 => ColorType::Rgb8,
_ => return None,
};
if !identity_decode(doc, s, 3) {
// `/Decode` must match the *image's* component count (one for DeviceGray, three for RGB):
// a gray scan with an identity `/Decode [0 1]` would otherwise fail the check and be
// removed whole — the very blanking this fix exists to prevent.
if !identity_decode(doc, s, if space == ColorType::L8 { 1 } else { 3 }) {
return None;
}
// The JPEG header is inspected *before* the sample allocation: a dictionary that declares a
// small image but carries a much larger JPEG must not be decoded first — the crate's
// `MAX_PIXELS` cap is enforced against the header, and only then are the samples decoded.
let reader = image::ImageReader::with_format(std::io::Cursor::new(s.raw.as_ref()), ImageFormat::Jpeg);
let (iw, ih) = reader.into_dimensions().ok()?;
if (iw as usize, ih as usize) != (w, h) {
return None;
}
let reader = image::ImageReader::with_format(std::io::Cursor::new(s.raw.as_ref()), ImageFormat::Jpeg);
let decoded = reader.decode().ok()?;
let (iw, ih) = (decoded.width() as usize, decoded.height() as usize);
if iw != w || ih != h {
return None;
}
debug_assert_eq!((decoded.width() as usize, decoded.height() as usize), (w, h));
let (px, ncomp) = match space {
ColorType::L8 => (decoded.into_luma8().into_raw(), 1),
_ => (decoded.into_rgb8().into_raw(), 3),
@@ -228,6 +269,11 @@ fn raw_clear(doc: &Document, s: &Stream, ctm: &Matrix, rects: &[[f64; 4]], w: us
}
}
if cleared == 0 {
// Same fail-closed rule as the JPEG path: a mark that touches this image but whose
// rectangle covers no sample centre (an edge sliver) removes the image whole.
if image_touched(ctm, rects) {
return None;
}
return Some(None);
}
let mut dict = s.dict.clone();
+87 -1
View File
@@ -959,7 +959,7 @@ fn a_scanned_page_keeps_its_image_and_loses_the_text_under_the_mark() {
// image — pixels 16–23 on both axes; the text mark covers R1234, x 20–40.
let img = stream(
"/Type /XObject /Subtype /Image /Width 40 /Height 40 /ColorSpace /DeviceRGB /BitsPerComponent 8 /Filter /DCTDecode",
&jpeg_white(40, 40),
jpeg_white(40, 40),
);
let src = b"q 100 0 0 100 10 10 cm /Im1 Do Q BT /F1 10 Tf 10 200 Td (OCR1234) Tj ET";
let mut doc = one_page(src, "/XObject << /Im1 7 0 R >>", vec![img]);
@@ -996,6 +996,92 @@ fn a_scanned_page_keeps_its_image_and_loses_the_text_under_the_mark() {
assert!(!content(&saved, 0).contains("0031"));
}
/// A redaction mark over a **rotated** scan must still clear the covered pixels. `row_hits` builds
/// the bounding box of each transformed sample row; with a 90° `cm`, every row is vertical (zero
/// X extent) and `overlaps` — strict in both axes — used to skip them all, leaving the original
/// JPEG (and its content under the mark) untouched while the overlay made it *look* redacted.
/// The row skip must be conservative for degenerate rows on either axis.
#[test]
fn a_rotated_scan_is_still_cleared_under_the_mark() {
// The same 40×40 white JPEG, drawn 100×100 at (10,10) but rotated 90°:
// `q 0 100 -100 0 110 10 cm` maps the image square to (10,10)-(110,110) sideways.
let img = stream(
"/Type /XObject /Subtype /Image /Width 40 /Height 40 /ColorSpace /DeviceRGB /BitsPerComponent 8 /Filter /DCTDecode",
jpeg_white(40, 40),
);
let src = b"q 0 100 -100 0 110 10 cm /Im1 Do Q";
let mut doc = one_page(src, "/XObject << /Im1 7 0 R >>", vec![img]);
mark(&mut doc, 0, &[[50.0, 50.0, 70.0, 70.0]], "");
let r = apply(&mut doc, None).unwrap();
assert_eq!((r.images_removed, r.images_cleared), (0, 1), "the rotated scan is cleared in place, never removed: {r:?}");
let (w, h, px) = page_image(&doc, 0).expect("the cleared image copy is there");
assert_eq!((w, h), (40, 40));
let at = |i: u32, j: u32| &px[((j * 40 + i) * 3) as usize..][..3];
// The mark covers 20 pt = 8 image pixels; under the rotated CTM the covered sample block is
// still the middle 8×8 (the rotation is about the square's centre), so the same pixels must
// be black now as in the unrotated case.
for (i, j) in [(16, 16), (20, 20), (23, 23), (18, 21)] {
assert!(at(i, j).iter().all(|&b| b < 96), "the marked pixels are black at ({i},{j}): {:?}", at(i, j));
}
for (i, j) in [(0, 0), (39, 0), (0, 39), (39, 39), (2, 20), (37, 20)] {
assert!(at(i, j).iter().all(|&b| b > 160), "the rest of the scan survives at ({i},{j}): {:?}", at(i, j));
}
}
/// A grayscale scan with an explicit identity `/Decode [0 1]` is cleared in place too — the
/// component count of the `/Decode` check must be the image's own (one), not three, or every
/// explicit-declaration gray scan would be removed whole and the page blanked again.
#[test]
fn a_gray_scan_with_an_identity_decode_is_cleared_not_removed() {
let mut gray = vec![200u8; 20 * 20];
gray[..20].fill(20);
let mut jpeg = Vec::new();
::image::codecs::jpeg::JpegEncoder::new_with_quality(&mut jpeg, 90).encode(&gray, 20, 20, ::image::ExtendedColorType::L8).unwrap();
let img = stream(
"/Type /XObject /Subtype /Image /Width 20 /Height 20 /ColorSpace /DeviceGray /BitsPerComponent 8 /Decode [0 1] /Filter /DCTDecode",
&jpeg,
);
let mut doc = one_page(b"q 100 0 0 100 10 10 cm /Im0 Do Q", "/XObject << /Im0 7 0 R >>", vec![img]);
mark(&mut doc, 0, &[[40.0, 40.0, 60.0, 60.0]], "");
let r = apply(&mut doc, None).unwrap();
assert_eq!((r.images_removed, r.images_cleared), (0, 1), "the gray scan is cleared, not removed: {r:?}");
}
/// A gray scan with an *inverted* `/Decode [1 0]` still fails closed: cleared samples would paint
/// white on an inverted image, so the whole image goes instead.
#[test]
fn a_gray_scan_with_an_inverted_decode_still_fails_closed() {
let gray = vec![200u8; 20 * 20];
let mut jpeg = Vec::new();
::image::codecs::jpeg::JpegEncoder::new_with_quality(&mut jpeg, 90).encode(&gray, 20, 20, ::image::ExtendedColorType::L8).unwrap();
let img = stream(
"/Type /XObject /Subtype /Image /Width 20 /Height 20 /ColorSpace /DeviceGray /BitsPerComponent 8 /Decode [1 0] /Filter /DCTDecode",
&jpeg,
);
let mut doc = one_page(b"q 100 0 0 100 10 10 cm /Im0 Do Q", "/XObject << /Im0 7 0 R >>", vec![img]);
mark(&mut doc, 0, &[[40.0, 40.0, 60.0, 60.0]], "");
let r = apply(&mut doc, None).unwrap();
assert_eq!(r.images_removed, 1, "the inverted scan is removed whole: {r:?}");
assert!(!content(&doc, 0).contains("/Im0"));
}
/// A mark that only clips the image's edge — too thin to contain any pixel centre — must not
/// leave the original image untouched behind the overlay: the image is removed whole instead.
#[test]
fn an_edge_sliver_mark_fails_closed_instead_of_leaving_the_image() {
let img = stream(
"/Type /XObject /Subtype /Image /Width 40 /Height 40 /ColorSpace /DeviceRGB /BitsPerComponent 8 /Filter /DCTDecode",
jpeg_white(40, 40),
);
// Drawn at (10,10)-(110,110): a mark from x=10 to x=10.5 overlaps the first column's
// footprint, but its centre (x=11.25) is outside — no pixel would be cleared.
let mut doc = one_page(b"q 100 0 0 100 10 10 cm /Im1 Do Q", "/XObject << /Im1 7 0 R >>", vec![img]);
mark(&mut doc, 0, &[[10.0, 10.0, 10.5, 110.0]], "");
let r = apply(&mut doc, None).unwrap();
assert_eq!((r.images_removed, r.images_cleared), (1, 0), "an image the mark touches is never left as-is: {r:?}");
}
/// An image whose codec PdfCraft cannot clear in place (a JBIG2 fax scan, say) still fails closed:
/// the whole image goes, rather than the mark being quietly ignored.
#[test]