Keep the current revision of objects superseded by an incremental update

[mgralikowski]

TL;DR: getPages() returns a wrong, too low page count for PDFs that were saved incrementally and use object streams. We hit it in production as 1 page for a 13-page contract re-saved in macOS Preview, which put content meant for the last page on the first one.

Refs #471: this is the collision @PrinsFrank narrowed down there ($this->objects[$id] = $object in Parser::parseObject()), with a fix that follows the cross-reference data instead of keeping both copies.

Problem

An incremental update (ISO 32000-1, 7.5.6) appends new revisions of objects, and the newest cross-reference section decides which one is current. The object streams of an older revision (7.5.7) still carry the stale copies.

Two things combine:

1. RawDataParser stores cross-reference stream entries of type 2 (compressed objects) under a "<stream>_0_<index>" key. The object's own number is lost, so the newest-first rule that type 1 entries follow (if (!isset($xref['xref'][$index]))) does not apply to them.
2. Parser::parseObject() unpacks every object stream wholesale and overwrites whatever was parsed before under the same id.

Files re-saved on macOS (Preview, Pages) can carry an older revision of /Pages in an object stream and the current one uncompressed. The stale copy wins, and getPages() returns too few pages; we hit 1 page for a 13-page contract. The same happens after a signing stamp appends an incremental update.

Still present in v2.13.0-beta1 (4e964c7, also current master) and in v2.12.5: both new tests below fail there with 1 page instead of 2.

Fix

• RawDataParser: type 2 entries are keyed by object number ("<num>_0", as compressed objects always have generation 0), with the same first-seen rule as type 1 entries. The object stream holding the current revision is remembered in $xref['objstm']. The -1 offset marker is unchanged, so iterateIndirectObjects() still skips them.
• Parser: when unpacking an object stream, only the members the cross-reference data places in that stream are taken. Members that a later revision rewrote uncompressed, or moved to a newer object stream, are skipped.

Tests

• PageTest::testGetPagesIncrementalUpdateObjectStream on the new sample samples/bugs/IncrementalUpdateObjectStream.pdf (35 KB, a public lorem-ipsum sample re-saved on macOS). It has three revisions: the second stores /Pages (/Count 1) in an object stream, the third rewrites it uncompressed (/Count 2). Fails on master (1 page instead of 2).
• IncrementalUpdateTest builds two-revision PDFs in memory and covers both directions:
  • compressed /Pages in revision 1, uncompressed in revision 2: fails on master;
  • uncompressed in revision 1, compressed in revision 2: passes on master too, and guards that the fix does not invert the priority.

The full suite passes: 245 tests, 1441 assertions (PHP 8.3). The changed files lint on PHP 7.1, and the sample parses to 2 pages there. PHPStan and PHP-CS-Fixer report no issues.

[PrinsFrank]

Hi! Fun to be referenced in an investigation from many years ago, and I think your solution is better than my initial proposal. Please note that I've moved away from using this library, and have since started my own from scratch. There is some conflict of interest with me replying to this. ;)

Some context if you're interested

Issue #471 and resulting PR #533 was for me the final straw that made me decide to write a new parser. When looking further into this, I concluded that trailer parsing did not conform to the PDF specification section 7.5.

According to the spec, Parsers should start at the EOF marker, read the byte_offset_last_crossreference_section, and parse the cross reference stream or table at that position. This will always be the latest cross-reference source for a PDF, and any previous cross-reference source is referenced with the Prev key in the dictionary.

This parser doesn't do that, instead it scans the entire document with a regex for any trailers and keeps the last one. That is correct in most cases, but not spec-compliant and more fragile.

My initial PR was made in May of 2022, and was left open for a while before being closed by me. It shouldn't have been merged anyway, because my initial approach was naive and didn't fix the underlying bugs. Part of the issue with this library is that the original author is not involved anymore, and it's difficult to have an overview of how it works end to end. That's why I decided to start on my own library from scratch in modern PHP.