From bf9da54e668f26a41d5149b07c743d8615f26f46 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 6 Sep 2026 17:12:54 +0200 Subject: [PATCH] test(pdf): pin the object-stream page rewrite The one case Phase 1 left open. A page dictionary compressed into an object stream is rewritten uncompressed in the appended section and the newer type-1 entry wins, which is what modern producers make the common case. No code change: it already worked, and now says so. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018e3PEzyU2oAFSzsEoWsSmz --- docs/design/pdf-annotation.md | 12 ++++---- test/src/internal/pdf/pdf_writer.cpp | 42 ++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/docs/design/pdf-annotation.md b/docs/design/pdf-annotation.md index 74a032612..a182bec57 100644 --- a/docs/design/pdf-annotation.md +++ b/docs/design/pdf-annotation.md @@ -228,10 +228,9 @@ Three things the spike did **not** settle, and Phase 1 and 2 owe tests for each: one of those engines painted — the `/QuadPoints` were never consulted. The ordering matters only to a viewer that regenerates the appearance, and to text-selection semantics. The note below stands as a note. -- **A page dictionary inside an object stream.** The fixture's was plain, and - Phase 1 did not close this either — no fixture we have puts one there. -- **Appending to a file whose newest section is an xref stream.** The fixture's - was a classic table; Phase 1's tests cover both flavors. +- **A page dictionary inside an object stream**, and **appending to a file + whose newest section is an xref stream.** The spike's fixture had neither; + Phase 1's tests cover both. ## Implementation plan @@ -277,9 +276,8 @@ identically first, then a `/Rotate` rewrite. `qpdf --check` passes, and ghostscript, CoreGraphics and our own renderer all honour the new rotation (the page box turns 8.5×11in into 11×8.5in). -**Still untested: a page dictionary living inside an object stream.** It has to -be rewritten uncompressed in the new section — legal, the newer entry wins — -but no fixture we have puts one there. Owed before Phase 2 ships. +A page dictionary living inside an object stream is rewritten uncompressed in +the new section, the newer type-1 entry winning over the older type-2 one. ### Phase 2 — highlight (2 d, ~200 lines) diff --git a/test/src/internal/pdf/pdf_writer.cpp b/test/src/internal/pdf/pdf_writer.cpp index 999f5a4d6..48040adf9 100644 --- a/test/src/internal/pdf/pdf_writer.cpp +++ b/test/src/internal/pdf/pdf_writer.cpp @@ -249,3 +249,45 @@ TEST(IncrementalWriter, rewrites_a_page_of_a_real_fixture) { } } } + +// A page dictionary compressed into an object stream is rewritten uncompressed +// in the new section; the newer type-1 entry wins over the older type-2 one. +TEST(IncrementalWriter, rewrites_a_page_out_of_an_object_stream) { + const auto file = std::make_shared( + TestData::test_file_path("odr-public/pdf/opendocument-app-website.pdf")); + + std::ostringstream out; + ObjectReference page_reference; + { + DocumentParser parser(file->stream()); + ASSERT_EQ(parser.xref_kind(), DocumentParser::XrefKind::stream); + + const std::unique_ptr document = parser.parse_document(); + const Page *page = first_page(*document); + ASSERT_NE(page, nullptr); + page_reference = page->object_reference; + // the fixture keeps its page objects in object streams + ASSERT_TRUE(parser.xref().table.at(page_reference).is_compressed()); + + IncrementalWriter writer(parser); + Dictionary rotated = page->object.as_dictionary(); + rotated["Rotate"] = Object(Integer{90}); + writer.set_object(page_reference, Object(std::move(rotated))); + writer.write(out); + } + + DocumentParser parser( + std::make_unique(std::move(out).str())); + EXPECT_TRUE(parser.xref().table.at(page_reference).is_used()); + + const std::unique_ptr document = parser.parse_document(); + const std::vector pages = document->collect_pages(); + ASSERT_EQ(pages.size(), 9); + EXPECT_EQ(pages[0]->rotate, 90); + EXPECT_EQ(pages[1]->rotate, 0); + for (const Page *page : pages) { + for (const auto &content_reference : page->contents_reference) { + EXPECT_FALSE(parser.read_decoded_stream(content_reference).empty()); + } + } +}