From 0046177f47fbba53658447b8a0897fd09dc477c3 Mon Sep 17 00:00:00 2001 From: FredApps <153173854+FredApps@users.noreply.github.com> Date: Sun, 1 Nov 2026 14:57:41 +0100 Subject: [PATCH] fix(engine): validate image transforms and promote arbitrary added-image transforms Reject finite but singular or overflowing determinants at the engine boundary. Promote added images for arbitrary transforms, as RotateBy already does, while preserving editable quarter turns and reflections. Regressions verify transformed placement, promotion, undo and refusal without mutation. Co-Authored-By: Codex GPT-6.1 --- crates/engine/src/edit/run.rs | 11 ++++++++-- crates/engine/src/tests.rs | 41 ++++++++++++++++++++++++++--------- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/crates/engine/src/edit/run.rs b/crates/engine/src/edit/run.rs index 98d7205434..5ed70aed7a 100644 --- a/crates/engine/src/edit/run.rs +++ b/crates/engine/src/edit/run.rs @@ -256,8 +256,12 @@ fn run_edit_page_image(doc: &mut pdfcraft_cos::Document, page: usize, index: usi ImageEdit::Move(to) if !to.iter().all(|v| v.is_finite()) => { return Err(EditError::Edit(pdfcraft_edit::EditError::Invalid("invalid position".into()))); } - ImageEdit::Transform(t) if !t.iter().all(|v| v.is_finite()) => { - return Err(EditError::Edit(pdfcraft_edit::EditError::Invalid("the image transform must be finite".into()))); + ImageEdit::Transform(t) => { + let [a, b, c, d, _, _] = *t; + let determinant = a * d - b * c; + if !t.iter().all(|v| v.is_finite()) || !determinant.is_finite() || determinant.abs() < 1e-9 { + return Err(EditError::Edit(pdfcraft_edit::EditError::Invalid("the image transform must be finite and nonsingular".into()))); + } } ImageEdit::Crop(keep) if !keep.iter().all(|v| v.is_finite()) => { return Err(EditError::Edit(pdfcraft_edit::EditError::Invalid("invalid crop".into()))); @@ -326,7 +330,10 @@ fn run_edit_page_image(doc: &mut pdfcraft_cos::Document, page: usize, index: usi || (v[2] - img.rect[2]).abs() > 1e-4 || (v[3] - img.rect[3]).abs() > 1e-4 }); + let non_axis_transform = matches!(change, ImageEdit::Transform([a, b, c, d, _, _]) + if !((b.abs() < 1e-5 && c.abs() < 1e-5) || (a.abs() < 1e-5 && d.abs() < 1e-5))); let should_promote = has_active_crop + || non_axis_transform || matches!(change, ImageEdit::Crop(_) | ImageEdit::RemoveCrop | ImageEdit::RotateBy(_) | ImageEdit::Arrange(_) | ImageEdit::Delete); if should_promote { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index d8a7b793fc..51a19b7b24 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -9439,7 +9439,7 @@ fn mirroring_transform_on_added_image_flips_and_non_finite_is_refused() { } #[test] -fn transform_on_added_image_folds_quarter_turns_and_rejects_non_axis_aligned() { +fn transform_on_added_image_folds_quarter_turns_and_promotes_non_axis_aligned() { let make_png = |rgb: [u8; 3]| { let mut png = Vec::new(); { @@ -9482,16 +9482,21 @@ fn transform_on_added_image_folds_quarter_turns_and_rejects_non_axis_aligned() { assert!((ai2.rect[2] - 200.0).abs() < 1e-4); assert!((ai2.rect[3] - 150.0).abs() < 1e-4); - // 3. Non-axis-aligned transforms (e.g. 45° rotation or skew) are rejected: + // Arbitrary transforms promote the added image, just like RotateBy. + let initial_matrix = s.get(id).unwrap().page_images(0)[0].matrix; let rot_45 = pdfcraft_edit::turn_by_degrees([100.0, 100.0, 200.0, 150.0], 45.0).unwrap(); - let res_45 = s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform(rot_45) }); - assert!(res_45.is_err(), "non-axis-aligned 45° rotation must be rejected for added image"); - assert!(res_45.unwrap_err().to_string().contains("non-axis-aligned")); - - let skew = [1.0, 0.5, 0.0, 1.0, 0.0, 0.0]; - let res_skew = s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform(skew) }); - assert!(res_skew.is_err(), "skew must be rejected for added image"); - assert!(res_skew.unwrap_err().to_string().contains("non-axis-aligned")); + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform(rot_45) }).unwrap(); + let cos = pdfcraft_cos::Document::open(s.get(id).unwrap().bytes.clone()).unwrap(); + assert!(pdfcraft_edit::list_added(&cos).is_empty(), "arbitrary transform promotes the image"); + assert_eq!(s.get(id).unwrap().page_images(0).len(), 1); + let expected = pdfcraft_content::Matrix(initial_matrix).then(&pdfcraft_content::Matrix(rot_45)).0; + let actual = s.get(id).unwrap().page_images(0)[0].matrix; + assert!(actual.into_iter().zip(expected).all(|(a, b)| (a - b).abs() < 1e-3), "{actual:?} != {expected:?}"); + s.undo(id).unwrap(); + let cos = pdfcraft_cos::Document::open(s.get(id).unwrap().bytes.clone()).unwrap(); + assert_eq!(pdfcraft_edit::list_added(&cos).len(), 1, "undo restores added-image editability"); + s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform([1.0, 0.5, 0.0, 1.0, 0.0, 0.0]) }).unwrap(); + assert_eq!(s.get(id).unwrap().page_images(0).len(), 1); } #[test] @@ -9988,3 +9993,19 @@ fn original_image_edit_does_not_match_an_added_sibling_of_the_same_xobject() { assert_eq!(doc.added()[0].content, before_added, "matching an original Do must not select the added sibling's metadata"); assert_eq!(doc.page_images(0)[0].rect, [40.0, 50.0, 140.0, 150.0]); } + +#[test] +fn image_transform_engine_refuses_singular_matrices_without_mutation() { + let mut png = Vec::new(); + let mut enc = png::Encoder::new(&mut png, 1, 1); + enc.set_color(png::ColorType::Rgb); + enc.write_header().unwrap().write_image_data(&[100, 150, 200]).unwrap(); + let mut s = Session::new(); + let bytes = s.create_from_images(&[("pixel.png".into(), png)]).unwrap(); + let id = s.open("image.pdf", None, bytes, None).unwrap(); + for matrix in [[0.0, 0.0, 0.0, 1.0, 0.0, 0.0], [1.0, 1.0, 1.0, 1.0 + 1e-12, 0.0, 0.0], [1e308, 0.0, 0.0, 1e308, 0.0, 0.0]] { + let before = s.get(id).unwrap().bytes.clone(); + assert!(s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform(matrix) }).is_err()); + assert_eq!(s.get(id).unwrap().bytes, before); + } +}