fix(pdf): read a comment as white space, and half a pair as one mark

CHANGELOG.md

@@ -28,6 +28,10 @@ The release run heads these entries with the version and opens a fresh
 - A frame that names a side instead of an offset sits on that side, so a centred
   image in an odt is centred. The side it names is `GraphicStyle`'s new
   `horizontal_position`, carried by the python, java and objc bindings.
+- A pdf that comments its content stream renders: `%` to the end of the line is
+  white space wherever it stands, not the start of a token.
+- Text that decodes to half a surrogate pair costs that character a replacement
+  mark instead of the whole document.
 
 ## v6.5.0 - 2026-08-10
 

src/odr/internal/pdf/pdf_file_parser.cpp

@@ -20,16 +20,17 @@ IndirectObject FileParser::read_indirect_object() {
   IndirectObject result;
 
   result.reference.id = m_parser.read_unsigned_integer();
-  m_parser.skip_whitespace();
+  m_parser.skip_whitespace_and_comments();
   result.reference.gen = m_parser.read_unsigned_integer();
-  m_parser.skip_whitespace();
+  m_parser.skip_whitespace_and_comments();
   // `obj` is not necessarily followed by a newline; some producers keep the
   // object on the same line
   m_parser.expect_characters("obj");
-  m_parser.skip_whitespace();
+  // A comment is white space in the gaps around the body (7.2.4).
+  m_parser.skip_whitespace_and_comments();
 
   result.object = m_parser.read_object();
-  m_parser.skip_whitespace();
+  m_parser.skip_whitespace_and_comments();
   // an indirect object whose body is itself a reference (`5 0 obj 12 0 R
   // endobj`): the body is followed by `endobj`/`stream`, so a digit here can
   // only be the generation of an `n g R`
@@ -57,7 +58,7 @@ IndirectObject FileParser::read_indirect_object() {
 
 Trailer FileParser::read_trailer() {
   m_parser.expect_characters("trailer");
-  m_parser.skip_whitespace();
+  m_parser.skip_whitespace_and_comments();
 
   Trailer result;
 
@@ -188,9 +189,9 @@ ObjectStream FileParser::read_object_stream(const std::uint32_t n,
   offsets.reserve(n);
   {
     for (std::uint32_t i = 0; i < n; ++i) {
-      parser().skip_whitespace();
+      parser().skip_whitespace_and_comments();
       const std::uint64_t id = parser().read_unsigned_integer();
-      parser().skip_whitespace();
+      parser().skip_whitespace_and_comments();
       const std::uint64_t offset = parser().read_unsigned_integer();
       offsets.emplace_back(id, first + static_cast<std::uint32_t>(offset));
     }
@@ -202,7 +203,7 @@ ObjectStream FileParser::read_object_stream(const std::uint32_t n,
   for (const auto &[id, position] : offsets) {
     in().clear();
     in().seekg(position);
-    parser().skip_whitespace();
+    parser().skip_whitespace_and_comments();
     members.push_back({id, parser().read_object()});
   }
 

src/odr/internal/pdf/pdf_graphics_operator_parser.cpp

@@ -210,9 +210,10 @@ std::string GraphicsOperatorParser::read_operator_name() {
       return result;
     }
     // Any white-space (7.2.2, incl. `\r` in CRLF streams) or the start of a
-    // following token ends the bareword.
+    // following token ends the bareword. `%` is a delimiter too (7.2.2), so a
+    // comment may follow an operator with nothing in between.
     if (ObjectParser::is_whitespace(static_cast<char_type>(c)) || c == '/' ||
-        c == '<' || c == '[') {
+        c == '<' || c == '[' || c == '%') {
       return result;
     }
 
@@ -224,6 +225,8 @@ std::string GraphicsOperatorParser::read_operator_name() {
 GraphicsOperator GraphicsOperatorParser::read_operator() {
   GraphicsOperator result;
 
+  m_parser.skip_whitespace_and_comments();
+
   std::string operator_name;
   while (true) {
     if (m_parser.peek_number()) {
@@ -252,7 +255,7 @@ GraphicsOperator GraphicsOperatorParser::read_operator() {
         break;
       }
     }
-    m_parser.skip_whitespace();
+    m_parser.skip_whitespace_and_comments();
   }
 
   result.type = operator_name_to_type(operator_name);
@@ -273,7 +276,7 @@ GraphicsOperator GraphicsOperatorParser::read_operator() {
     result.arguments.emplace_back(StandardString(std::move(data)));
   }
 
-  m_parser.skip_whitespace();
+  m_parser.skip_whitespace_and_comments();
 
   return result;
 }

src/odr/internal/pdf/pdf_object_parser.cpp

@@ -132,6 +132,25 @@ void ObjectParser::skip_whitespace() {
   }
 }
 
+void ObjectParser::skip_whitespace_and_comments() {
+  while (true) {
+    skip_whitespace();
+    if (geti() != '%') {
+      return;
+    }
+    while (true) {
+      const int_type c = sb().sbumpc();
+      if (c == eof) {
+        in().setstate(std::ios::eofbit);
+        return;
+      }
+      if (c == '\n' || c == '\r') {
+        break;
+      }
+    }
+  }
+}
+
 void ObjectParser::skip_line() { read_line(); }
 
 std::string ObjectParser::read_line(const bool inclusive) {
@@ -467,22 +486,22 @@ Array ObjectParser::read_array() {
   if (bumpc() != '[') {
     throw std::runtime_error("unexpected character");
   }
-  skip_whitespace();
+  skip_whitespace_and_comments();
 
   while (true) {
     if (const char_type c = getc(); c == ']') {
       bumpc();
       return Array(std::move(result));
     }
     result.emplace_back(read_object());
-    skip_whitespace();
+    skip_whitespace_and_comments();
 
     // Array elements may be bare adjacent integers, so a reference can only be
     // recognised once the `R` token actually appears: it retroactively folds
     // the two preceding integers (`n g`) into an `n g R` reference.
     if (const char_type c = getc(); c == 'R' && result.size() >= 2) {
       bumpc();
-      skip_whitespace();
+      skip_whitespace_and_comments();
 
       const UnsignedInteger gen = result.back().as_integer();
       result.pop_back();
@@ -517,7 +536,7 @@ Dictionary ObjectParser::read_dictionary() {
   if (bumpc() != '<') {
     throw std::runtime_error("unexpected character");
   }
-  skip_whitespace();
+  skip_whitespace_and_comments();
 
   while (true) {
     if (const char_type c = getc(); c == '>') {
@@ -527,9 +546,9 @@ Dictionary ObjectParser::read_dictionary() {
     }
 
     Name name = read_name();
-    skip_whitespace();
+    skip_whitespace_and_comments();
     Object value = read_object();
-    skip_whitespace();
+    skip_whitespace_and_comments();
     promote_indirect_reference(value);
 
     result.emplace(std::move(name.string), std::move(value));
@@ -576,19 +595,19 @@ void ObjectParser::promote_indirect_reference(Object &value) {
   }
   const auto id = static_cast<UnsignedInteger>(value.as_integer());
   const UnsignedInteger gen = read_unsigned_integer();
-  skip_whitespace();
+  skip_whitespace_and_comments();
   if (bumpc() != 'R') {
     throw std::runtime_error("expected 'R' to complete indirect reference");
   }
-  skip_whitespace();
+  skip_whitespace_and_comments();
   value = Object(ObjectReference{id, gen});
 }
 
 ObjectReference ObjectParser::read_object_reference() {
   UnsignedInteger id = read_unsigned_integer();
-  skip_whitespace();
+  skip_whitespace_and_comments();
   UnsignedInteger gen = read_unsigned_integer();
-  skip_whitespace();
+  skip_whitespace_and_comments();
 
   if (bumpc() != 'R') {
     throw std::runtime_error("unexpected character");

src/odr/internal/pdf/pdf_object_parser.hpp

@@ -44,6 +44,10 @@ class ObjectParser {
   static bool is_whitespace(char c);
   [[nodiscard]] bool peek_whitespace();
   void skip_whitespace();
+  /// White space plus the comments (`%` to the end of the line, 7.2.4) it may
+  /// be interleaved with. Not between entries: `%PDF-` and `%%EOF` are read as
+  /// entries of their own.
+  void skip_whitespace_and_comments();
   void skip_line();
   std::string read_line(bool inclusive = false);
   /// Advance the cursor just past the next occurrence of `marker`. Returns true

src/odr/internal/util/string_util.cpp

@@ -176,7 +176,33 @@ std::size_t string::utf8_length(const std::string &string) {
 }
 
 std::string string::u16string_to_string(const std::u16string &string) {
-  return utf8::utf16to8(string);
+  static constexpr char32_t replacement = 0xfffd;
+
+  std::string result;
+  result.reserve(string.size());
+
+  for (std::size_t i = 0; i < string.size(); ++i) {
+    const char32_t unit = string[i];
+
+    if (unit < 0xd800 || unit > 0xdfff) {
+      utf8::append(unit, result);
+      continue;
+    }
+    // a low surrogate first, or a high one with nothing to pair with
+    if (unit > 0xdbff || i + 1 >= string.size()) {
+      utf8::append(replacement, result);
+      continue;
+    }
+    const char32_t second = string[i + 1];
+    if (second < 0xdc00 || second > 0xdfff) {
+      utf8::append(replacement, result);
+      continue;
+    }
+    utf8::append(0x10000 + ((unit - 0xd800) << 10) + (second - 0xdc00), result);
+    ++i;
+  }
+
+  return result;
 }
 
 std::u16string string::string_to_u16string(const std::string_view string) {

src/odr/internal/util/string_util.hpp

@@ -66,6 +66,7 @@ std::string to_string(double d, int precision);
 
 std::size_t utf8_length(const std::string &string);
 
+/// A surrogate completing no pair becomes U+FFFD rather than throwing.
 std::string u16string_to_string(const std::u16string &string);
 std::u16string string_to_u16string(std::string_view string);
 /// @p length is a byte count, not a number of code units.

test/data.cmake

@@ -7,19 +7,19 @@
 odr_test_data(
         PATH "input/odr-public"
         URL "https://github.com/opendocument-app/OpenDocument.test.git"
-        REVISION "d7c093dffc987e8e5cf093ea7412371ce9d7363d")
+        REVISION "77ed98d72075bbd375be328b7232f40568a32563")
 
 odr_test_data(
         PATH "input/odr-private"
         URL "https://github.com/opendocument-app/OpenDocument.test-private.git"
-        REVISION "753b7e5c313c23c0524cb9ed678ac294508f3e21")
+        REVISION "d92bbbc453e6dbc0187ec2bdf0560a6e48d5643b")
 
 odr_test_data(
         PATH "reference-output/odr-public"
         URL "https://github.com/opendocument-app/OpenDocument.test.output.git"
-        REVISION "1964c34c2a6bd84d8178d9586ba59a04f172b7f2")
+        REVISION "6d1ed797b3833b99b09ce359f2689c5a3af105c1")
 
 odr_test_data(
         PATH "reference-output/odr-private"
         URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git"
-        REVISION "5e5f035f8a6e6c86ad5d978887607e8b9167c019")
+        REVISION "90841441c293ec665632b570bdfe63842926859c")

test/src/internal/pdf/pdf_file_parser.cpp

@@ -132,6 +132,18 @@ TEST(IndirectObject, reference_body) {
   EXPECT_EQ(object.object.as_reference().gen, 0u);
 }
 
+// A comment is white space in the gaps around an object's body (7.2.4).
+TEST(IndirectObject, comment_around_body) {
+  std::istringstream in("5 0 obj\n% the catalog\n<< /Type /Catalog >>\n"
+                        "% and done\nendobj");
+  FileParser parser(in);
+
+  const IndirectObject object = parser.read_indirect_object();
+  EXPECT_EQ(object.reference.id, 5u);
+  ASSERT_TRUE(object.object.is_dictionary());
+  EXPECT_EQ(object.object.as_dictionary()["Type"].as_name(), "Catalog");
+}
+
 // read_stream with a known /Length reads exactly that many bytes, then the
 // `endstream` and `endobj` keywords.
 TEST(ReadStream, known_length) {

test/src/internal/pdf/pdf_object_parser.cpp

@@ -192,6 +192,27 @@ TEST(PdfObjectParser, read_object_reference_in_containers) {
   }
 }
 
+// 7.2.4: a comment runs from `%` to the end of the line and stands wherever
+// white space may, so `%` inside an object is not the start of a token.
+TEST(PdfObjectParser, comment_inside_containers) {
+  {
+    const auto [object, rest] =
+        read_object("<< % a key follows\n/A [1 % and a value\n2] /B 3 >>");
+    ASSERT_TRUE(object.is_dictionary());
+    const Dictionary &dictionary = object.as_dictionary();
+    ASSERT_TRUE(dictionary["A"].is_array());
+    EXPECT_EQ(dictionary["A"].as_array().size(), 2u);
+    EXPECT_EQ(dictionary["B"].as_integer(), 3);
+  }
+  {
+    // `%` is a delimiter (7.2.2), so a comment may sit between the generation
+    // and the `R` that completes a reference — with no white space at all
+    const auto [object, rest] = read_object("<< /A 5 0% a comment\nR >>");
+    ASSERT_TRUE(object.is_dictionary());
+    EXPECT_EQ(object.as_dictionary()["A"].as_reference().id, 5u);
+  }
+}
+
 // 7.3.4.2: a literal string is the bytes between balanced parentheses.
 TEST(PdfObjectParser, standard_string_basic) {
   EXPECT_EQ(read_standard_string("(Hello)"), "Hello");

test/src/internal/pdf/pdf_page_extractor.cpp

@@ -71,6 +71,27 @@ TEST(PdfPageExtractor, crlf_delimited_operators) {
   EXPECT_EQ(texts[0].codes, "Hi");
 }
 
+// A comment stands where white space may (7.2.4), so a `%<TAG>` line between
+// operators is a comment and not the start of a hex string that swallows the
+// rest of the page.
+TEST(PdfPageExtractor, comment_between_operators) {
+  const auto texts = run("%<ODT_ATTRIBUTION_LABEL_SIGNAL>\nq\n"
+                         "%<ODT_ATTRIBUTION_LABEL_SIGNAL>\n"
+                         "BT /F1 12 Tf 1 0 0 1 100 700 Tm (Hi) Tj ET\nQ\n");
+  ASSERT_EQ(texts.size(), 1);
+  EXPECT_DOUBLE_EQ(texts[0].transform.e, 100);
+  EXPECT_EQ(texts[0].codes, "Hi");
+}
+
+// `%` delimits (7.2.2), so a comment needs no white space before it.
+TEST(PdfPageExtractor, comment_directly_after_operator) {
+  const auto texts =
+      run("BT /F1 12 Tf 1 0 0 1 100 700 Tm (Hi) Tj% right after\nET");
+  ASSERT_EQ(texts.size(), 1);
+  EXPECT_DOUBLE_EQ(texts[0].transform.e, 100);
+  EXPECT_EQ(texts[0].codes, "Hi");
+}
+
 // `Tm` sets the text matrix outright, scaling and all.
 TEST(PdfPageExtractor, tm_scaling) {
   const auto texts = run("BT /F1 10 Tf 2 0 0 2 50 60 Tm (X) Tj ET");

test/src/internal/util/string_util_test.cpp

@@ -172,3 +172,17 @@ TEST(string_util, find_ignore_case) {
   // `from` past the end is not an out-of-range read.
   EXPECT_EQ(find_ignore_case("abc", "a", 99), std::string_view::npos);
 }
+
+TEST(string_util, u16string_to_string) {
+  EXPECT_EQ(u16string_to_string(u"abc"), "abc");
+  EXPECT_EQ(u16string_to_string(u"\U0001f600"), "\xf0\x9f\x98\x80");
+
+  // A surrogate completing no pair is not a code point: it costs its own
+  // character rather than the whole string.
+  EXPECT_EQ(u16string_to_string(std::u16string{u'a', 0xdc62, u'b'}),
+            "a\xef\xbf\xbd"
+            "b");
+  EXPECT_EQ(u16string_to_string(std::u16string{0xd83d}), "\xef\xbf\xbd");
+  EXPECT_EQ(u16string_to_string(std::u16string{0xd83d, u'a'}), "\xef\xbf\xbd"
+                                                               "a");
+}