diff --git a/crates/render/src/raster/tests.rs b/crates/render/src/raster/tests.rs index 696eaddb61..39a489bb1d 100644 --- a/crates/render/src/raster/tests.rs +++ b/crates/render/src/raster/tests.rs @@ -7058,8 +7058,9 @@ fn a_malformed_token_does_not_end_the_content_stream() { // A `re` with three operands: the typed iterator used to abort on a path operator it // could not dispatch, which is what #3568 reports. "20 10 10 re", - // More operands than the limit (ISO 32000-2 Annex C gives 10): the instruction is - // dropped, not the stream. + // Eleven operands: under the 256 the stack holds, so this one runs on its own last + // operand. Past the limit is covered by + // `an_operator_past_the_operand_limit_reads_its_own_operands` below. "0 0 0 0 0 0 0 0 0 0 0 g", // A stray delimiter: the lexer could not read an operator and jumped to the end. ")", @@ -7080,6 +7081,35 @@ fn a_malformed_token_does_not_end_the_content_stream() { } } +/// An operator preceded by more operands than the stack holds is applied to its own operands, +/// which are the ones nearest it, rather than to whichever of the strays survived. The parser +/// had a branch meant to drop such an instruction whole, dead because `Stack::push` always +/// reported success (noted in review of #3636); reviving it loses legitimate content, which +/// `an_operator_with_hundreds_of_operands_does_not_end_the_content` measures, so the dead branch +/// is gone and this pins what the ring buffer does instead. +#[test] +fn an_operator_past_the_operand_limit_reads_its_own_operands() { + // The strays are `1` and the operator's own operand is `0`, so the two answers differ: white + // if the operator read a stray, black if it read its own. + let page = |strays: usize| { + let stream = format!("{}0 g 0 0 20 20 re f", "1 ".repeat(strays)); + let pdf = format!( + "%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 20 20] /Contents 4 0 R >> endobj\n4 0 obj << /Length {} >> stream\n{stream}\nendstream endobj\ntrailer << /Root 1 0 R >>\n%%EOF", + stream.len() + ); + let mut r = PageRenderer::new(Arc::new(pdf.into_bytes()), RenderConfig::default()); + let p = r.render(RenderRequest { page: 0, kind: RequestKind::Pixels, scale: 1.0, ..Default::default() }); + assert!(p.error.is_none(), "{strays} strays: {:?}", p.error); + p.rgba.as_chunks::<4>().0.to_vec() + }; + let all_black = |pixels: &[[u8; 4]]| pixels.iter().all(|p| *p == [0, 0, 0, 255]); + + // None, a few, the limit, and past it: the fill is the black the `g` asked for every time. + for strays in [0, 5, 255, 256, 257, 600] { + assert!(all_black(&page(strays)), "{strays} stray operands: the `g` reads its own operand"); + } +} + /// Content after an inline image whose data holds `EI` is still read. This pins the property /// but does **not** exercise the look-ahead gating added for #3636's review: accepting a false /// `EI` truncates the image, while the text after the real one is read either way, and this diff --git a/vendor/hayro-syntax/src/content/mod.rs b/vendor/hayro-syntax/src/content/mod.rs index 33c0e5bd05..1d71d73d2e 100644 --- a/vendor/hayro-syntax/src/content/mod.rs +++ b/vendor/hayro-syntax/src/content/mod.rs @@ -190,11 +190,6 @@ impl<'a> UntypedIter<'a> { self.committed = self.skip_blanks_and_comments(); } - // PdfCraft patch: set when this instruction carried more operands than the limit. Its - // operator is then dropped with them, rather than applied to the ones that did fit - // (#3568). - let mut overflowed = false; - while !self.reader.at_end() { // I believe booleans/null never appear as an operator? if matches!( @@ -212,9 +207,7 @@ impl<'a> UntypedIter<'a> { if self.undecided() { return None; } - if self.stack.push(object).is_none() { - overflowed = true; - } + self.stack.push(object); } else if self.reader.read_without_context::>().is_some() { if self.undecided() { return None; @@ -234,7 +227,6 @@ impl<'a> UntypedIter<'a> { // content was deliberately not parsed. warn!("skipped an unreadable name in content stream"); self.stack.clear(); - overflowed = false; continue; } else { return None; @@ -253,7 +245,6 @@ impl<'a> UntypedIter<'a> { // the rest of the page was lost. Step over it and carry on (#3568). if !self.lookahead && self.skip_bad_token() { self.stack.clear(); - overflowed = false; continue; } @@ -302,7 +293,7 @@ impl<'a> UntypedIter<'a> { UnfilteredEnd::Found(end_data, end_after_ei) => { let image_data = stream_data.get(..end_data)?; let stream = Stream::new(image_data, dict.clone()); - self.stack.push(Object::Stream(stream))?; + self.stack.push(Object::Stream(stream)); self.reader.read_bytes(end_after_ei)?; self.reader.skip_white_spaces(); accepted = true; @@ -464,7 +455,7 @@ impl<'a> UntypedIter<'a> { return None; } - self.stack.push(Object::Stream(stream))?; + self.stack.push(Object::Stream(stream)); self.reader.read_bytes(2)?; self.reader.skip_white_spaces(); @@ -482,17 +473,6 @@ impl<'a> UntypedIter<'a> { } } - if overflowed && !self.lookahead { - // PdfCraft patch: see `overflowed` above. ISO 32000-2 Annex C gives 10 as the - // historical operand limit; past it the instruction is dropped whole. - warn!("dropped an instruction with more operands than the limit"); - self.stack.clear(); - overflowed = false; - self.committed = self.reader.offset(); - self.reader.skip_white_spaces_and_comments(); - continue; - } - self.operator = Some(operator); self.committed = self.reader.offset(); // PdfCraft patch (28): operands in reading order, if some were dropped. @@ -717,11 +697,16 @@ impl<'a> Stack<'a> { } /// PdfCraft patch (28): push an operand, dropping the oldest once `MAX_OPERANDS` are held - /// (see `MAX_OPERANDS`). Never fails: it used to, and the failure ended the content stream. - fn push(&mut self, operand: Object<'a>) -> Option<()> { + /// (see `MAX_OPERANDS`). It cannot fail: it used to, and the failure ended the content stream. + /// + /// The operands kept are the newest, so the ones nearest the operator -- which are its own. + /// No operator takes anywhere near `MAX_OPERANDS` of them (`scn` takes one per colourant, and + /// ISO 32000 allows a DeviceN space 32), so an instruction preceded by stray operands is + /// applied to its own and the strays fall off the back. + fn push(&mut self, operand: Object<'a>) { if self.data.len() < MAX_OPERANDS { self.data.push(operand); - return Some(()); + return; } self.dropped = true; @@ -729,7 +714,6 @@ impl<'a> Stack<'a> { *slot = operand; } self.oldest = (self.oldest + 1) % MAX_OPERANDS; - Some(()) } /// PdfCraft patch (28): put the operands back in the order they were read, oldest first,