From 392406eb4ad488bddacb9eae12ad73b987e13a30 Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Sat, 17 Oct 2026 21:50:58 +0200 Subject: [PATCH 1/3] fix(content): read a comment at the end of a content stream as the renderer does (#3081) Pieces::join separated a page's content streams with a newline, which ended a comment left open at the end of one stream. hayro, MuPDF and Poppler join them with a space, so the comment runs on into the next stream and hides its first line. Edit text, object_list, the redaction audit, redaction and the Office/HTML export therefore saw text and boxes that nothing draws, and redacting unrelated text on the page made the hidden content visible: the rewritten stream dropped the comment. - Pieces::join uses a space, so every reader of the joined streams agrees with rendering. - The parser records, per piece, whether a top-level comment runs into it and out of it (boundaries only, not every comment). A redaction rewrite of a piece ends an incoming comment before its first operator and leaves an outgoing one open, so hidden content stays hidden. - Reading for export parses the streams joined, instead of one by one. - The redaction tag check compares the joined streams the same way. - Pieces::piece_of uses a binary search. --- crates/audit/src/tests.rs | 19 +++++++ crates/content/src/lib.rs | 72 +++++++++++++++++++++++-- crates/content/src/tests.rs | 105 ++++++++++++++++++++++++++++++++++++ crates/edit/src/text.rs | 14 ++++- crates/redact/src/interp.rs | 16 +++++- crates/redact/src/lib.rs | 3 +- crates/redact/src/tests.rs | 39 ++++++++++++++ 7 files changed, 258 insertions(+), 10 deletions(-) diff --git a/crates/audit/src/tests.rs b/crates/audit/src/tests.rs index ed36a8d5e..64ea05bde 100644 --- a/crates/audit/src/tests.rs +++ b/crates/audit/src/tests.rs @@ -500,3 +500,22 @@ fn forms_nested_past_depth_limit_fail_closed() { let result = audit_redactions(&open(nested_forms(33))); assert!(matches!(result, Err(crate::AuditError::TooDeep)), "{result:?}"); } + +#[test] +fn comment_across_streams_neither_invents_text_nor_a_cover() { + let doc = open(two_streams(b"BT /F1 24 Tf 72 700 Td (Visible text) Tj ET % note", b"0 g 70 690 200 40 re f")); + assert!(audit_redactions(&doc).unwrap().is_empty(), "commented cover must not be reported"); + let doc = open(two_streams(b"BT /F1 24 Tf 72 700 Td (Before comment) Tj ET % note", b"BT /F1 24 Tf 72 650 Td (After comment) Tj ET")); + let text: String = pdfcraft_edit::text_lines(&doc, 0).unwrap().into_iter().map(|line| line.text).collect(); + assert!(!text.contains("After comment"), "editor exposed phantom text: {text}"); +} + +#[test] +fn reading_blocks_respect_stream_comments_and_split_operands() { + let doc = open(two_streams(b"BT /F1 24 Tf 72 700 Td (Before comment) Tj ET % note", b"BT /F1 24 Tf 72 650 Td (After comment) Tj ET")); + let text: String = pdfcraft_edit::reading_blocks(&doc, 0).unwrap().into_iter().map(|block| block.editor_text).collect(); + assert_eq!(text, "Before comment"); + let doc = open(two_streams(b"BT /F1 24 Tf 72 700 Td (Split operand)", b"Tj ET")); + let text: String = pdfcraft_edit::reading_blocks(&doc, 0).unwrap().into_iter().map(|block| block.editor_text).collect(); + assert_eq!(text, "Split operand"); +} diff --git a/crates/content/src/lib.rs b/crates/content/src/lib.rs index 32334e047..c7c8bca9e 100644 --- a/crates/content/src/lib.rs +++ b/crates/content/src/lib.rs @@ -60,6 +60,10 @@ pub struct Parsed { pub ops: Vec, /// Bytes that could not be read as operands or operators. pub skipped: usize, + /// For [`Pieces::parse`]: per piece, whether a comment runs into it from the piece before + /// and whether one runs out of it into the next. Only boundaries are recorded, so the size + /// depends on the number of pieces, not of comments. + comment_edges: Vec<(bool, bool)>, } fn is_ws(b: u8) -> bool { @@ -72,12 +76,25 @@ fn is_delim(b: u8) -> bool { /// Parse a (decoded) content stream. pub fn parse(data: &[u8]) -> Parsed { - let mut out = Parsed::default(); + parse_pieces(data, &[]) +} + +/// Parse `data`, recording which of the piece boundaries `ends` (as in [`Pieces`]) a comment +/// crosses. +fn parse_pieces(data: &[u8], ends: &[usize]) -> Parsed { + let mut out = Parsed { comment_edges: vec![(false, false); ends.len()], ..Parsed::default() }; + let mut boundary = 0; let mut lx = Lexer::new(data, 0); let mut operands: Vec = Vec::new(); let mut start: Option = None; loop { + let trivia = lx.pos; lx.skip_ws(); + // Only the whitespace the lexer skips here holds top-level comments: a `%` inside a + // string, a name, an array or dictionary operand or inline image data is not one. + if boundary < ends.len() { + comment_edges(data, trivia..lx.pos, ends, &mut boundary, &mut out.comment_edges); + } let Some(&b) = data.get(lx.pos) else { break }; let at = lx.pos; let operand_start = matches!(b, b'/' | b'(' | b'<' | b'[' | b'+' | b'-' | b'.') || b.is_ascii_digit(); @@ -148,6 +165,39 @@ pub fn parse(data: &[u8]) -> Parsed { out } +/// Record the piece boundaries crossed by the comments in `trivia` (skipped whitespace and +/// comments). `boundary` is the first boundary not yet passed; boundaries only move forward. +fn comment_edges(data: &[u8], trivia: std::ops::Range, ends: &[usize], boundary: &mut usize, edges: &mut [(bool, bool)]) { + let mut at = trivia.start; + while at < trivia.end { + if data.get(at) != Some(&b'%') { + at += 1; + continue; + } + let comment = at; + while at < trivia.end && !matches!(data.get(at), Some(b'\r' | b'\n')) { + at += 1; + } + // The separator of piece `boundary` is the last byte before its end. + while let Some(&end) = ends.get(*boundary) { + let separator = end.saturating_sub(1); + if separator >= at { + break; + } + if separator >= comment { + if let Some(edge) = edges.get_mut(*boundary) { + edge.1 = true; + } + // Includes a comment whose end of line is the next piece's first byte. + if let Some(edge) = edges.get_mut(boundary.saturating_add(1)) { + edge.0 = true; + } + } + *boundary += 1; + } + } +} + /// Read an inline image starting after `BI`: (parameters, data, position after `EI`). `Err` holds /// the position after the `EI` (or the end of the stream) of an image whose parameters can't be read. fn inline_image(data: &[u8], pos: usize) -> Result<(Dict, Vec, usize), usize> { @@ -305,7 +355,9 @@ pub fn serialize_ops(ops: &[Op]) -> Vec { /// A page's content streams joined into the one stream they are. Writers may split it between /// any two tokens (ISO 32000-2 §7.8.2), so an operator's operands can end one piece and its /// keyword start the next: parse the pieces joined ([`Pieces::parse`]) and map positions back -/// with [`Pieces::piece_of`]. A newline separates the pieces so tokens never run together. +/// with [`Pieces::piece_of`]. A space separates the pieces, as renderers join them (hayro, +/// MuPDF and Poppler alike): tokens never run together, and a comment at the end of a piece +/// runs on into the next one, up to its first end of line. #[derive(Clone, Debug, Default, PartialEq)] pub struct Pieces { data: Vec, @@ -319,7 +371,7 @@ impl Pieces { let mut ends = Vec::with_capacity(pieces.len()); for p in pieces { data.extend_from_slice(p.as_ref()); - data.push(b'\n'); + data.push(b' '); ends.push(data.len()); } Pieces { data, ends } @@ -341,12 +393,22 @@ impl Pieces { /// Parse the pieces as one stream. pub fn parse(&self) -> Parsed { - parse(&self.data) + parse_pieces(&self.data, &self.ends) + } + + /// Per piece, from `parsed` (this piece set's [`Pieces::parse`]): whether a comment runs + /// into it from the piece before, and whether one runs out of it into the next. A writer + /// that rewrites a piece without its comments must keep both: end the incoming comment + /// before its first operator, and leave a comment open at its end. + pub fn comment_edges(&self, parsed: &Parsed) -> Vec<(bool, bool)> { + let mut edges = parsed.comment_edges.clone(); + edges.resize(self.len(), (false, false)); + edges } /// The piece holding joined position `at` (the last piece for positions past the end). pub fn piece_of(&self, at: usize) -> usize { - self.ends.iter().position(|&e| at < e).unwrap_or(self.ends.len().saturating_sub(1)) + self.ends.partition_point(|&e| e <= at).min(self.ends.len().saturating_sub(1)) } /// The pieces an operator spans: (the one its first token is in, the one its keyword is in). diff --git a/crates/content/src/tests.rs b/crates/content/src/tests.rs index 18e0b8cbc..1119bdaf9 100644 --- a/crates/content/src/tests.rs +++ b/crates/content/src/tests.rs @@ -283,3 +283,108 @@ fn pieces_parse_as_one_stream_and_splice_back() { assert_eq!(out.len(), 3); assert!(Pieces::join::<&[u8]>(&[]).splice([(0..1, b"x".to_vec())]).is_empty()); } + +#[test] +fn stream_boundary_comment_matches_space_concatenation() { + for eol in [b"\n".as_slice(), b"\r", b"\r\n"] { + let mut tail = b"(HIDDEN) Tj".to_vec(); + tail.extend(eol); + tail.extend(b"(LIVE) Tj"); + let pieces = Pieces::join(&[b"BT % comment".as_slice(), b"(ALSO HIDDEN) Tj", &tail, b"ET"]); + let parsed = pieces.parse(); + let shown: Vec<_> = parsed.ops.iter().filter(|op| op.is("Tj")).collect(); + assert_eq!(shown.len(), 1); + assert_eq!(shown[0].operands[0].as_string().unwrap().bytes, b"LIVE"); + } +} + +#[test] +fn comment_edges_terminate_at_next_piece_byte_zero() { + for eol in [b"\n".as_slice(), b"\r", b"\r\n"] { + let mut next = eol.to_vec(); + next.extend_from_slice(b"q Q"); + let pieces = Pieces::join(&[b"% outgoing".as_slice(), next.as_slice()]); + let parsed = pieces.parse(); + assert_eq!(pieces.comment_edges(&parsed), vec![(false, true), (true, false)], "EOL {eol:?}"); + assert_eq!(parsed.ops.iter().map(|op| op.op.as_slice()).collect::>(), vec![b"q".as_slice(), b"Q"]); + } +} + +#[test] +fn comment_edges_span_empty_pieces_and_end_of_content() { + let pieces = Pieces::join(&[b"q % outgoing".as_slice(), b"", b"", b"ignored\nQ"]); + let parsed = pieces.parse(); + assert_eq!(pieces.comment_edges(&parsed), vec![(false, true), (true, true), (true, true), (true, false)]); + assert_eq!(parsed.ops.iter().map(|op| op.op.as_slice()).collect::>(), vec![b"q".as_slice(), b"Q"]); + + let pieces = Pieces::join(&[b"% outgoing".as_slice(), b"", b"still ignored"]); + let parsed = pieces.parse(); + assert_eq!(pieces.comment_edges(&parsed), vec![(false, true), (true, true), (true, true)]); + assert!(parsed.ops.is_empty()); + + let pieces = Pieces::join::<&[u8]>(&[]); + assert!(pieces.comment_edges(&pieces.parse()).is_empty()); +} + +#[test] +fn comment_edges_ignore_percent_inside_operands_and_image_data() { + for first in [ + b"(literal % not a comment) Tj".as_slice(), + b"/Percent#25Name MP", + b"[(literal % not a comment)] TJ", + b"/P << /ActualText (literal % not a comment) >> BDC EMC", + b"BI /W 1 /H 1 /BPC 8 /CS /G ID % EI", + ] { + let pieces = Pieces::join(&[first, b"q Q"]); + let parsed = pieces.parse(); + assert_eq!(pieces.comment_edges(&parsed), vec![(false, false), (false, false)], "{first:?}"); + assert_eq!(parsed.ops.last().unwrap().op, b"Q"); + assert_eq!(parsed.skipped, 0); + } +} + +#[test] +fn compound_operand_comment_split_is_owned_by_spanning_operator() { + for parts in [[b"[(A) % ignored".as_slice(), b"still ignored\n(B)] TJ"], [b"/P << /MCID 0 % ignored".as_slice(), b"still ignored\n>> BDC"]] { + let pieces = Pieces::join(&parts); + let parsed = pieces.parse(); + // The object lexer consumes this nested comment. Its containing operator spans + // both pieces, which forces both to be rewritten together by the redactor. + assert_eq!(pieces.comment_edges(&parsed), vec![(false, false), (false, false)]); + assert_eq!(parsed.ops.len(), 1); + assert_eq!(pieces.pieces_of(&parsed.ops[0]), (0, 1)); + assert_eq!(parsed.skipped, 0); + } +} + +#[test] +fn comment_edges_do_not_confuse_local_comments_with_boundary_comments() { + let pieces = Pieces::join(&[b"% local\nq % crossing".as_slice(), b"ignored\n% local again\nQ\n% final local\n", b"q Q"]); + let parsed = pieces.parse(); + assert_eq!(pieces.comment_edges(&parsed), vec![(false, true), (true, false), (false, false)]); + assert_eq!(parsed.ops.iter().map(|op| op.op.as_slice()).collect::>(), vec![b"q".as_slice(), b"Q", b"q", b"Q"]); +} + +#[test] +fn comment_edge_storage_depends_on_piece_count_not_comment_count() { + let mut first = b"%\n".repeat(16_384); + first.extend_from_slice(b"q % crossing"); + let pieces = Pieces::join(&[first.as_slice(), b"ignored\nQ"]); + let parsed = pieces.parse(); + assert_eq!(pieces.comment_edges(&parsed), vec![(false, true), (true, false)]); + assert_eq!(parsed.ops.len(), 2); + // Also exercise public parsing without piece-boundary metadata. + assert_eq!(parse(&first).ops.len(), 1); +} + +#[test] +fn piece_lookup_preserves_exact_boundary_and_empty_piece_mapping() { + // Joined layout: qQ; separator belongs to prior piece. + let pieces = Pieces::join(&[b"q".as_slice(), b"", b"Q"]); + for (at, expected) in [(0, 0), (1, 0), (2, 1), (3, 2), (4, 2), (5, 2), (usize::MAX, 2)] { + assert_eq!(pieces.piece_of(at), expected, "position {at}"); + } + let empty = Pieces::join::<&[u8]>(&[]); + assert_eq!(empty.piece_of(0), 0); + assert_eq!(empty.piece_of(usize::MAX), 0); +} diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index 9c83cf9a3..93d1b54fc 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -1276,8 +1276,18 @@ fn reading_lines(doc: &Document, page: usize) -> Result, EditError let streams = content_streams(doc, &p.dict); let mut w = Walk { doc, lines: Vec::new(), next_stream: streams.len(), path: Vec::new(), visits: 0 }; let mut carry = Carry::new(); - for (si, (_, data)) in streams.into_iter().enumerate() { - walk(&mut w, &parse(&data).ops, &res, &mut carry, si); + // Read the page's streams as the one stream they are, as rendering and editing do: a + // comment or an operator's operands can run on from one stream into the next. Each operator + // still counts as its first stream's, which keeps lines from different streams apart. + let pieces = Pieces::join(&streams.into_iter().map(|(_, data)| data).collect::>()); + let mut grouped: Vec> = (0..pieces.len()).map(|_| Vec::new()).collect(); + for op in pieces.parse().ops { + if let Some(group) = grouped.get_mut(pieces.pieces_of(&op).0) { + group.push(op); + } + } + for (si, ops) in grouped.iter().enumerate() { + walk(&mut w, ops, &res, &mut carry, si); } let mut lines = w.lines; // A visual-order Indic font draws pre-base vowel signs before their consonant. diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index c76e11995..50802eff3 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -380,6 +380,9 @@ pub(crate) fn process(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec< // next. Parse them joined, then give each operator back to the piece its keyword is in. let joined = Pieces::join(streams); let mut parsed = joined.parse(); + // A rewritten piece loses its comments, so keep where they began and ended: a comment that + // ran on into the next piece still hides that piece's first line. + let comment_edges = joined.comment_edges(&parsed); if !scope.hidden_layers.is_empty() { let props = res_dict(doc, resources, b"Properties"); let (kept, removed) = strip_hidden_layers(doc, scope, parsed.ops, &props); @@ -405,7 +408,7 @@ pub(crate) fn process(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec< } } - for (piece, mut changed) in pieces.into_iter().zip(rewrite) { + for ((piece, mut changed), (comment_in, comment_out)) in pieces.into_iter().zip(rewrite).zip(comment_edges) { let mut ops: Vec = Vec::with_capacity(piece.len()); for op in piece { let o = op.op.as_slice(); @@ -622,7 +625,16 @@ pub(crate) fn process(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec< if !path.is_empty() { ops.append(&mut path); } - out.streams.push(changed.then(|| serialize_ops(&ops))); + out.streams.push(changed.then(|| { + // The piece before still ends inside a comment: end it before the first operator. + let mut bytes = if comment_in { b"\n".to_vec() } else { Vec::new() }; + bytes.extend(serialize_ops(&ops)); + // The piece ended inside a comment that hid the start of the next one: keep it open. + if comment_out { + bytes.push(b'%'); + } + bytes + })); } out } diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 61231bd98..21af3691c 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -400,7 +400,8 @@ pub fn apply(doc: &mut Document, pages: Option<&[usize]>) -> Result