From 39f2a0d3422364c1c5c389b0619d595160846ea0 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Thu, 20 Aug 2026 10:19:25 +0200 Subject: [PATCH] fix(pdf): read a literal string with nested parentheses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ISO 32000-1 7.3.4.2 lets a balanced pair of parentheses stand unescaped inside a literal string, so the reader now tracks nesting depth and ends the string only on the `)` closing the outermost pair. `cairo` and `pdfTeX` write their `/Producer` and `/Creator` that way; those files lost their document metadata to the best-effort catch in `pdf_file.cpp`, and a catalog reached through such a string made the whole render fail. Also give `ViewStream` the seeking it was documented not to have: `FileParser::read_object_stream` addresses each member by absolute position, and a silently failing `seekg` left it reading them in order — correct only while the members are laid out as the header lists them. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XDs5aK3ZGSZsEvqUUwBBXU --- CHANGELOG.md | 3 ++ src/odr/internal/pdf/pdf_object_parser.cpp | 14 ++++++- src/odr/internal/util/stream_util.cpp | 30 ++++++++++++++- test/CMakeLists.txt | 1 + test/data.cmake | 6 +-- test/src/internal/pdf/pdf_object_parser.cpp | 12 ++++++ test/src/internal/util/stream_util_test.cpp | 42 +++++++++++++++++++++ 7 files changed, 102 insertions(+), 6 deletions(-) create mode 100644 test/src/internal/util/stream_util_test.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index d436ed030..0294b38e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- A pdf that nests parentheses inside a string opens, and keeps its document + metadata — `cairo` and `pdfTeX` write their `/Producer` that way. + ## v6.9.0 - 2026-08-18 - A filled pdf form shows what was filled in, and a marked-up one its markup: diff --git a/src/odr/internal/pdf/pdf_object_parser.cpp b/src/odr/internal/pdf/pdf_object_parser.cpp index 638b8d9d2..9859d1f16 100644 --- a/src/odr/internal/pdf/pdf_object_parser.cpp +++ b/src/odr/internal/pdf/pdf_object_parser.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -393,6 +394,10 @@ std::variant ObjectParser::read_string() { char_type c = bumpc(); if (c == '(') { + // 7.3.4.2: a balanced pair of parentheses inside a literal string needs no + // escaping, so only the `)` closing the outermost pair ends the string. + std::uint32_t depth = 1; + while (true) { c = bumpc(); @@ -436,8 +441,13 @@ std::variant ObjectParser::read_string() { } continue; } - if (c == ')') { - return StandardString(std::move(string)); + if (c == '(') { + ++depth; + } else if (c == ')') { + --depth; + if (depth == 0) { + return StandardString(std::move(string)); + } } string += c; diff --git a/src/odr/internal/util/stream_util.cpp b/src/odr/internal/util/stream_util.cpp index 477831f94..1a969a1f6 100644 --- a/src/odr/internal/util/stream_util.cpp +++ b/src/odr/internal/util/stream_util.cpp @@ -115,7 +115,7 @@ namespace odr::internal::util::stream { namespace { /// Read-only stream buffer over an existing `string_view`, which must outlive -/// it. No seeking. +/// it. class ViewStreamBuf : public std::streambuf { public: explicit ViewStreamBuf(std::string_view view) { @@ -123,6 +123,34 @@ class ViewStreamBuf : public std::streambuf { auto *begin = const_cast(view.data()); setg(begin, begin, begin + view.size()); } + +protected: + pos_type seekoff(const off_type off, const std::ios_base::seekdir dir, + const std::ios_base::openmode which) override { + if ((which & std::ios_base::in) == 0) { + return pos_type(off_type(-1)); + } + + off_type position = off; + if (dir == std::ios_base::cur) { + position += gptr() - eback(); + } else if (dir == std::ios_base::end) { + position += egptr() - eback(); + } else if (dir != std::ios_base::beg) { + return pos_type(off_type(-1)); + } + + if (position < 0 || position > egptr() - eback()) { + return pos_type(off_type(-1)); + } + setg(eback(), eback() + position, egptr()); + return position; + } + + pos_type seekpos(const pos_type pos, + const std::ios_base::openmode which) override { + return seekoff(pos, std::ios_base::beg, which); + } }; } // namespace diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 4e8b5b10a..62a8e540d 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -101,6 +101,7 @@ add_executable(odr_test "src/internal/util/map_util_test.cpp" "src/internal/util/number_util_test.cpp" + "src/internal/util/stream_util_test.cpp" "src/internal/util/string_util_test.cpp" "src/internal/util/xml_util_test.cpp" diff --git a/test/data.cmake b/test/data.cmake index f925e7d64..2196cd61d 100644 --- a/test/data.cmake +++ b/test/data.cmake @@ -7,7 +7,7 @@ odr_test_data( PATH "input/odr-public" URL "https://github.com/opendocument-app/OpenDocument.test.git" - REVISION "1ad8965bfc65529a715d5b0e39743292e879329e") + REVISION "3a5eaa1e559e420ee620d890937e0b11a64a294b") odr_test_data( PATH "input/odr-private" @@ -17,9 +17,9 @@ odr_test_data( odr_test_data( PATH "reference-output/odr-public" URL "https://github.com/opendocument-app/OpenDocument.test.output.git" - REVISION "c667f1d619e31909ec81effe9148c79283291c01") + REVISION "c7258ff998e22aaedf514ef2c1475dc3c22d0a96") odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "fabb5651c6406570b735cf6acd1213a14f5b390c") + REVISION "08b933720c39706541d2ba74f62e248d3d2a8b21") diff --git a/test/src/internal/pdf/pdf_object_parser.cpp b/test/src/internal/pdf/pdf_object_parser.cpp index 9db6d2d1f..3bac2db02 100644 --- a/test/src/internal/pdf/pdf_object_parser.cpp +++ b/test/src/internal/pdf/pdf_object_parser.cpp @@ -242,6 +242,18 @@ TEST(PdfObjectParser, standard_string_literal_escapes) { EXPECT_EQ(read_standard_string("(a\\qb)"), "aqb"); } +// 7.3.4.2: a balanced pair of parentheses needs no escaping, so only the `)` +// closing the outermost pair ends the string. `cairo` writes its `/Producer` +// this way. +TEST(PdfObjectParser, standard_string_balanced_parentheses) { + EXPECT_EQ(read_standard_string("(a (b) c)"), "a (b) c"); + EXPECT_EQ(read_standard_string("(a (b (c)) d)"), "a (b (c)) d"); + EXPECT_EQ(read_standard_string("(cairo 1.18.4 (https://cairographics.org))"), + "cairo 1.18.4 (https://cairographics.org)"); + // an escaped parenthesis does not open or close a pair + EXPECT_EQ(read_standard_string("(a \\(b (c) d)"), "a (b (c) d"); +} + // 7.3.4.2: a `\ddd` octal escape (1-3 digits) is the byte of that value. TEST(PdfObjectParser, standard_string_octal_escape) { EXPECT_EQ(read_standard_string("(\\101)"), "A"); diff --git a/test/src/internal/util/stream_util_test.cpp b/test/src/internal/util/stream_util_test.cpp new file mode 100644 index 000000000..fb499f1f7 --- /dev/null +++ b/test/src/internal/util/stream_util_test.cpp @@ -0,0 +1,42 @@ +#include + +#include +#include +#include + +#include + +using namespace odr::internal::util; + +// A `ViewStream` is seekable: pdf object streams address their members by +// absolute position rather than reading them in order. +TEST(ViewStream, seek) { + const std::string_view view("0123456789"); + stream::ViewStream in(view); + + in.seekg(4); + EXPECT_EQ(in.tellg(), 4); + EXPECT_EQ(stream::read(in, 3), "456"); + + in.seekg(-2, std::ios::cur); + EXPECT_EQ(stream::read(in, 2), "56"); + + in.seekg(-1, std::ios::end); + EXPECT_EQ(stream::read(in, 1), "9"); + + in.seekg(0); + EXPECT_EQ(stream::read(in), "0123456789"); +} + +// An out-of-range seek fails the stream instead of moving the cursor. +TEST(ViewStream, seek_out_of_range) { + const std::string_view view("0123456789"); + stream::ViewStream in(view); + + in.seekg(11); + EXPECT_TRUE(in.fail()); + + in.clear(); + in.seekg(-1, std::ios::beg); + EXPECT_TRUE(in.fail()); +}