fix: handle PDF file boundaries and final cross-reference markers

CHANGELOG.md

@@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- PDF file traversal preserves the first object without a binary comment,
+  handles objects sharing a line, and uses the last trailing `startxref`.
+
 - PDF color-space parsing bounds recursive definitions and rejects invalid
   component counts and palette ranges. Large finite palette indices clip
   correctly.

src/odr/internal/pdf/pdf_file_parser.cpp

@@ -213,23 +213,19 @@ ObjectStream FileParser::read_object_stream(const std::uint32_t n,
 }
 
 void FileParser::read_header() {
-  const std::string header1 = m_parser.read_line();
-  // the second line is an optional binary-marker comment; read past it
-  m_parser.read_line();
-
-  if (!util::string::starts_with(header1, "%PDF-")) {
+  if (!m_parser.read_line().starts_with("%PDF-")) {
     throw std::runtime_error("illegal header");
   }
-
-  m_parser.skip_whitespace();
+  m_parser.skip_whitespace_and_comments();
 }
 
 Entry FileParser::read_entry() {
   const std::uint32_t position = in().tellg();
   const std::string entry_header = m_parser.read_line();
+  in().clear();
   in().seekg(position);
 
-  if (util::string::ends_with(entry_header, "obj")) {
+  if (m_parser.peek_number()) {
     return {read_indirect_object(), position};
   }
   if (entry_header == "xref") {
@@ -241,32 +237,46 @@ Entry FileParser::read_entry() {
   if (entry_header == "startxref") {
     return {read_start_xref(), position};
   }
-  if (entry_header == "%PDF-") {
+  if (entry_header.starts_with("%PDF-")) {
     read_header();
     return {Header{}, position};
   }
   if (entry_header == "%%EOF") {
+    m_parser.skip_line();
     return {Eof{}, position};
   }
 
   throw std::runtime_error("unknown entry");
 }
 
 void FileParser::seek_start_xref(const std::uint32_t margin) {
+  in().clear();
   in().seekg(0, std::ios::end);
-  const std::int64_t size = in().tellg();
-  in().seekg(std::max(static_cast<std::int64_t>(0), size - margin),
-             std::ios::beg);
-
-  while (!m_parser.in().eof()) {
-    const std::uint32_t position = m_parser.in().tellg();
-    if (const std::string line = m_parser.read_line(); line == "startxref") {
-      m_parser.in().seekg(position);
-      return;
+  const std::streamoff size = in().tellg();
+  if (size < 0) {
+    throw std::runtime_error("cannot seek PDF trailer");
+  }
+  const std::streamoff start = std::max(std::streamoff{0}, size - margin);
+  in().seekg(start > 0 ? start - 1 : start, std::ios::beg);
+  if (start > 0) {
+    const char previous = m_parser.bumpc();
+    if (previous != '\r' && previous != '\n') {
+      m_parser.skip_line();
     }
   }
 
-  throw std::runtime_error("unexpected stream exhaust");
+  std::optional<std::streampos> last;
+  while (in().good()) {
+    const std::streampos position = in().tellg();
+    if (m_parser.read_line() == "startxref") {
+      last = position;
+    }
+  }
+  if (!last.has_value() || in().bad()) {
+    throw std::runtime_error("missing PDF startxref");
+  }
+  in().clear();
+  in().seekg(*last);
 }
 
 Xref FileParser::read_xref_stream_table(

src/odr/internal/pdf/pdf_file_parser.hpp

@@ -42,7 +42,8 @@ class FileParser {
   void read_header();
   [[nodiscard]] Entry read_entry();
 
-  void seek_start_xref(std::uint32_t margin = 64);
+  /// Seek the last startxref line in the trailing search window.
+  void seek_start_xref(std::uint32_t margin = 1024);
 
   /// Decode the entry table of a cross-reference stream (ISO 32000-1 7.5.8.3)
   /// from the already de-filtered `data`. `field_widths` is the `/W` array

test/src/internal/pdf/pdf_file_parser.cpp

@@ -327,3 +327,44 @@ TEST(ObjectStream, parse_members) {
   ASSERT_EQ(members[1].object.as_array().size(), 3);
   EXPECT_EQ(members[1].object.as_array()[2].as_integer(), 3);
 }
+
+TEST(FileParser, headers_preserve_objects_without_a_binary_comment) {
+  for (const std::string comments :
+       {"", "%binary marker\n", "%first\n%second\n"}) {
+    std::istringstream in("%PDF-1.7\n" + comments +
+                          "1 0 obj 42 endobj 2 0 obj\n13\nendobj\n%%EOF");
+    FileParser parser(in);
+    ASSERT_TRUE(parser.read_entry().is_header());
+    for (const auto &[id, value] : {std::pair{Integer{1}, Integer{42}},
+                                    std::pair{Integer{2}, Integer{13}}}) {
+      const Entry entry = parser.read_entry();
+      ASSERT_TRUE(entry.is_object());
+      EXPECT_EQ(entry.as_object().reference.id, id);
+      EXPECT_EQ(entry.as_object().object.as_integer(), value);
+    }
+    EXPECT_TRUE(parser.read_entry().is_eof());
+    EXPECT_TRUE(in.eof());
+  }
+}
+
+TEST(FileParser, selects_the_last_startxref_after_eof) {
+  for (const std::string &padding : {std::string{}, std::string(100, ' ')}) {
+    for (const auto state :
+         {std::ios::goodbit, std::ios::eofbit | std::ios::failbit}) {
+      std::istringstream in("startxref\n1\n%%EOF\nstartxref\n2\n%%EOF\n" +
+                            padding);
+      FileParser parser(in);
+      in.setstate(state);
+      parser.seek_start_xref();
+      EXPECT_EQ(parser.read_start_xref().start, 2u);
+    }
+  }
+}
+
+TEST(FileParser, trailer_search_does_not_match_a_partial_line) {
+  const std::string tail = "startxref\n2\n%%EOF\n";
+  std::istringstream in("not-" + tail);
+  EXPECT_THROW(
+      FileParser(in).seek_start_xref(static_cast<std::uint32_t>(tail.size())),
+      std::runtime_error);
+}