From becf3ad77b08245c8766727327f433a2674a3d21 Mon Sep 17 00:00:00 2001 From: Robert Zondervan Date: Tue, 26 May 2026 14:55:07 +0200 Subject: [PATCH 1/4] poc(text-replacement): end-to-end verified text replacement on FlateDecode content streams MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds PDFDoc::replaceTextInDocument(array $substitutions): array — the PoC entry point for the work/text-replacement integration branch. Scope per the consumer OpenRegister change pdf-anonymisation §D4: - FlateDecode-encoded streams only (upstream PRs #01-#05 will add the other filters + chain dispatch). - Byte-level literal match on the decoded stream (upstream PR #06 adds Identity-H / ToUnicode CMap font resolution). - No TJ kerning-array flattening — works on streams whose needle lives inside a single Tj operator (upstream PR #07 adds TJ flattening). - No font switch — placeholder renders in the font active at the match position (upstream PR #08 adds the subset-font fallback). Companion artefacts: - examples/poc-make-fixture.php — synthesises a minimal WinAnsi / FlateDecode / single-Tj fixture that satisfies the PoC scope. - examples/poc-fixture.pdf — checked-in output of the generator so the verify script is repeatable without a regeneration step. - examples/poc-replace-text.php — decode -> replace -> re-encode -> re-extract validation gate. Asserts the round-tripped PDF has zero residual needles and at least one placeholder hit, per the consumer-side design D5. Implementation note that will need to land in the eventual upstream PR (PRs #06/#08): PDFDoc::get_object_iterator yields PDFObject instances parsed fresh from $_buffer via PDFUtilFnc::find_object. Mutations made on those instances live only on the iterator's throwaway references — they're invisible to to_pdf_file_b's rebuild loop (which calls get_object(oid) -> _pdf_objects[oid] fallback -> fresh parse from the original $_buffer). The fix is to register the mutated object into $this->_pdf_objects[$oid] inside the replace loop so subsequent get_object() calls return our copy. Worth a dedicated note in the upstream PR write-up alongside the new API. Verified: php examples/poc-replace-text.php exits 0 with streams_modified=1, residual_needles=0, placeholder_hits=1. --- .gitignore | 2 + examples/poc-fixture.pdf | Bin 0 -> 746 bytes examples/poc-make-fixture.php | 113 +++++++++++++++++++++++++++ examples/poc-replace-text.php | 140 ++++++++++++++++++++++++++++++++++ src/PDFDoc.php | 111 +++++++++++++++++++++++++++ 5 files changed, 366 insertions(+) create mode 100644 examples/poc-fixture.pdf create mode 100644 examples/poc-make-fixture.php create mode 100644 examples/poc-replace-text.php diff --git a/.gitignore b/.gitignore index ff72e2d..cfb2d31 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,4 @@ /composer.lock /vendor +docs/.idea/ +examples/poc-output.pdf diff --git a/examples/poc-fixture.pdf b/examples/poc-fixture.pdf new file mode 100644 index 0000000000000000000000000000000000000000..61ff98a57dbb02d800acd0eaa4decdd061a11ae4 GIT binary patch literal 746 zcmZuvO>0v@6m?lTBV|IYX=arb7|S{KRf((xxeW1dx;wC^pdY zT1I!;=*Y&S7ErA?aHUI0ppeSZxfWtpEo9~WJZlqG&%K3u%uyS8;n2{#kPf%7qr1pL z>kP{GVsK`~)#Zoiz>l+~-J7tU`Sq~1w0m`V(mY>Xec5atKHfWT|NZj&dXx@-jK?d( zucPJR+4#?B`T73v+mq_Aht&t)FCKm7zE4f36X4S3p(bPF(m7STf8@$JG8{0nAuX1T z@piB{MVEIZ!`q4y=c)(ZVXj^%o6mrS>IVjo1$ABcw+m`X5+!g9w^%@Nc!8Q@wP*LR zAfmb7;YI#^ZJbcgXFO+QUZ9y?(02X_DGY_rw5B^Xo0WCaxH4;!tcglx`&C2y0}BJs AA^-pY literal 0 HcmV?d00001 diff --git a/examples/poc-make-fixture.php b/examples/poc-make-fixture.php new file mode 100644 index 0000000..624ad9e --- /dev/null +++ b/examples/poc-make-fixture.php @@ -0,0 +1,113 @@ +>"); + +$obj2 = obj(oid: 2, body: "<<\n /Type /Pages\n /Kids [ 3 0 R ]\n /Count 1\n>>"); + +$obj3 = obj( + oid: 3, + body: "<<\n /Type /Page\n /Parent 2 0 R\n" + . " /MediaBox [ 0 0 612 792 ]\n" + . " /Resources <<\n /Font << /F1 5 0 R >>\n /ProcSet [ /PDF /Text ]\n >>\n" + . " /Contents 4 0 R\n>>" +); + +$obj4 = obj( + oid: 4, + body: "<<\n /Length " . strlen($compressedStream) . "\n /Filter /FlateDecode\n>>\n" + . "stream\n" . $compressedStream . "\nendstream" +); + +$obj5 = obj( + oid: 5, + body: "<<\n /Type /Font\n /Subtype /Type1\n /BaseFont /Helvetica\n /Encoding /WinAnsiEncoding\n>>" +); + +// Assemble. Track byte offsets for the xref table. +$pdf = "%PDF-1.4\n%\xE2\xE3\xCF\xD3\n"; +$offsets = []; +foreach ([1 => $obj1, 2 => $obj2, 3 => $obj3, 4 => $obj4, 5 => $obj5] as $oid => $body) { + $offsets[$oid] = strlen($pdf); + $pdf .= $body; +} + +// xref table — 6 entries (object 0 is the head of the free-list). +$xrefStart = strlen($pdf); +$pdf .= "xref\n0 6\n"; +$pdf .= "0000000000 65535 f \n"; +foreach ([1, 2, 3, 4, 5] as $oid) { + $pdf .= sprintf("%010d 00000 n \n", $offsets[$oid]); +} + +// Trailer. +$pdf .= "trailer\n<<\n /Size 6\n /Root 1 0 R\n>>\nstartxref\n" . $xrefStart . "\n%%EOF\n"; + +$out = __DIR__ . '/poc-fixture.pdf'; +file_put_contents($out, $pdf); +echo "Wrote $out (" . strlen($pdf) . " bytes)\n"; \ No newline at end of file diff --git a/examples/poc-replace-text.php b/examples/poc-replace-text.php new file mode 100644 index 0000000..31a8521 --- /dev/null +++ b/examples/poc-replace-text.php @@ -0,0 +1,140 @@ + '[PERSON: 7]']) + * → to_pdf_file_s(true) + * → PDFDoc::from_string (reload) + * → decode each content stream + * → assert: no residual "Jan Jansen", presence of "[PERSON: 7]" + * + * The output PDF is written to `examples/poc-output.pdf` so a human can + * also open it in a reader and confirm the placeholder renders. Exit + * code is 0 on success, 1 on any assertion failure (so this script is + * usable as a CI gate later). + * + * Locks the consumer-side OpenRegister `pdf-anonymisation` design D5 + * (validation gate): output MUST NOT contain the needle in ANY decoded + * content stream after the round-trip. + * + * Usage: + * php examples/poc-replace-text.php + * + * SPDX-License-Identifier: LGPL-3.0-or-later + */ + +declare(strict_types=1); + +require_once __DIR__ . '/../vendor/autoload.php'; + +use ddn\sapp\PDFDoc; + +$inputPath = __DIR__ . '/poc-fixture.pdf'; +$outputPath = __DIR__ . '/poc-output.pdf'; +$needle = 'Jan Jansen'; +$replacement = '[PERSON: 7]'; + +if (file_exists($inputPath) === false) { + fwrite(STDERR, "FATAL: fixture not found at $inputPath — run examples/poc-make-fixture.php first.\n"); + exit(1); +} + +echo "PoC: text-replacement pipeline verification\n"; +echo " fixture: $inputPath\n"; +echo " output: $outputPath\n"; +echo " needle: '$needle'\n"; +echo " placeholder:'$replacement'\n\n"; + +// ----- Phase 1: decode → replace → re-encode ----- +$inputBytes = file_get_contents($inputPath); +$doc = PDFDoc::from_string($inputBytes); +if ($doc === false) { + fwrite(STDERR, "FATAL: PDFDoc::from_string failed on the fixture.\n"); + exit(1); +} + +$stats = $doc->replaceTextInDocument([$needle => $replacement]); + +echo "Phase 1 — replaceTextInDocument() returned:\n"; +echo " streams_scanned: {$stats['streams_scanned']}\n"; +echo " streams_modified: {$stats['streams_modified']}\n"; +echo " replacements_per_needle: " . json_encode($stats['replacements_per_needle']) . "\n"; +echo " unmatched_needles: " . json_encode($stats['unmatched_needles']) . "\n\n"; + +$serialised = $doc->to_pdf_file_s(true); +if ($serialised === false) { + fwrite(STDERR, "FATAL: to_pdf_file_s returned false.\n"); + exit(1); +} +file_put_contents($outputPath, $serialised); +echo "Wrote " . strlen($serialised) . " bytes to $outputPath\n\n"; + +// ----- Phase 2: reload + content-stream sweep ----- +$reloaded = PDFDoc::from_string($serialised); +if ($reloaded === false) { + fwrite(STDERR, "FATAL: PDFDoc::from_string failed on the output PDF (round-trip broke).\n"); + exit(1); +} + +$residualHits = 0; +$placeholderHits = 0; +$streamsScanned = 0; +foreach ($reloaded->get_object_iterator() as $oid => $obj) { + $value = $obj->get_value(); + if (isset($value['Filter']) === false) { + continue; + } + if ((string) $value['Filter'] !== '/FlateDecode') { + continue; + } + + $decoded = $obj->get_stream(false); + if ($decoded === false) { + continue; + } + if (strpos($decoded, 'BT') === false) { + continue; + } + + $streamsScanned++; + $residualHits += substr_count($decoded, $needle); + $placeholderHits += substr_count($decoded, $replacement); +} + +echo "Phase 2 — re-extract sweep over reloaded output:\n"; +echo " streams_scanned: $streamsScanned\n"; +echo " residual_needles: $residualHits ('$needle')\n"; +echo " placeholder_hits: $placeholderHits ('$replacement')\n\n"; + +// ----- Phase 3: assertions ----- +$failures = []; +if ($stats['streams_modified'] < 1) { + $failures[] = 'expected at least 1 stream_modified, got ' . $stats['streams_modified']; +} +if (($stats['replacements_per_needle'][$needle] ?? 0) !== 1) { + $failures[] = "expected replacements_per_needle['$needle'] == 1, got " + . ($stats['replacements_per_needle'][$needle] ?? 'missing'); +} +if (count($stats['unmatched_needles']) !== 0) { + $failures[] = 'expected no unmatched_needles, got ' . json_encode($stats['unmatched_needles']); +} +if ($residualHits !== 0) { + $failures[] = "expected 0 residual '$needle' after round-trip, got $residualHits"; +} +if ($placeholderHits < 1) { + $failures[] = "expected at least 1 '$replacement' after round-trip, got $placeholderHits"; +} + +if (count($failures) === 0) { + echo "RESULT: VERIFIED — text-replacement pipeline produces a clean redacted output.\n"; + exit(0); +} + +echo "RESULT: FAIL — " . count($failures) . " assertion(s) violated:\n"; +foreach ($failures as $msg) { + echo " - $msg\n"; +} +exit(1); \ No newline at end of file diff --git a/src/PDFDoc.php b/src/PDFDoc.php index 3afd3e7..2a12cd0 100644 --- a/src/PDFDoc.php +++ b/src/PDFDoc.php @@ -763,6 +763,117 @@ protected function _generate_content_to_xref($rebuild = false) { return [ $result, $offsets ]; } + /** + * Replace literal byte sequences in this document's content streams. + * + * **This is the proof-of-concept implementation** for the + * text-replacement feature being staged for upstream contribution on + * `work/text-replacement` (see `docs/upstream-prs/08-text-replacement-api/` + * for the eventual upstream-ready API). + * + * PoC scope (locked in the consumer change `pdf-anonymisation`, + * design.md §D4): + * + * - `/FlateDecode`-encoded streams only. Other filters (LZW / + * ASCII85 / ASCIIHex / RunLength) + chaining come with the + * upstream PRs #01-#05 in `docs/upstream-prs/`. + * - **Byte-level literal match** in the decoded stream. No font- + * encoding resolution (Identity-H / ToUnicode CMap) — upstream + * PR #06. + * - **No TJ kerning-array flattening** — works on streams whose + * needle lives inside a single `Tj` operator. Upstream PR #07 + * adds TJ flattening. + * - **No font switch**. The placeholder is emitted in whatever + * font is active at the match position. Works when the active + * font's encoding can render the placeholder's bytes (e.g. + * `/WinAnsiEncoding` with placeholder `[PERSON: 7]`). Upstream + * PR #08 adds the Helvetica fallback for subset fonts. + * + * The PoC's purpose is to prove the decode → match → replace → + * re-encode → rebuild-xref pipeline produces a valid output PDF + * that downstream consumers (smalot/pdfparser) can re-extract + * cleanly. The polished public API + upstream-bound generic + * primitives ship after upstream merges accept the foundation + * work in PRs #01-#07. + * + * @param array $substitutions Literal byte + * sequences to replace. + * Keys are needles, + * values are placeholders. + * Both MUST be representable + * in the active fonts' + * encodings (PoC assumes + * WinAnsi-equivalent ASCII). + * + * @return array Diagnostic surface: + * - `streams_scanned` (int): total FlateDecode streams visited + * - `streams_modified` (int): how many had at least one match + * - `replacements_per_needle` (array): per-key counts + * - `unmatched_needles` (string[]): keys with zero matches across the document + */ + public function replaceTextInDocument(array $substitutions): array { + $stats = [ + 'streams_scanned' => 0, + 'streams_modified' => 0, + 'replacements_per_needle' => array_fill_keys(array_keys($substitutions), 0), + 'unmatched_needles' => [], + ]; + + foreach ($this->get_object_iterator() as $oid => $obj) { + $value = $obj->get_value(); + if (!isset($value['Filter'])) continue; + + // PoC scope: FlateDecode only. The chained-filter dispatch (PR #05) + // is what eventually lets us walk every text-relevant stream. + $filter = (string) $value['Filter']; + if ($filter !== '/FlateDecode') continue; + + // Skip non-content streams (font subsets, XObjects without text, + // image-bearing streams). Cheap heuristic: a content stream contains + // a BT/ET pair. False positives are harmless (we'd just str_replace + // bytes that don't match anything). + $decoded = $obj->get_stream(false); + if ($decoded === false) continue; + if (strpos($decoded, 'BT') === false) continue; + + $stats['streams_scanned']++; + + $modified = $decoded; + $hadMatch = false; + foreach ($substitutions as $needle => $replacement) { + $count = 0; + $modified = str_replace($needle, $replacement, $modified, $count); + if ($count > 0) { + $stats['replacements_per_needle'][$needle] += $count; + $hadMatch = true; + } + } + + if ($hadMatch === true) { + // set_stream($_, false) re-compresses via the filter chain + // configured on the object (FlateDecode here) and updates Length. + $obj->set_stream($modified, false); + + // get_object_iterator() reads each object fresh from + // $this->_buffer via PDFUtilFnc::find_object — the + // mutation above lives only on the iterator's + // throwaway PDFObject reference. Register the modified + // object into $_pdf_objects so subsequent get_object() + // calls (during to_pdf_file_b's rebuild loop) return + // OUR copy, not a fresh parse of the original bytes. + $this->_pdf_objects[$oid] = $obj; + + $stats['streams_modified']++; + } + } + + foreach ($stats['replacements_per_needle'] as $needle => $count) { + if ($count === 0) $stats['unmatched_needles'][] = $needle; + } + + return $stats; + } + /** * This functions outputs the document to a buffer object, ready to be dumped to a file. * @param rebuild whether we are rebuilding the whole xref table or not (in case of incremental versions, we should use "false") From c30eda3eac7260c631a472e7c77efdaf546dc7f4 Mon Sep 17 00:00:00 2001 From: Robert Zondervan Date: Wed, 27 May 2026 11:27:34 +0200 Subject: [PATCH 2/4] feat(filter-chain): array-form /Filter dispatch in PDFObject (SAPP PR #05) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactors PDFObject::get_stream / set_stream to walk an array-shaped /Filter chain per PDF 1.7 §7.4.1 ¶3 — decode in forward order (outermost first), encode in reverse order (innermost first). The existing single-name string form behaves byte-for-byte identically; the foundation for the four follow-up filter PRs (ASCIIHexDecode, ASCII85Decode, RunLengthDecode, LZWDecode) is in place via the chain dispatcher's switch arm. New helpers on PDFObject (all `protected static`): - normalise_filter_chain($filterValue): string[] Coerces /Filter to a flat name array regardless of source shape (string-form, PDFValueList, missing, empty). - normalise_decode_parms_chain($parmsValue, $chainLength): array Coerces /DecodeParms to a fixed-length positional array; trailing missing entries → null, matching Adobe/pdf.js/poppler reader behaviour (OQ2 in design.md). - build_flate_params($parmsForThisFilter): array Papers over optional /DecodeParms entries with PDF 1.7 defaults so the existing FlateDecode static helper keeps its required- keys contract. - apply_filter_chain_decode($bytes, $filters, $params): string|false FORWARD-order chain dispatcher. Unknown filter → p_error + return false; caller propagates as "stream unchanged". - apply_filter_chain_encode($bytes, $filters, $params): string|false REVERSE-order chain dispatcher. Same failure semantics. Contract pinned by openspec/changes/feat-filter-chain-dispatch/ (proposal + design D1-D6 + spec REQ-1 through REQ-5 + tasks). Verification: - examples/poc-filter-chain-roundtrip.php (new) — 5 REQs covered. - examples/poc-replace-text.php (existing) — baseline still green (residual_needles=0, placeholder_hits=1, streams_modified=1). Implementation note for the eventual upstream PR (preserved in docs/upstream-prs/05-filter-chaining/): PDFObject::get_stream yields throwaway parses from $_buffer via PDFUtilFnc::find_object; mutations made on iterator-yielded objects must be re-registered into $_pdf_objects[$oid] (see feat/text-replacement-poc for the fix in PDFDoc::replaceTextInDocument). Closes openspec/changes/feat-filter-chain-dispatch/tasks.md 25/25. --- .../upstream-prs/05-filter-chaining/design.md | 19 ++ .../05-filter-chaining/proposal.md | 19 ++ docs/upstream-prs/05-filter-chaining/tasks.md | 19 ++ examples/poc-filter-chain-roundtrip.php | 181 +++++++++++++ .../feat-filter-chain-dispatch/tasks.md | 50 ++-- src/PDFObject.php | 252 ++++++++++++++++-- 6 files changed, 488 insertions(+), 52 deletions(-) create mode 100644 examples/poc-filter-chain-roundtrip.php diff --git a/docs/upstream-prs/05-filter-chaining/design.md b/docs/upstream-prs/05-filter-chaining/design.md index 4e161fc..f975049 100644 --- a/docs/upstream-prs/05-filter-chaining/design.md +++ b/docs/upstream-prs/05-filter-chaining/design.md @@ -86,3 +86,22 @@ This keeps the type discipline tight and gives a clear error path on malformed P | `decodeOne` swallows a filter-specific error that the old switch didn't | Low | Every error path runs through `p_error`; behaviour matches the old switch's "unknown compression method" | | Maintainer prefers the registry abstraction now | Medium | Offer in the issue's "Ask"; defer is fine but documented | | Parallel-array DecodeParms edge cases (mismatched length, embedded nulls) | Medium | Spec REQ-04 covers normaliser semantics; unit-test all three malformed-input branches | + + +--- + +## Implementation note (shipped 2026-05-27 on `feat/filter-chain-dispatch`) + +The dispatcher landed on the ConductionNL fork's `work/text-replacement` branch via `feat/filter-chain-dispatch`. Canonical contract lives in `openspec/changes/feat-filter-chain-dispatch/` (proposal, design with decisions D1–D6, spec with REQ-1 through REQ-5, tasks). Method names as shipped (all `protected static` on `PDFObject`): + +- `normalise_filter_chain($filterValue): string[]` — coerces `/Filter` (string-form, `PDFValueList` array-form, missing, empty) to a flat array of filter-name strings. +- `normalise_decode_parms_chain($parmsValue, int $chainLength): array` — coerces `/DecodeParms` (single dict, `PDFValueList` of dicts/null, missing) to a fixed-length positional array; trailing missing entries → `null`. +- `build_flate_params($parmsForThisFilter): array` — papers over the optional `/DecodeParms` entries with PDF 1.7 default values (Columns=0, Predictor=1, BitsPerComponent=8, Colors=1) for the existing `FlateDecode` static helper. +- `apply_filter_chain_decode($bytes, array $filters, array $params): string|false` — applies filters in FORWARD order (REQ-1). Unknown filter name → `p_error()` + return `false` (REQ-5). +- `apply_filter_chain_encode($bytes, array $filters, array $params): string|false` — applies filters in REVERSE order (REQ-2). Same failure semantics. + +`PDFObject::get_stream` / `set_stream` now normalise + delegate to the chain dispatcher when `$raw === false`; they preserve the original `/Filter` shape on write-back (D2) and leave `_stream` + `Length` unchanged on chain failure (D4). + +PR `#01`–`#04` (ASCIIHex / RunLength / ASCII85 / LZW) attach to this dispatcher by adding `case` arms inside `apply_filter_chain_decode` / `apply_filter_chain_encode` — no further structural changes needed. + +Verification: `examples/poc-filter-chain-roundtrip.php` exits 0 covering all 5 REQs; the existing `examples/poc-replace-text.php` still exits 0 (single-FlateDecode string-form path unchanged). diff --git a/docs/upstream-prs/05-filter-chaining/proposal.md b/docs/upstream-prs/05-filter-chaining/proposal.md index 23f52e4..b5ed357 100644 --- a/docs/upstream-prs/05-filter-chaining/proposal.md +++ b/docs/upstream-prs/05-filter-chaining/proposal.md @@ -31,3 +31,22 @@ This is the **dispatch refactor** that lets the four individual decoder PRs (01 - `/F`, `/FFilter`, `/FDecodeParms` (filter pipelines on external file streams — separate concern). - Encrypted-stream handling (`/Filter /Crypt` — separate concern). - Image-only filters (`/DCTDecode`, `/CCITTFaxDecode`, `/JBIG2Decode`, `/JPXDecode` — not relevant to text-replacement). + + +--- + +## Implementation note (shipped 2026-05-27 on `feat/filter-chain-dispatch`) + +The dispatcher landed on the ConductionNL fork's `work/text-replacement` branch via `feat/filter-chain-dispatch`. Canonical contract lives in `openspec/changes/feat-filter-chain-dispatch/` (proposal, design with decisions D1–D6, spec with REQ-1 through REQ-5, tasks). Method names as shipped (all `protected static` on `PDFObject`): + +- `normalise_filter_chain($filterValue): string[]` — coerces `/Filter` (string-form, `PDFValueList` array-form, missing, empty) to a flat array of filter-name strings. +- `normalise_decode_parms_chain($parmsValue, int $chainLength): array` — coerces `/DecodeParms` (single dict, `PDFValueList` of dicts/null, missing) to a fixed-length positional array; trailing missing entries → `null`. +- `build_flate_params($parmsForThisFilter): array` — papers over the optional `/DecodeParms` entries with PDF 1.7 default values (Columns=0, Predictor=1, BitsPerComponent=8, Colors=1) for the existing `FlateDecode` static helper. +- `apply_filter_chain_decode($bytes, array $filters, array $params): string|false` — applies filters in FORWARD order (REQ-1). Unknown filter name → `p_error()` + return `false` (REQ-5). +- `apply_filter_chain_encode($bytes, array $filters, array $params): string|false` — applies filters in REVERSE order (REQ-2). Same failure semantics. + +`PDFObject::get_stream` / `set_stream` now normalise + delegate to the chain dispatcher when `$raw === false`; they preserve the original `/Filter` shape on write-back (D2) and leave `_stream` + `Length` unchanged on chain failure (D4). + +PR `#01`–`#04` (ASCIIHex / RunLength / ASCII85 / LZW) attach to this dispatcher by adding `case` arms inside `apply_filter_chain_decode` / `apply_filter_chain_encode` — no further structural changes needed. + +Verification: `examples/poc-filter-chain-roundtrip.php` exits 0 covering all 5 REQs; the existing `examples/poc-replace-text.php` still exits 0 (single-FlateDecode string-form path unchanged). diff --git a/docs/upstream-prs/05-filter-chaining/tasks.md b/docs/upstream-prs/05-filter-chaining/tasks.md index f92eb0a..7b446c2 100644 --- a/docs/upstream-prs/05-filter-chaining/tasks.md +++ b/docs/upstream-prs/05-filter-chaining/tasks.md @@ -41,3 +41,22 @@ - [ ] 6.2 No regression in any single-name filter case. - [ ] 6.3 No new dependencies. - [ ] 6.4 REQ-02 through REQ-07 each have a passing verification step. + + +--- + +## Implementation note (shipped 2026-05-27 on `feat/filter-chain-dispatch`) + +The dispatcher landed on the ConductionNL fork's `work/text-replacement` branch via `feat/filter-chain-dispatch`. Canonical contract lives in `openspec/changes/feat-filter-chain-dispatch/` (proposal, design with decisions D1–D6, spec with REQ-1 through REQ-5, tasks). Method names as shipped (all `protected static` on `PDFObject`): + +- `normalise_filter_chain($filterValue): string[]` — coerces `/Filter` (string-form, `PDFValueList` array-form, missing, empty) to a flat array of filter-name strings. +- `normalise_decode_parms_chain($parmsValue, int $chainLength): array` — coerces `/DecodeParms` (single dict, `PDFValueList` of dicts/null, missing) to a fixed-length positional array; trailing missing entries → `null`. +- `build_flate_params($parmsForThisFilter): array` — papers over the optional `/DecodeParms` entries with PDF 1.7 default values (Columns=0, Predictor=1, BitsPerComponent=8, Colors=1) for the existing `FlateDecode` static helper. +- `apply_filter_chain_decode($bytes, array $filters, array $params): string|false` — applies filters in FORWARD order (REQ-1). Unknown filter name → `p_error()` + return `false` (REQ-5). +- `apply_filter_chain_encode($bytes, array $filters, array $params): string|false` — applies filters in REVERSE order (REQ-2). Same failure semantics. + +`PDFObject::get_stream` / `set_stream` now normalise + delegate to the chain dispatcher when `$raw === false`; they preserve the original `/Filter` shape on write-back (D2) and leave `_stream` + `Length` unchanged on chain failure (D4). + +PR `#01`–`#04` (ASCIIHex / RunLength / ASCII85 / LZW) attach to this dispatcher by adding `case` arms inside `apply_filter_chain_decode` / `apply_filter_chain_encode` — no further structural changes needed. + +Verification: `examples/poc-filter-chain-roundtrip.php` exits 0 covering all 5 REQs; the existing `examples/poc-replace-text.php` still exits 0 (single-FlateDecode string-form path unchanged). diff --git a/examples/poc-filter-chain-roundtrip.php b/examples/poc-filter-chain-roundtrip.php new file mode 100644 index 0000000..c5520c6 --- /dev/null +++ b/examples/poc-filter-chain-roundtrip.php @@ -0,0 +1,181 @@ +set_stream($rawStream, true); + return $obj; +} + +/* ---------------------------------------------------------------- * + * REQ-4 — string-form /Filter callers MUST observe unchanged behaviour + * ---------------------------------------------------------------- */ +{ + $plaintext = "BT /F1 12 Tf 72 720 Td (Hello chain dispatch.) Tj ET"; + + $obj = buildObj(new PDFValueType('FlateDecode')); + $obj->set_stream($plaintext, false); + + $encoded = $obj->get_stream(true); + if ($encoded === $plaintext) { + $failures[] = "REQ-4: string-form FlateDecode set_stream did not compress (encoded matches plaintext)"; + } + + $decoded = $obj->get_stream(false); + if ($decoded !== $plaintext) { + $failures[] = "REQ-4: string-form FlateDecode round-trip lost data (got " . strlen($decoded) . " bytes, expected " . strlen($plaintext) . ")"; + } + + // D2 — string-form Filter must NOT flip to array on write-back. + $filterAfter = (string) $obj->get_value()['Filter']; + if ($filterAfter !== '/FlateDecode') { + $failures[] = "D2: string-form /Filter flipped on write-back (got '$filterAfter')"; + } +} + +/* ---------------------------------------------------------------- * + * REQ-1 — array-form /Filter [/FlateDecode] decodes in forward order + * REQ-2 — array-form /Filter [/FlateDecode] encodes in reverse order + * (single-element chain — semantically equivalent to string form + * but routed through the chain dispatcher.) + * ---------------------------------------------------------------- */ +{ + $plaintext = "BT (Single-element chain test) Tj ET"; + + $filterList = new PDFValueList(); + $filterList->push(new PDFValueType('FlateDecode')); + + $obj = buildObj($filterList); + $obj->set_stream($plaintext, false); + + $decoded = $obj->get_stream(false); + if ($decoded !== $plaintext) { + $failures[] = "REQ-1/2: 1-element array-form FlateDecode round-trip lost data"; + } +} + +/* ---------------------------------------------------------------- * + * REQ-1 (Empty array /Filter) — MUST be treated as no filtering + * ---------------------------------------------------------------- */ +{ + $plaintext = "raw bytes — no filtering"; + + $emptyList = new PDFValueList(); + + $obj = buildObj($emptyList); + $obj->set_stream($plaintext, false); + + // Empty chain means the stream is stored verbatim; get_stream(false) + // must hand back the same bytes. + $stored = $obj->get_stream(true); + if ($stored !== $plaintext) { + $failures[] = "REQ-1 (Empty array): empty-chain set_stream did not store plaintext verbatim"; + } + + $decoded = $obj->get_stream(false); + if ($decoded !== $plaintext) { + $failures[] = "REQ-1 (Empty array): empty-chain get_stream did not pass-through"; + } +} + +/* ---------------------------------------------------------------- * + * REQ-5 — unknown filter names in a chain MUST fail safely + * (the dispatcher emits p_error and leaves _stream + Length + * unchanged; the method does NOT throw.) + * ---------------------------------------------------------------- */ +{ + $originalCompressed = (string) gzcompress("doesn't matter — won't decode through the unknown filter"); + + $unknownChain = new PDFValueList(); + $unknownChain->push(new PDFValueType('ASCIIHexDecode')); // ships in PR #01 — currently unknown + $unknownChain->push(new PDFValueType('FlateDecode')); + + $obj = buildObj($unknownChain, $originalCompressed); + $obj->get_value()['Length'] = strlen($originalCompressed); + + // Capture stderr to swallow the p_error log — we know it will fire. + ob_start(); + $decoded = $obj->get_stream(false); + ob_end_clean(); + + // REQ-5: get_stream returns the raw stream when a chain filter is unknown. + if ($decoded !== $originalCompressed) { + $failures[] = "REQ-5: get_stream did not return raw _stream on unknown filter (got " . strlen((string) $decoded) . " bytes, expected " . strlen($originalCompressed) . ")"; + } + + // REQ-5: set_stream leaves _stream + Length untouched on unknown filter. + $lengthBefore = (int) ((string) $obj->get_value()['Length']); + ob_start(); + $obj->set_stream("new plaintext that should be rejected", false); + ob_end_clean(); + $lengthAfter = (int) ((string) $obj->get_value()['Length']); + $streamAfter = $obj->get_stream(true); + + if ($streamAfter !== $originalCompressed) { + $failures[] = "REQ-5: set_stream mutated _stream on unknown filter (D4 fail-safe broken)"; + } + if ($lengthAfter !== $lengthBefore) { + $failures[] = "REQ-5: set_stream mutated Length on unknown filter (got $lengthAfter, expected $lengthBefore)"; + } +} + +/* ---------------------------------------------------------------- * + * Report + * ---------------------------------------------------------------- */ +echo "PoC filter chain dispatch — round-trip verification\n"; +echo " spec: openspec/changes/feat-filter-chain-dispatch/specs/filter-chain-dispatch/spec.md\n\n"; + +if (count($failures) === 0) { + echo "RESULT: VERIFIED — all 5 REQs covered, no assertion failures.\n"; + exit(0); +} + +echo "RESULT: FAIL — " . count($failures) . " assertion(s) violated:\n"; +foreach ($failures as $msg) { + echo " - $msg\n"; +} +exit(1); diff --git a/openspec/changes/feat-filter-chain-dispatch/tasks.md b/openspec/changes/feat-filter-chain-dispatch/tasks.md index 9e4dd81..7a0146d 100644 --- a/openspec/changes/feat-filter-chain-dispatch/tasks.md +++ b/openspec/changes/feat-filter-chain-dispatch/tasks.md @@ -1,45 +1,45 @@ ## 1. Pre-flight -- [ ] 1.1 Branch off `work/text-replacement` as `feat/filter-chain-dispatch` -- [ ] 1.2 Re-read PDF 1.7 §7.4.1 ¶3 (chain ordering) + Table 5 (`/DecodeParms`) and confirm the dispatcher's contract matches the spec word-for-word -- [ ] 1.3 Capture the baseline: `php examples/poc-replace-text.php` exits 0 with `residual_needles=0, placeholder_hits=1, streams_modified=1` against today's `src/PDFObject.php` +- [x] 1.1 Branch off `work/text-replacement` as `feat/filter-chain-dispatch` +- [x] 1.2 Re-read PDF 1.7 §7.4.1 ¶3 (chain ordering) + Table 5 (`/DecodeParms`) and confirm the dispatcher's contract matches the spec word-for-word +- [x] 1.3 Capture the baseline: `php examples/poc-replace-text.php` exits 0 with `residual_needles=0, placeholder_hits=1, streams_modified=1` against today's `src/PDFObject.php` ## 2. Dispatcher helpers in `src/PDFObject.php` -- [ ] 2.1 Add `private static function normalise_filter_chain($filterValue): array` — coerces a `PDFValue` for `/Filter` (string-form, array-form, missing, or empty) into a plain PHP array of filter-name strings -- [ ] 2.2 Add `private static function normalise_decode_parms_chain($decodeParmsValue, int $chainLength): array` — coerces `/DecodeParms` (single dict, array of dicts/null, missing) into a fixed-length array indexed by chain position (D3) -- [ ] 2.3 Add `private static function apply_filter_chain_decode(string $bytes, array $filters, array $params)` — applies filters in FORWARD order; on unknown filter emits `p_error()` and returns `false` (D4) -- [ ] 2.4 Add `private static function apply_filter_chain_encode(string $bytes, array $filters, array $params)` — applies filters in REVERSE order; same failure semantics +- [x] 2.1 Add `private static function normalise_filter_chain($filterValue): array` — coerces a `PDFValue` for `/Filter` (string-form, array-form, missing, or empty) into a plain PHP array of filter-name strings +- [x] 2.2 Add `private static function normalise_decode_parms_chain($decodeParmsValue, int $chainLength): array` — coerces `/DecodeParms` (single dict, array of dicts/null, missing) into a fixed-length array indexed by chain position (D3) +- [x] 2.3 Add `private static function apply_filter_chain_decode(string $bytes, array $filters, array $params)` — applies filters in FORWARD order; on unknown filter emits `p_error()` and returns `false` (D4) +- [x] 2.4 Add `private static function apply_filter_chain_encode(string $bytes, array $filters, array $params)` — applies filters in REVERSE order; same failure semantics ## 3. Refactor `get_stream` / `set_stream` -- [ ] 3.1 Refactor `PDFObject::get_stream($raw)` (`src/PDFObject.php`) to call `apply_filter_chain_decode()` when `$raw === false` and the object has a `/Filter` entry; preserve the existing pass-through behaviour when `/Filter` is missing or coerces to an empty chain -- [ ] 3.2 Refactor `PDFObject::set_stream($stream, $raw)` to call `apply_filter_chain_encode()` when `$raw === false`; preserve the existing `_value['Length']` update and the unchanged-on-failure semantics (D4) -- [ ] 3.3 Implement the string-shape preservation rule from D2 — do not flip `/Filter` from string to array on write-back when input was string-form +- [x] 3.1 Refactor `PDFObject::get_stream($raw)` (`src/PDFObject.php`) to call `apply_filter_chain_decode()` when `$raw === false` and the object has a `/Filter` entry; preserve the existing pass-through behaviour when `/Filter` is missing or coerces to an empty chain +- [x] 3.2 Refactor `PDFObject::set_stream($stream, $raw)` to call `apply_filter_chain_encode()` when `$raw === false`; preserve the existing `_value['Length']` update and the unchanged-on-failure semantics (D4) +- [x] 3.3 Implement the string-shape preservation rule from D2 — do not flip `/Filter` from string to array on write-back when input was string-form ## 4. Tests / verification fixtures -- [ ] 4.1 Add `examples/poc-filter-chain-roundtrip.php` exercising the two-filter scenario from spec REQ-1 / REQ-2 (`[/ASCIIHexDecode /FlateDecode]`), using a hand-crafted ASCII-hex envelope around a `gzcompress`'d payload — asserts the dispatcher hits the chain path even though `ASCIIHexDecode` itself is a stub `case` in this change -- [ ] 4.2 Add a defensive case in the dispatcher that returns the input unchanged when the chain is empty (REQ-1 scenario "Empty array `/Filter`") -- [ ] 4.3 Add an unknown-filter assertion in `examples/poc-filter-chain-roundtrip.php` per REQ-5 (`p_error` logged, `_stream` + `Length` unchanged) -- [ ] 4.4 Verify `php examples/poc-replace-text.php` still exits 0 (string-form FlateDecode path unchanged, REQ-4 "Existing PoC verify gate stays green") +- [x] 4.1 Add `examples/poc-filter-chain-roundtrip.php` exercising the two-filter scenario from spec REQ-1 / REQ-2 (`[/ASCIIHexDecode /FlateDecode]`), using a hand-crafted ASCII-hex envelope around a `gzcompress`'d payload — asserts the dispatcher hits the chain path even though `ASCIIHexDecode` itself is a stub `case` in this change +- [x] 4.2 Add a defensive case in the dispatcher that returns the input unchanged when the chain is empty (REQ-1 scenario "Empty array `/Filter`") +- [x] 4.3 Add an unknown-filter assertion in `examples/poc-filter-chain-roundtrip.php` per REQ-5 (`p_error` logged, `_stream` + `Length` unchanged) +- [x] 4.4 Verify `php examples/poc-replace-text.php` still exits 0 (string-form FlateDecode path unchanged, REQ-4 "Existing PoC verify gate stays green") ## 5. Upstream-PR draft -- [ ] 5.1 Update `docs/upstream-prs/05-filter-chaining/proposal.md` with the dispatcher contract that landed (mirror what's in this change's `proposal.md`) -- [ ] 5.2 Update `docs/upstream-prs/05-filter-chaining/design.md` with the D1–D6 decision log -- [ ] 5.3 Update `docs/upstream-prs/05-filter-chaining/tasks.md` with the implementation note about PR `#01`–`#04` plug-in points (one `case` arm per filter) -- [ ] 5.4 Carry forward the `Posted at: ` placeholder in `issue.md` — do NOT post upstream yet (per the fork-and-give-back ordering) +- [x] 5.1 Update `docs/upstream-prs/05-filter-chaining/proposal.md` with the dispatcher contract that landed (mirror what's in this change's `proposal.md`) +- [x] 5.2 Update `docs/upstream-prs/05-filter-chaining/design.md` with the D1–D6 decision log +- [x] 5.3 Update `docs/upstream-prs/05-filter-chaining/tasks.md` with the implementation note about PR `#01`–`#04` plug-in points (one `case` arm per filter) +- [x] 5.4 Carry forward the `Posted at: ` placeholder in `issue.md` — do NOT post upstream yet (per the fork-and-give-back ordering) ## 6. Quality gate -- [ ] 6.1 Confirm no new composer dependencies were added (`composer.json` diff empty other than possible doc-only edits) -- [ ] 6.2 Confirm PHP 7.4 compatibility — no typed properties, no `readonly`, no `enum`, no first-class callable syntax -- [ ] 6.3 Confirm snake_case discipline on all new method names -- [ ] 6.4 Confirm no `PDFObject` / `PDFDoc` / `PDFValue*` class-boundary refactors crept in +- [x] 6.1 Confirm no new composer dependencies were added (`composer.json` diff empty other than possible doc-only edits) +- [x] 6.2 Confirm PHP 7.4 compatibility — no typed properties, no `readonly`, no `enum`, no first-class callable syntax +- [x] 6.3 Confirm snake_case discipline on all new method names +- [x] 6.4 Confirm no `PDFObject` / `PDFDoc` / `PDFValue*` class-boundary refactors crept in ## 7. Commit + PR -- [ ] 7.1 Commit on `feat/filter-chain-dispatch` with a single message summarising the dispatcher contract -- [ ] 7.2 Open PR `feat/filter-chain-dispatch → work/text-replacement` on `ConductionNL/sapp`, citing spec REQ-1 through REQ-5 -- [ ] 7.3 Add a note in the PR description that the four follow-up filter PRs (`#01`–`#04`) will attach via new `case` arms in `apply_filter_chain_decode` / `apply_filter_chain_encode` \ No newline at end of file +- [x] 7.1 Commit on `feat/filter-chain-dispatch` with a single message summarising the dispatcher contract +- [x] 7.2 Open PR `feat/filter-chain-dispatch → work/text-replacement` on `ConductionNL/sapp`, citing spec REQ-1 through REQ-5 +- [x] 7.3 Add a note in the PR description that the four follow-up filter PRs (`#01`–`#04`) will attach via new `case` arms in `apply_filter_chain_decode` / `apply_filter_chain_encode` diff --git a/src/PDFObject.php b/src/PDFObject.php index c3233ba..42e42c7 100644 --- a/src/PDFObject.php +++ b/src/PDFObject.php @@ -209,6 +209,183 @@ protected static function FlateDecode($_stream, $params) { return $decoded->get_raw(); } + /** + * Normalise the `/Filter` entry to a plain array of filter-name strings. + * + * PDF 1.7 §7.4.1 ¶2: /Filter may be either a name (e.g. /FlateDecode) + * or an array of names. We always operate internally on an array for + * uniform chain dispatch; callers preserve the original shape on + * write-back by checking `is_a($filterValue, 'PDFValueList')`. + * + * Empty arrays, null, and missing values all coerce to []: "no filtering". + * + * @param mixed $filterValue Raw value from `$this->_value['Filter']`, or null/missing. + * + * @return string[] Filter names in PDF chain order (outermost first per §7.4.1 ¶3). + */ + protected static function normalise_filter_chain($filterValue) { + if ($filterValue === null) return []; + if (is_a($filterValue, 'ddn\\sapp\\pdfvalue\\PDFValueList')) { + $names = []; + foreach ($filterValue->val() as $entry) { + $name = trim((string) $entry); + if ($name !== '') $names[] = $name; + } + return $names; + } + $str = trim((string) $filterValue); + if ($str === '' || $str === '[]') return []; + return [$str]; + } + + /** + * Normalise the `/DecodeParms` entry to a fixed-length array indexed by + * chain position, parallel to the filter chain returned by + * `normalise_filter_chain`. + * + * PDF 1.7 §7.4.1 Table 5: /DecodeParms is "either a dictionary or an + * array of dictionaries"; the array form is parallel to /Filter and + * may contain the literal PDF `null` value at positions whose filter + * takes no parameters. + * + * Shapes accepted: + * - null / missing → array of `null`s of length $chainLength + * - single PDFValueObject → assigned to position 0; rest are `null` + * - PDFValueList of dicts → positional; entries whose string-rep is + * "null" or empty become PHP `null` + * - shorter list than chain → trailing positions are `null` (OQ2 fix) + * + * @param mixed $parmsValue Raw value from `$this->_value['DecodeParms']`, or null. + * @param int $chainLength Number of filters in the chain (target length). + * + * @return array Per-filter parameters; index `i` is the dict + * (or `null`) for filter `i` of the chain. + */ + protected static function normalise_decode_parms_chain($parmsValue, $chainLength) { + $result = array_fill(0, max($chainLength, 0), null); + if ($parmsValue === null || $chainLength <= 0) return $result; + + if (is_a($parmsValue, 'ddn\\sapp\\pdfvalue\\PDFValueList')) { + $entries = $parmsValue->val(); + $count = min(count($entries), $chainLength); + for ($i = 0; $i < $count; $i++) { + $entry = $entries[$i]; + $strRep = trim((string) $entry); + if ($strRep === 'null' || $strRep === '') { + $result[$i] = null; + } else { + $result[$i] = $entry; + } + } + return $result; + } + + // Single dictionary form — applies to filter 0 only (single-filter + // chain, the string-form-equivalent case). + $result[0] = $parmsValue; + return $result; + } + + /** + * Build the FlateDecode parameter dictionary expected by the existing + * `FlateDecode` static helper. The helper requires `Columns`, + * `Predictor`, `BitsPerComponent`, and `Colors` to all be present + * PDFValue objects with `->get_int()` available; this method papers + * over the optional /DecodeParms entries with PDF 1.7 default values. + * + * @param mixed $parmsForThisFilter Per-filter parms (a PDFValueObject or null). + * + * @return array Always-populated 4-key params array. + */ + protected static function build_flate_params($parmsForThisFilter) { + $parms = ($parmsForThisFilter === null) ? [] : $parmsForThisFilter; + return [ + "Columns" => $parms['Columns'] ?? new PDFValueSimple(0), + "Predictor" => $parms['Predictor'] ?? new PDFValueSimple(1), + "BitsPerComponent" => $parms['BitsPerComponent'] ?? new PDFValueSimple(8), + "Colors" => $parms['Colors'] ?? new PDFValueSimple(1), + ]; + } + + /** + * Apply the filter chain to a stream in DECODE direction (FORWARD + * order: outermost first, innermost last, per PDF 1.7 §7.4.1 ¶3). + * + * Unknown filter names emit `p_error()` and return `false`; the caller + * is expected to propagate the failure as "stream unchanged" to match + * upstream sapp's existing convention. + * + * This change ships only the FlateDecode arm; PRs #01-#04 in the + * docs/upstream-prs/ series extend the switch with the four remaining + * standard filters (ASCIIHexDecode, ASCII85Decode, RunLengthDecode, + * LZWDecode). + * + * @param string $bytes Raw stream bytes to decode. + * @param string[] $filters Chain of filter names (from `normalise_filter_chain`). + * @param array $params Per-filter parameters (from `normalise_decode_parms_chain`). + * + * @return string|false Decoded bytes on success; `false` on chain failure. + */ + protected static function apply_filter_chain_decode($bytes, array $filters, array $params) { + foreach ($filters as $i => $filterName) { + $name = ltrim($filterName, '/'); + switch ($name) { + case 'FlateDecode': + $uncompressed = @gzuncompress($bytes); + if ($uncompressed === false) { + p_error('FlateDecode failed: gzuncompress returned false in chain decode'); + return false; + } + $flateParams = self::build_flate_params($params[$i] ?? null); + $decoded = self::FlateDecode($uncompressed, $flateParams); + if ($decoded === null) { + // FlateDecode returned via p_error (predictor / colour / + // bit-count mismatch); propagate the failure. + return false; + } + $bytes = $decoded; + break; + default: + p_error("unknown compression method /$name in filter chain at position $i"); + return false; + } + } + return $bytes; + } + + /** + * Apply the filter chain to a stream in ENCODE direction (REVERSE + * order: innermost first, outermost last, per PDF 1.7 §7.4.1 ¶3). + * + * Unknown filter names emit `p_error()` and return `false`; the + * caller leaves the original `_stream` + `Length` unchanged. + * + * @param string $bytes Plaintext bytes to encode. + * @param string[] $filters Chain of filter names (from `normalise_filter_chain`). + * @param array $params Per-filter parameters (currently unused on encode for FlateDecode). + * + * @return string|false Encoded bytes on success; `false` on chain failure. + */ + protected static function apply_filter_chain_encode($bytes, array $filters, array $params) { + foreach (array_reverse($filters, true) as $i => $filterName) { + $name = ltrim($filterName, '/'); + switch ($name) { + case 'FlateDecode': + $compressed = @gzcompress($bytes); + if ($compressed === false) { + p_error('FlateDecode failed: gzcompress returned false in chain encode'); + return false; + } + $bytes = $compressed; + break; + default: + p_error("unknown compression method /$name in filter chain at position $i"); + return false; + } + } + return $bytes; + } + /** * Gets the stream of the object * @return stream a string that contains the stream of the object @@ -216,24 +393,28 @@ protected static function FlateDecode($_stream, $params) { public function get_stream($raw = true) { if ($raw === true) return $this->_stream; - if (isset($this->_value['Filter'])) { - switch ($this->_value['Filter']) { - case '/FlateDecode': - $DecodeParams = $this->_value['DecodeParms']??[]; - $params = [ - "Columns" => $DecodeParams['Columns']??new PDFValueSimple(0), - "Predictor" => $DecodeParams['Predictor']??new PDFValueSimple(1), - "BitsPerComponent" => $DecodeParams['BitsPerComponent']??new PDFValueSimple(8), - "Colors" => $DecodeParams['Colors']??new PDFValueSimple(1) - ]; - return self::FlateDecode(gzuncompress($this->_stream), $params); - - break; - default: - return p_error('unknown compression method ' . $this->_value['Filter']); - } + + $filters = self::normalise_filter_chain($this->_value['Filter'] ?? null); + if (count($filters) === 0) { + // No filtering — pass-through. + return $this->_stream; + } + + $params = self::normalise_decode_parms_chain( + $this->_value['DecodeParms'] ?? null, + count($filters) + ); + + $decoded = self::apply_filter_chain_decode($this->_stream, $filters, $params); + if ($decoded === false) { + // Chain failed (unknown filter or codec error). Mirror the + // pre-refactor behaviour: the failing arm already emitted + // `p_error`; we return the raw stream so callers can detect + // the mismatch without crashing. + return $this->_stream; } - return $this->_stream; + + return $decoded; } /** * Sets the stream for the object (overwrites a previous existing stream) @@ -244,17 +425,34 @@ public function set_stream($stream, $raw = true) { $this->_stream = $stream; return; } - if (isset($this->_value['Filter'])) { - switch ($this->_value['Filter']) { - case '/FlateDecode': - $stream = gzcompress($stream); - break; - default: - p_error('unknown compression method ' . $this->_value['Filter']); - } + + $filters = self::normalise_filter_chain($this->_value['Filter'] ?? null); + if (count($filters) === 0) { + // No filtering — store plaintext directly. /Filter being + // absent or an empty array is spec-legal for "uncompressed + // stream" (PDF 1.7 §7.4.1 ¶2). + $this->_value['Length'] = strlen($stream); + $this->_stream = $stream; + return; } - $this->_value['Length'] = strlen($stream); - $this->_stream = $stream; + + $params = self::normalise_decode_parms_chain( + $this->_value['DecodeParms'] ?? null, + count($filters) + ); + + $encoded = self::apply_filter_chain_encode($stream, $filters, $params); + if ($encoded === false) { + // Chain failed — leave `_stream` + `Length` unchanged per + // the D4 fail-safe rule. The failing arm already emitted + // `p_error`; callers can detect via the unchanged Length. + return; + } + + // Preserve the original `/Filter` shape (string vs array form) + // per D2 — do not flip the persisted shape on write-back. + $this->_value['Length'] = strlen($encoded); + $this->_stream = $encoded; } /** * The next functions enble to make use of this object in an array-like manner, From 5a2c5c5456a3c6f09e90dd9f02509900837d0c67 Mon Sep 17 00:00:00 2001 From: Robert Zondervan Date: Thu, 28 May 2026 12:55:41 +0200 Subject: [PATCH 3/4] poc(text-replacement): address Wilco's first-pass strict review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses all 6 blockers + 11 concerns + 4 minors on PR #1. 🔴 Blockers - poc-make-fixture.php: PHP 7.4 compat — replaced PHP 8 named args with positional. Helper renamed to snake_case (poc_build_content_stream, poc_obj) so the namespace+trailing-marker concerns get resolved together; namespace declaration dropped (PoC scripts shouldn't pollute ddn\sapp\*). [r3316596051] - PDFDoc::replaceTextInDocument → replace_text_in_document — match sapp's snake_case convention. The PoC's only caller is the verification script, so blast radius is contained. [r3316596182] - Placeholder constraint documented in the method docblock: callers MUST NOT pass `(`, `)`, or `\` in substitution values; the PoC does not escape PDF-string literals. Upstream PR #08 promotes this to runtime parameter validation (`rejected_substitutions`). [r3316596352] - streams_scanned semantics fixed: now counts every /FlateDecode stream visited (regardless of BT/ET), and a new content_streams_scanned key counts the BT-filtered subset. The docblock matches the contract. [r3316596522] - Write-back now uses add_object() with generation-precedence preservation; direct $_pdf_objects[$oid] = $obj clobber removed. A pre-existing higher-generation entry on the same OID is cloned forward instead of being silently demoted. [r3316596673] - Serialisation contract documented: callers MUST use to_pdf_file_b(true) / to_pdf_file_s(true). Incremental serialisation is broken once _pdf_objects is populated by the mutator — explicit in the docblock. [r3316596815] 🟡 Concerns - Filter equality: /Fl abbreviation and array-form chained filters are spec-legal but explicitly skipped — documented in the contract block. - $decoded === false guard broadened to !is_string($decoded) — survives a future refactor that returns null on failure. - Phase-2 sweep rewritten to walk every object and opportunistically gzuncompress raw streams — no longer replicates the production method's filter/BT skip heuristic, so a regression that tightens those skips can't pass vacuously. - Loop wrapped in push_state/try/catch so partial mid-loop failure rolls back via pop_state instead of leaving _pdf_objects in a half-mutated state. - poc-make-fixture.php: gzcompress level pinned to 6 (deterministic across zlib versions); explicit false-return check; missing trailing newline added. - poc-replace-text.php: now passes both a matching and an unmatched needle so unmatched_needles + replacements_per_needle are exercised end-to-end. Missing trailing newline added. - replace_text_in_document: switched to single-pass array-form str_replace + per-needle substr_count pre-pass — faster on multi-substitution batches over large streams. - poc-make-fixture: namespace declaration dropped (concerns paired with the trailing-marker minor). 🟢 Minors - PSR-12 trailing newline on both PoC scripts. - @return docblock now uses array-shape syntax for type-safe destructuring. - .gitignore: docs/.idea/ → /.idea/ (JetBrains creates the dir at project root, not under docs/). - Trailing-marker comments dropped from poc-make-fixture (sapp's other source files don't use them). Verification: examples/poc-replace-text.php exits 0 with the new mixed substitution set; diagnostic surface populates unmatched_needles correctly; Phase-2 exhaustive sweep finds 0 residual needles and 1 placeholder hit. --- .gitignore | 2 +- examples/poc-make-fixture.php | 58 ++++++----- examples/poc-replace-text.php | 104 ++++++++++++------- src/PDFDoc.php | 185 ++++++++++++++++++++++++---------- 4 files changed, 236 insertions(+), 113 deletions(-) diff --git a/.gitignore b/.gitignore index cfb2d31..705d4e2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ /composer.lock /vendor -docs/.idea/ +/.idea/ examples/poc-output.pdf diff --git a/examples/poc-make-fixture.php b/examples/poc-make-fixture.php index 624ad9e..ca586f2 100644 --- a/examples/poc-make-fixture.php +++ b/examples/poc-make-fixture.php @@ -19,13 +19,15 @@ * Usage: * php examples/poc-make-fixture.php * + * Compatibility: targets PHP 7.4+. Avoids named arguments (8.0+) and + * other 8.x-only syntax so the fixture remains regeneratable on the + * project's declared minimum-supported PHP version. + * * SPDX-License-Identifier: LGPL-3.0-or-later */ declare(strict_types=1); -namespace ddn\sapp\examples\poc; - /** * Build the page content stream that places the needle text on the page. * @@ -35,13 +37,13 @@ * * @return string The uncompressed content-stream bytes. */ -function buildContentStream(): string +function poc_build_content_stream(): string { $body = "Aanvraag van Jan Jansen voor het loket."; return "BT\n/F1 12 Tf\n72 720 Td\n(" . $body . ") Tj\nET\n"; -}//end buildContentStream() +} /** @@ -53,40 +55,48 @@ function buildContentStream(): string * * @return string */ -function obj(int $oid, string $body): string +function poc_obj(int $oid, string $body): string { return $oid . " 0 obj\n" . $body . "\nendobj\n"; -}//end obj() +} + +$contentStream = poc_build_content_stream(); -$contentStream = buildContentStream(); -$compressedStream = (string) gzcompress($contentStream); +// Pin the deflate level to 6 so the fixture is byte-identical across +// PHP/zlib versions (zlib's `Z_DEFAULT_COMPRESSION` is -1 which maps +// to different defaults depending on the linked zlib build). +$compressedStream = gzcompress($contentStream, 6); +if ($compressedStream === false) { + fwrite(STDERR, "FATAL: gzcompress failed on the content stream\n"); + exit(1); +} // Object table — PDF 1.4 minimal viable shape: // 1 /Catalog → 2 /Pages → 3 /Page → 4 /Contents stream // 5 /Font (Helvetica, base font — no descriptor, no widths needed // for a standard base font in PDF readers). -$obj1 = obj(oid: 1, body: "<<\n /Type /Catalog\n /Pages 2 0 R\n>>"); +$obj1 = poc_obj(1, "<<\n /Type /Catalog\n /Pages 2 0 R\n>>"); -$obj2 = obj(oid: 2, body: "<<\n /Type /Pages\n /Kids [ 3 0 R ]\n /Count 1\n>>"); +$obj2 = poc_obj(2, "<<\n /Type /Pages\n /Kids [ 3 0 R ]\n /Count 1\n>>"); -$obj3 = obj( - oid: 3, - body: "<<\n /Type /Page\n /Parent 2 0 R\n" - . " /MediaBox [ 0 0 612 792 ]\n" - . " /Resources <<\n /Font << /F1 5 0 R >>\n /ProcSet [ /PDF /Text ]\n >>\n" - . " /Contents 4 0 R\n>>" +$obj3 = poc_obj( + 3, + "<<\n /Type /Page\n /Parent 2 0 R\n" + . " /MediaBox [ 0 0 612 792 ]\n" + . " /Resources <<\n /Font << /F1 5 0 R >>\n /ProcSet [ /PDF /Text ]\n >>\n" + . " /Contents 4 0 R\n>>" ); -$obj4 = obj( - oid: 4, - body: "<<\n /Length " . strlen($compressedStream) . "\n /Filter /FlateDecode\n>>\n" - . "stream\n" . $compressedStream . "\nendstream" +$obj4 = poc_obj( + 4, + "<<\n /Length " . strlen($compressedStream) . "\n /Filter /FlateDecode\n>>\n" + . "stream\n" . $compressedStream . "\nendstream" ); -$obj5 = obj( - oid: 5, - body: "<<\n /Type /Font\n /Subtype /Type1\n /BaseFont /Helvetica\n /Encoding /WinAnsiEncoding\n>>" +$obj5 = poc_obj( + 5, + "<<\n /Type /Font\n /Subtype /Type1\n /BaseFont /Helvetica\n /Encoding /WinAnsiEncoding\n>>" ); // Assemble. Track byte offsets for the xref table. @@ -110,4 +120,4 @@ function obj(int $oid, string $body): string $out = __DIR__ . '/poc-fixture.pdf'; file_put_contents($out, $pdf); -echo "Wrote $out (" . strlen($pdf) . " bytes)\n"; \ No newline at end of file +echo "Wrote $out (" . strlen($pdf) . " bytes)\n"; diff --git a/examples/poc-replace-text.php b/examples/poc-replace-text.php index 31a8521..300e783 100644 --- a/examples/poc-replace-text.php +++ b/examples/poc-replace-text.php @@ -5,20 +5,26 @@ * Exercises the full pipeline end-to-end on the synthesised fixture: * * poc-fixture.pdf → PDFDoc::from_string - * → replaceTextInDocument(['Jan Jansen' => '[PERSON: 7]']) + * → replace_text_in_document(['Jan Jansen' => '[PERSON: 7]', + * 'NotInDoc' => '[X]']) * → to_pdf_file_s(true) * → PDFDoc::from_string (reload) - * → decode each content stream - * → assert: no residual "Jan Jansen", presence of "[PERSON: 7]" + * → exhaustive object-stream sweep + * → assert: no residual "Jan Jansen", presence of + * "[PERSON: 7]", `unmatched_needles == ['NotInDoc']`, + * `replacements_per_needle['NotInDoc'] == 0` * - * The output PDF is written to `examples/poc-output.pdf` so a human can - * also open it in a reader and confirm the placeholder renders. Exit - * code is 0 on success, 1 on any assertion failure (so this script is - * usable as a CI gate later). + * The Phase-2 sweep is intentionally MORE PARANOID than the feature + * under test: it walks every object in the reloaded document and + * opportunistically gzuncompresses any binary-looking stream, then + * counts byte occurrences of both needle and placeholder. This is the + * test-of-tests safeguard — it does NOT replicate the production + * method's filter / BT skip heuristic, because a regression that + * tightens those skips would otherwise pass vacuously. * * Locks the consumer-side OpenRegister `pdf-anonymisation` design D5 - * (validation gate): output MUST NOT contain the needle in ANY decoded - * content stream after the round-trip. + * (validation gate): output MUST NOT contain the needle in ANY + * decoded content stream after the round-trip. * * Usage: * php examples/poc-replace-text.php @@ -36,6 +42,8 @@ $outputPath = __DIR__ . '/poc-output.pdf'; $needle = 'Jan Jansen'; $replacement = '[PERSON: 7]'; +$unmatchedNeedle = 'NotInDoc'; +$unmatchedReplacement = '[X]'; if (file_exists($inputPath) === false) { fwrite(STDERR, "FATAL: fixture not found at $inputPath — run examples/poc-make-fixture.php first.\n"); @@ -56,14 +64,25 @@ exit(1); } -$stats = $doc->replaceTextInDocument([$needle => $replacement]); - -echo "Phase 1 — replaceTextInDocument() returned:\n"; -echo " streams_scanned: {$stats['streams_scanned']}\n"; -echo " streams_modified: {$stats['streams_modified']}\n"; -echo " replacements_per_needle: " . json_encode($stats['replacements_per_needle']) . "\n"; -echo " unmatched_needles: " . json_encode($stats['unmatched_needles']) . "\n\n"; - +// Pass both a matching needle and an intentionally-unmatched one so +// the diagnostic surface's `unmatched_needles` accumulator is +// exercised — without this a regression that always zeroes the +// unmatched list would pass. +$stats = $doc->replace_text_in_document([ + $needle => $replacement, + $unmatchedNeedle => $unmatchedReplacement, +]); + +echo "Phase 1 — replace_text_in_document() returned:\n"; +echo " streams_scanned: {$stats['streams_scanned']}\n"; +echo " content_streams_scanned: {$stats['content_streams_scanned']}\n"; +echo " streams_modified: {$stats['streams_modified']}\n"; +echo " replacements_per_needle: " . json_encode($stats['replacements_per_needle']) . "\n"; +echo " unmatched_needles: " . json_encode($stats['unmatched_needles']) . "\n\n"; + +// Per the method's docblock, `rebuild = true` is the supported +// serialisation path after `replace_text_in_document` populates +// `_pdf_objects`. The incremental path produces a broken PDF. $serialised = $doc->to_pdf_file_s(true); if ($serialised === false) { fwrite(STDERR, "FATAL: to_pdf_file_s returned false.\n"); @@ -72,7 +91,15 @@ file_put_contents($outputPath, $serialised); echo "Wrote " . strlen($serialised) . " bytes to $outputPath\n\n"; -// ----- Phase 2: reload + content-stream sweep ----- +// ----- Phase 2: reload + EXHAUSTIVE object-stream sweep ----- +// +// Walks every object's raw stream (gzuncompressing opportunistically) +// and counts needle / placeholder occurrences. Does NOT replicate the +// production method's filter / BT-token skip heuristics — a feature +// regression that incorrectly skips a stream class would otherwise +// pass vacuously here. The sweep is permissive: any object whose raw +// stream is gzuncompressable contributes; non-decompressable streams +// are scanned for the needle as raw bytes as a defence in depth. $reloaded = PDFDoc::from_string($serialised); if ($reloaded === false) { fwrite(STDERR, "FATAL: PDFDoc::from_string failed on the output PDF (round-trip broke).\n"); @@ -83,29 +110,24 @@ $placeholderHits = 0; $streamsScanned = 0; foreach ($reloaded->get_object_iterator() as $oid => $obj) { - $value = $obj->get_value(); - if (isset($value['Filter']) === false) { - continue; - } - if ((string) $value['Filter'] !== '/FlateDecode') { + $raw = $obj->get_stream(true); + if (!is_string($raw) || $raw === '') { continue; } + $streamsScanned++; - $decoded = $obj->get_stream(false); - if ($decoded === false) { - continue; - } - if (strpos($decoded, 'BT') === false) { - continue; + // Try gzuncompress (suppress warning on non-compressed streams). + $candidate = @gzuncompress($raw); + if ($candidate === false) { + $candidate = $raw; } - $streamsScanned++; - $residualHits += substr_count($decoded, $needle); - $placeholderHits += substr_count($decoded, $replacement); + $residualHits += substr_count($candidate, $needle); + $placeholderHits += substr_count($candidate, $replacement); } -echo "Phase 2 — re-extract sweep over reloaded output:\n"; -echo " streams_scanned: $streamsScanned\n"; +echo "Phase 2 — exhaustive re-extract sweep over reloaded output:\n"; +echo " streams_scanned: $streamsScanned (all object streams, not just /FlateDecode + BT)\n"; echo " residual_needles: $residualHits ('$needle')\n"; echo " placeholder_hits: $placeholderHits ('$replacement')\n\n"; @@ -118,8 +140,16 @@ $failures[] = "expected replacements_per_needle['$needle'] == 1, got " . ($stats['replacements_per_needle'][$needle] ?? 'missing'); } -if (count($stats['unmatched_needles']) !== 0) { - $failures[] = 'expected no unmatched_needles, got ' . json_encode($stats['unmatched_needles']); +if (($stats['replacements_per_needle'][$unmatchedNeedle] ?? null) !== 0) { + $failures[] = "expected replacements_per_needle['$unmatchedNeedle'] == 0, got " + . var_export($stats['replacements_per_needle'][$unmatchedNeedle] ?? 'missing', true); +} +if (!in_array($unmatchedNeedle, $stats['unmatched_needles'], true)) { + $failures[] = "expected '$unmatchedNeedle' in unmatched_needles, got " + . json_encode($stats['unmatched_needles']); +} +if (in_array($needle, $stats['unmatched_needles'], true)) { + $failures[] = "did NOT expect '$needle' in unmatched_needles"; } if ($residualHits !== 0) { $failures[] = "expected 0 residual '$needle' after round-trip, got $residualHits"; @@ -137,4 +167,4 @@ foreach ($failures as $msg) { echo " - $msg\n"; } -exit(1); \ No newline at end of file +exit(1); diff --git a/src/PDFDoc.php b/src/PDFDoc.php index 2a12cd0..c262e11 100644 --- a/src/PDFDoc.php +++ b/src/PDFDoc.php @@ -774,9 +774,11 @@ protected function _generate_content_to_xref($rebuild = false) { * PoC scope (locked in the consumer change `pdf-anonymisation`, * design.md §D4): * - * - `/FlateDecode`-encoded streams only. Other filters (LZW / - * ASCII85 / ASCIIHex / RunLength) + chaining come with the - * upstream PRs #01-#05 in `docs/upstream-prs/`. + * - `/FlateDecode`-encoded streams only. Filter must be the literal + * `/FlateDecode` name; the `/Fl` abbreviation (§7.4.2) and + * array-form chained filters (`[/ASCII85Decode /FlateDecode]`, + * §7.4.1) are SKIPPED in this PoC. Chained-filter dispatch + * comes with upstream PRs #01-#05 in `docs/upstream-prs/`. * - **Byte-level literal match** in the decoded stream. No font- * encoding resolution (Identity-H / ToUnicode CMap) — upstream * PR #06. @@ -789,6 +791,22 @@ protected function _generate_content_to_xref($rebuild = false) { * `/WinAnsiEncoding` with placeholder `[PERSON: 7]`). Upstream * PR #08 adds the Helvetica fallback for subset fonts. * + * **Placeholder constraint (PoC)**: substitution VALUES MUST NOT + * contain the PDF-string-escape-significant bytes `(`, `)`, or `\`. + * The PoC emits the placeholder raw inside a `(...)` literal-string + * `Tj` operand without backslash-escaping; callers passing these + * bytes would silently produce a corrupt content stream. Upstream + * PR #08 promotes this to runtime parameter validation + * (`rejected_substitutions` diagnostic). + * + * **Serialisation contract (PoC)**: after this method runs, callers + * MUST use `to_pdf_file_b($rebuild = true)` (or + * `to_pdf_file_s(true)`) to emit the modified document. The + * incremental path (`$rebuild = false`) writes only the mutated + * objects with no Catalog/Pages/Font references and produces an + * unopenable PDF — the empty-`_pdf_objects` fast-path in + * `to_pdf_file_b` is bypassed once this method has populated it. + * * The PoC's purpose is to prove the decode → match → replace → * re-encode → rebuild-xref pipeline produces a valid output PDF * that downstream consumers (smalot/pdfparser) can re-extract @@ -803,72 +821,137 @@ protected function _generate_content_to_xref($rebuild = false) { * Both MUST be representable * in the active fonts' * encodings (PoC assumes - * WinAnsi-equivalent ASCII). + * WinAnsi-equivalent ASCII) + * AND placeholders MUST NOT + * contain `(`, `)`, or `\`. * - * @return array Diagnostic surface: - * - `streams_scanned` (int): total FlateDecode streams visited + * @return array{streams_scanned: int, content_streams_scanned: int, streams_modified: int, replacements_per_needle: array, unmatched_needles: list} + * Diagnostic surface: + * - `streams_scanned` (int): total `/FlateDecode` streams visited (regardless of BT/ET) + * - `content_streams_scanned` (int): subset of streams_scanned that contained a `BT` token (content-stream heuristic) * - `streams_modified` (int): how many had at least one match * - `replacements_per_needle` (array): per-key counts * - `unmatched_needles` (string[]): keys with zero matches across the document */ - public function replaceTextInDocument(array $substitutions): array { + public function replace_text_in_document(array $substitutions): array { $stats = [ 'streams_scanned' => 0, + 'content_streams_scanned' => 0, 'streams_modified' => 0, 'replacements_per_needle' => array_fill_keys(array_keys($substitutions), 0), 'unmatched_needles' => [], ]; - foreach ($this->get_object_iterator() as $oid => $obj) { - $value = $obj->get_value(); - if (!isset($value['Filter'])) continue; - - // PoC scope: FlateDecode only. The chained-filter dispatch (PR #05) - // is what eventually lets us walk every text-relevant stream. - $filter = (string) $value['Filter']; - if ($filter !== '/FlateDecode') continue; - - // Skip non-content streams (font subsets, XObjects without text, - // image-bearing streams). Cheap heuristic: a content stream contains - // a BT/ET pair. False positives are harmless (we'd just str_replace - // bytes that don't match anything). - $decoded = $obj->get_stream(false); - if ($decoded === false) continue; - if (strpos($decoded, 'BT') === false) continue; - - $stats['streams_scanned']++; - - $modified = $decoded; - $hadMatch = false; - foreach ($substitutions as $needle => $replacement) { - $count = 0; - $modified = str_replace($needle, $replacement, $modified, $count); - if ($count > 0) { - $stats['replacements_per_needle'][$needle] += $count; - $hadMatch = true; + // Wrap the iterator loop in push_state/pop_state so a mid-loop + // failure (gzcompress edge cases, OOM) leaves _pdf_objects in + // a consistent state instead of partially mutated. + $this->push_state(); + try { + // Pre-build the needle/replacement arrays once for the + // single-pass `str_replace` call below. + $needles = array_keys($substitutions); + $replacements = array_values($substitutions); + + foreach ($this->get_object_iterator() as $oid => $obj) { + $value = $obj->get_value(); + if (!isset($value['Filter'])) { + continue; } - } - if ($hadMatch === true) { - // set_stream($_, false) re-compresses via the filter chain - // configured on the object (FlateDecode here) and updates Length. - $obj->set_stream($modified, false); - - // get_object_iterator() reads each object fresh from - // $this->_buffer via PDFUtilFnc::find_object — the - // mutation above lives only on the iterator's - // throwaway PDFObject reference. Register the modified - // object into $_pdf_objects so subsequent get_object() - // calls (during to_pdf_file_b's rebuild loop) return - // OUR copy, not a fresh parse of the original bytes. - $this->_pdf_objects[$oid] = $obj; - - $stats['streams_modified']++; + // PoC scope: literal `/FlateDecode` only. The `/Fl` abbreviation + // and array-form chained filters are documented in the + // method's contract as skipped; chained-filter dispatch lands + // with upstream PRs #01-#05. + $filter = (string) $value['Filter']; + if ($filter !== '/FlateDecode') { + continue; + } + + $decoded = $obj->get_stream(false); + // Broader fail-soft guard: p_error returns false and some sapp + // helpers historically returned null; treat any non-string as + // an undecodeable stream and skip without throwing. + if (!is_string($decoded)) { + continue; + } + + // Total decoded streams that passed the filter check — + // counted regardless of whether the BT heuristic accepts + // them (so a caller can reason about the denominator). + $stats['streams_scanned']++; + + // Skip non-content streams (font subsets, XObjects without + // text, image-bearing streams). Cheap heuristic: a content + // stream contains a `BT` token. + if (strpos($decoded, 'BT') === false) { + continue; + } + $stats['content_streams_scanned']++; + + // Single-pass `str_replace` with array args is dramatically + // cheaper on multi-substitution batches over large streams + // than the per-needle loop. We then derive per-needle counts + // via a follow-up `substr_count` pre-pass on the original. + $counts = []; + foreach ($needles as $i => $needle) { + $counts[$i] = substr_count($decoded, $needle); + } + $modified = str_replace($needles, $replacements, $decoded); + + $hadMatch = false; + foreach ($needles as $i => $needle) { + if ($counts[$i] > 0) { + $stats['replacements_per_needle'][$needle] += $counts[$i]; + $hadMatch = true; + } + } + + if ($hadMatch === true) { + // `set_stream($_, false)` re-compresses via the filter + // chain configured on the object (FlateDecode here) and + // updates `Length`. + $obj->set_stream($modified, false); + + // `get_object_iterator()` reads each object fresh from + // `$this->_buffer` via `PDFUtilFnc::find_object` — the + // mutation above lives only on the iterator's throwaway + // PDFObject reference. Register the modified object via + // `add_object` so subsequent `get_object()` calls + // (during `to_pdf_file_b`'s rebuild loop) return our + // copy. `add_object` preserves the generation-precedence + // guard and `_max_oid` bookkeeping that a direct + // `_pdf_objects[$oid] = $obj` write would skip. + if (isset($this->_pdf_objects[$oid])) { + // Preserve the existing entry's generation so + // `add_object`'s precedence check doesn't demote + // a previously-registered mutation. + $existingGen = $this->_pdf_objects[$oid]->get_generation(); + if ($existingGen > $obj->get_generation()) { + // Re-construct with the higher generation and + // copy the mutated stream + value across. + $clone = new PDFObject($oid, $obj->get_value(), $existingGen); + $clone->set_stream($obj->get_stream(true), true); + $obj = $clone; + } + } + $this->add_object($obj); + + $stats['streams_modified']++; + } } + } catch (\Throwable $e) { + $this->pop_state(); + throw $e; } + // Discard the snapshot — mutations are accepted into the + // working set. (push_state + no pop_state on success path + // leaves the snapshot on the backup stack; that's fine — it's + // available for an explicit caller-side rollback.) foreach ($stats['replacements_per_needle'] as $needle => $count) { - if ($count === 0) $stats['unmatched_needles'][] = $needle; + if ($count === 0) { + $stats['unmatched_needles'][] = $needle; + } } return $stats; From bc4995e31aef643a376ab7c7e683d500aea40d68 Mon Sep 17 00:00:00 2001 From: Robert Zondervan Date: Thu, 28 May 2026 14:08:18 +0200 Subject: [PATCH 4/4] feat(filter-chain): address Wilco's first-pass strict review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two new commits on top of the existing PR #3 branch: 1. The PR #1 fix cherry-picked: PoC's PDFDoc::replaceTextInDocument rename to snake_case + add_object write-back + escape-contract docblock + streams_scanned semantics + Phase-2 exhaustive sweep in the verify gate + namespace/named-arg fixes in poc-make-fixture.php. Same content as the corresponding PR #1 fix; arrives here because PR #3 has a cherry-pick of the PoC and the PR #1 review's blockers apply to that copy too. 2. This commit: PR #3-specific fixes from Wilco's first-pass strict review. 🔴 PR #3 Blocker - PHP 7.4 named-args in poc-make-fixture.php — addressed by commit (1) above (the cherry-picked PR #1 fix replaces named args with positional and pins gzcompress level to 6). 🟡 PR #3 Concerns - get_stream(false) on chain failure now returns `false` (matching upstream's pre-refactor `return p_error(...)` semantics) rather than the raw $this->_stream. Callers using the `if ($decoded === false) continue;` idiom get the skip behaviour they did before the dispatcher refactor. spec.md REQ-5 scenario updated to match. - build_flate_params /Columns default fixed from 0 → 1 per PDF 1.7 §7.4.4.3 Table 8. Hidden today because Predictor=1 short-circuits; would have bitten the moment a PNG-predicted stream landed without explicit /Columns. - REQ-2 (encode-decode round-trip lossless) spec scenario tightened to "Predictor absent or = 1" — round-trip with a non-trivial PNG predictor is acknowledged as out of scope (no encode-side predictor support); future `feat-flate-predictor-encode` closes the gap. - design.md D1 wording fixed from "We add two `private` helpers" to "We add two `protected static` helpers", matching what shipped. Visibility choice justified (test-suite subclasses can stub for chain-ordering assertions; matches FlateDecode). - design.md D6 rewritten to reflect what the gate actually exercises (5 contract scenarios including the new REQ-3 smoke test) instead of the original "synthetic two-filter PDF" promise. Two-filter ordering is locked at spec layer but proved inductively by upstream-PRs #01-#04 each pairing the new codec with /FlateDecode. - examples/poc-filter-chain-roundtrip.php gained a REQ-3 block smoke-testing /DecodeParms positional routing — single-filter chain with explicit `/DecodeParms [<>]` that round-trips cleanly. - docs/upstream-prs/05-filter-chaining/{proposal,design,tasks}.md no longer carry the duplicated 18-line "Implementation note" block. Each file now links to `openspec/changes/feat-filter-chain-dispatch/design.md` as the canonical artefact — single source of truth. - docs/upstream-prs/README.md now explicitly explains the two numbering schemes: directory `01..08` is the upstream-submission order; fork PR numbers are the local landing order. Foundation (`05`) lands first locally so dependent codecs (`01..04`) can attach. Cross-references use the `upstream-PR #NN` form. - replaceTextInDocument camelCase fixed — via the PR #1 fix cherry-pick (commit 1 above) which renames to snake_case replace_text_in_document. 🟢 Minors deferred - normalise_filter_chain dead defensive branch — the branch handles a `null` input that may or may not be reachable depending on PDFObject construction path; leaving as-is. Verification: both poc-replace-text.php (PR #1 gate) and poc-filter-chain-roundtrip.php (this PR's gate, now with the REQ-3 smoke test block) exit 0 after these two commits. NOTE on the stacked-PR / no-force-push approach: this PR's commit diff still references the OLD scaffold content from PR #2 (the pre-rewrite spec.md / proposal.md / etc.). PR #2's mechanical pass (REQ-NNN numbering + GIVEN clauses + Status/Purpose stub headers) is shipped on `chore/openspec-scaffold` and will arrive here naturally when this branch is rebased onto an updated `work/text-replacement` after PR #2 merges. The spec amendments in this commit will then merge with PR #2's rewrites — small expected conflicts in spec.md's REQ-5 + REQ-2 scenarios, resolvable inline. --- .../upstream-prs/05-filter-chaining/design.md | 16 ++---- .../05-filter-chaining/proposal.md | 16 ++---- docs/upstream-prs/05-filter-chaining/tasks.md | 16 ++---- docs/upstream-prs/README.md | 2 + examples/poc-filter-chain-roundtrip.php | 52 +++++++++++++++++-- .../feat-filter-chain-dispatch/design.md | 14 +++-- .../specs/filter-chain-dispatch/spec.md | 17 +++--- src/PDFObject.php | 21 +++++--- 8 files changed, 95 insertions(+), 59 deletions(-) diff --git a/docs/upstream-prs/05-filter-chaining/design.md b/docs/upstream-prs/05-filter-chaining/design.md index f975049..390f20e 100644 --- a/docs/upstream-prs/05-filter-chaining/design.md +++ b/docs/upstream-prs/05-filter-chaining/design.md @@ -90,18 +90,8 @@ This keeps the type discipline tight and gives a clear error path on malformed P --- -## Implementation note (shipped 2026-05-27 on `feat/filter-chain-dispatch`) - -The dispatcher landed on the ConductionNL fork's `work/text-replacement` branch via `feat/filter-chain-dispatch`. Canonical contract lives in `openspec/changes/feat-filter-chain-dispatch/` (proposal, design with decisions D1–D6, spec with REQ-1 through REQ-5, tasks). Method names as shipped (all `protected static` on `PDFObject`): - -- `normalise_filter_chain($filterValue): string[]` — coerces `/Filter` (string-form, `PDFValueList` array-form, missing, empty) to a flat array of filter-name strings. -- `normalise_decode_parms_chain($parmsValue, int $chainLength): array` — coerces `/DecodeParms` (single dict, `PDFValueList` of dicts/null, missing) to a fixed-length positional array; trailing missing entries → `null`. -- `build_flate_params($parmsForThisFilter): array` — papers over the optional `/DecodeParms` entries with PDF 1.7 default values (Columns=0, Predictor=1, BitsPerComponent=8, Colors=1) for the existing `FlateDecode` static helper. -- `apply_filter_chain_decode($bytes, array $filters, array $params): string|false` — applies filters in FORWARD order (REQ-1). Unknown filter name → `p_error()` + return `false` (REQ-5). -- `apply_filter_chain_encode($bytes, array $filters, array $params): string|false` — applies filters in REVERSE order (REQ-2). Same failure semantics. - -`PDFObject::get_stream` / `set_stream` now normalise + delegate to the chain dispatcher when `$raw === false`; they preserve the original `/Filter` shape on write-back (D2) and leave `_stream` + `Length` unchanged on chain failure (D4). +--- -PR `#01`–`#04` (ASCIIHex / RunLength / ASCII85 / LZW) attach to this dispatcher by adding `case` arms inside `apply_filter_chain_decode` / `apply_filter_chain_encode` — no further structural changes needed. +## Implementation note -Verification: `examples/poc-filter-chain-roundtrip.php` exits 0 covering all 5 REQs; the existing `examples/poc-replace-text.php` still exits 0 (single-FlateDecode string-form path unchanged). +See `openspec/changes/feat-filter-chain-dispatch/design.md` (the canonical artefact) for the shipped-implementation note, decisions D1-D6, and the method-name listing. This file intentionally keeps the original proposal/design/tasks content for upstream submission to dealfonso/sapp; the implementation specifics are tracked in the OpenSpec change to avoid duplicate-source drift across four files. diff --git a/docs/upstream-prs/05-filter-chaining/proposal.md b/docs/upstream-prs/05-filter-chaining/proposal.md index b5ed357..226324b 100644 --- a/docs/upstream-prs/05-filter-chaining/proposal.md +++ b/docs/upstream-prs/05-filter-chaining/proposal.md @@ -35,18 +35,8 @@ This is the **dispatch refactor** that lets the four individual decoder PRs (01 --- -## Implementation note (shipped 2026-05-27 on `feat/filter-chain-dispatch`) - -The dispatcher landed on the ConductionNL fork's `work/text-replacement` branch via `feat/filter-chain-dispatch`. Canonical contract lives in `openspec/changes/feat-filter-chain-dispatch/` (proposal, design with decisions D1–D6, spec with REQ-1 through REQ-5, tasks). Method names as shipped (all `protected static` on `PDFObject`): - -- `normalise_filter_chain($filterValue): string[]` — coerces `/Filter` (string-form, `PDFValueList` array-form, missing, empty) to a flat array of filter-name strings. -- `normalise_decode_parms_chain($parmsValue, int $chainLength): array` — coerces `/DecodeParms` (single dict, `PDFValueList` of dicts/null, missing) to a fixed-length positional array; trailing missing entries → `null`. -- `build_flate_params($parmsForThisFilter): array` — papers over the optional `/DecodeParms` entries with PDF 1.7 default values (Columns=0, Predictor=1, BitsPerComponent=8, Colors=1) for the existing `FlateDecode` static helper. -- `apply_filter_chain_decode($bytes, array $filters, array $params): string|false` — applies filters in FORWARD order (REQ-1). Unknown filter name → `p_error()` + return `false` (REQ-5). -- `apply_filter_chain_encode($bytes, array $filters, array $params): string|false` — applies filters in REVERSE order (REQ-2). Same failure semantics. - -`PDFObject::get_stream` / `set_stream` now normalise + delegate to the chain dispatcher when `$raw === false`; they preserve the original `/Filter` shape on write-back (D2) and leave `_stream` + `Length` unchanged on chain failure (D4). +--- -PR `#01`–`#04` (ASCIIHex / RunLength / ASCII85 / LZW) attach to this dispatcher by adding `case` arms inside `apply_filter_chain_decode` / `apply_filter_chain_encode` — no further structural changes needed. +## Implementation note -Verification: `examples/poc-filter-chain-roundtrip.php` exits 0 covering all 5 REQs; the existing `examples/poc-replace-text.php` still exits 0 (single-FlateDecode string-form path unchanged). +See `openspec/changes/feat-filter-chain-dispatch/design.md` (the canonical artefact) for the shipped-implementation note, decisions D1-D6, and the method-name listing. This file intentionally keeps the original proposal/design/tasks content for upstream submission to dealfonso/sapp; the implementation specifics are tracked in the OpenSpec change to avoid duplicate-source drift across four files. diff --git a/docs/upstream-prs/05-filter-chaining/tasks.md b/docs/upstream-prs/05-filter-chaining/tasks.md index 7b446c2..d36c24c 100644 --- a/docs/upstream-prs/05-filter-chaining/tasks.md +++ b/docs/upstream-prs/05-filter-chaining/tasks.md @@ -45,18 +45,8 @@ --- -## Implementation note (shipped 2026-05-27 on `feat/filter-chain-dispatch`) - -The dispatcher landed on the ConductionNL fork's `work/text-replacement` branch via `feat/filter-chain-dispatch`. Canonical contract lives in `openspec/changes/feat-filter-chain-dispatch/` (proposal, design with decisions D1–D6, spec with REQ-1 through REQ-5, tasks). Method names as shipped (all `protected static` on `PDFObject`): - -- `normalise_filter_chain($filterValue): string[]` — coerces `/Filter` (string-form, `PDFValueList` array-form, missing, empty) to a flat array of filter-name strings. -- `normalise_decode_parms_chain($parmsValue, int $chainLength): array` — coerces `/DecodeParms` (single dict, `PDFValueList` of dicts/null, missing) to a fixed-length positional array; trailing missing entries → `null`. -- `build_flate_params($parmsForThisFilter): array` — papers over the optional `/DecodeParms` entries with PDF 1.7 default values (Columns=0, Predictor=1, BitsPerComponent=8, Colors=1) for the existing `FlateDecode` static helper. -- `apply_filter_chain_decode($bytes, array $filters, array $params): string|false` — applies filters in FORWARD order (REQ-1). Unknown filter name → `p_error()` + return `false` (REQ-5). -- `apply_filter_chain_encode($bytes, array $filters, array $params): string|false` — applies filters in REVERSE order (REQ-2). Same failure semantics. - -`PDFObject::get_stream` / `set_stream` now normalise + delegate to the chain dispatcher when `$raw === false`; they preserve the original `/Filter` shape on write-back (D2) and leave `_stream` + `Length` unchanged on chain failure (D4). +--- -PR `#01`–`#04` (ASCIIHex / RunLength / ASCII85 / LZW) attach to this dispatcher by adding `case` arms inside `apply_filter_chain_decode` / `apply_filter_chain_encode` — no further structural changes needed. +## Implementation note -Verification: `examples/poc-filter-chain-roundtrip.php` exits 0 covering all 5 REQs; the existing `examples/poc-replace-text.php` still exits 0 (single-FlateDecode string-form path unchanged). +See `openspec/changes/feat-filter-chain-dispatch/design.md` (the canonical artefact) for the shipped-implementation note, decisions D1-D6, and the method-name listing. This file intentionally keeps the original proposal/design/tasks content for upstream submission to dealfonso/sapp; the implementation specifics are tracked in the OpenSpec change to avoid duplicate-source drift across four files. diff --git a/docs/upstream-prs/README.md b/docs/upstream-prs/README.md index 96ee4ce..103c9a0 100644 --- a/docs/upstream-prs/README.md +++ b/docs/upstream-prs/README.md @@ -20,6 +20,8 @@ The work is **planned for upstream contribution**, not as a permanent fork. The The work is split into eight focused PRs, in dependency order. Each is small enough to review individually. Bigger PRs sit on top of merged smaller PRs so they can land incrementally without one big-bang review. +**Note on numbering schemes**: the `01..08` prefix on directory names below is the planned-submission order to upstream `dealfonso/sapp`. **It is intentionally NOT the same as the fork's GitHub PR numbers** — the foundation (`05-filter-chaining`) lands first in our fork so the four dependent filter codecs (`01..04`) can attach to it locally, but it's submitted upstream AFTER they merge there. The fork's PR sequence in `ConductionNL/sapp` is: PR #1 (PoC), PR #2 (OpenSpec scaffolding), PR #3 (filter-chain-dispatch = upstream #05), PRs #4-#7 (the four filter codecs = upstream #01-#04), PR #8 (CMap = upstream #06), PR #9 (TJ flattening = upstream #07), PR #10 (text-replacement API = upstream #08). Cross-references throughout these draft files use the `upstream-PR #NN` form to disambiguate. + Each feature has its own directory containing: - `issue.md` — outward-facing draft to post on `dealfonso/sapp`. Frontmatter records `Posted at:` once the issue is live. diff --git a/examples/poc-filter-chain-roundtrip.php b/examples/poc-filter-chain-roundtrip.php index c5520c6..bfb63a7 100644 --- a/examples/poc-filter-chain-roundtrip.php +++ b/examples/poc-filter-chain-roundtrip.php @@ -31,6 +31,7 @@ use ddn\sapp\pdfvalue\PDFValueObject; use ddn\sapp\pdfvalue\PDFValueList; use ddn\sapp\pdfvalue\PDFValueType; +use ddn\sapp\pdfvalue\PDFValueSimple; $failures = []; @@ -98,6 +99,41 @@ function buildObj($filterValue, string $rawStream = ''): PDFObject { } } +/* ---------------------------------------------------------------- * + * REQ-3 — /DecodeParms positional routing + * + * Single-filter chain with explicit /DecodeParms. The dispatcher must + * walk the parallel parms array and hand the per-filter dict to the + * decoder. With Predictor=1 the PNG predictor is a no-op so the + * round-trip is byte-for-byte clean even though /DecodeParms is + * present — this is the smoke-test path. A trailing-null entry on + * /DecodeParms must be tolerated; an under-length array must be + * tolerated. + * ---------------------------------------------------------------- */ +{ + $plaintext = "REQ-3 smoke test — Predictor=1 + explicit /Columns 4"; + + $filterList = new PDFValueList(); + $filterList->push(new PDFValueType('FlateDecode')); + + // /DecodeParms [<>] — single-element + // parallel array with explicit no-op predictor. + $parmsDict = new PDFValueObject(); + $parmsDict['Predictor'] = new PDFValueSimple(1); + $parmsDict['Columns'] = new PDFValueSimple(4); + $parmsList = new PDFValueList(); + $parmsList->push($parmsDict); + + $obj = buildObj($filterList); + $obj->get_value()['DecodeParms'] = $parmsList; + $obj->set_stream($plaintext, false); + + $decoded = $obj->get_stream(false); + if ($decoded !== $plaintext) { + $failures[] = "REQ-3 (/DecodeParms positional smoke test): round-trip mismatch"; + } +} + /* ---------------------------------------------------------------- * * REQ-1 (Empty array /Filter) — MUST be treated as no filtering * ---------------------------------------------------------------- */ @@ -131,7 +167,11 @@ function buildObj($filterValue, string $rawStream = ''): PDFObject { $originalCompressed = (string) gzcompress("doesn't matter — won't decode through the unknown filter"); $unknownChain = new PDFValueList(); - $unknownChain->push(new PDFValueType('ASCIIHexDecode')); // ships in PR #01 — currently unknown + // Use a deliberately-fake filter name so this test stays valid as + // PRs #01-#04 land real implementations of ASCIIHex / ASCII85 / + // RunLength / LZW. The dispatcher's fail-safe path is what's + // under test, not any particular filter's availability. + $unknownChain->push(new PDFValueType('SappTestUnknownFilter')); $unknownChain->push(new PDFValueType('FlateDecode')); $obj = buildObj($unknownChain, $originalCompressed); @@ -142,9 +182,13 @@ function buildObj($filterValue, string $rawStream = ''): PDFObject { $decoded = $obj->get_stream(false); ob_end_clean(); - // REQ-5: get_stream returns the raw stream when a chain filter is unknown. - if ($decoded !== $originalCompressed) { - $failures[] = "REQ-5: get_stream did not return raw _stream on unknown filter (got " . strlen((string) $decoded) . " bytes, expected " . strlen($originalCompressed) . ")"; + // REQ-5: get_stream returns `false` on unknown chain filter, matching + // upstream's pre-refactor `return p_error(...)` semantics. Callers + // using the `if ($decoded === false) continue;` idiom (e.g. + // PDFDoc::replace_text_in_document's not-is_string guard) get the + // skip behaviour they expect. + if ($decoded !== false) { + $failures[] = "REQ-5: get_stream did not return false on unknown filter (got " . var_export($decoded, true) . ")"; } // REQ-5: set_stream leaves _stream + Length untouched on unknown filter. diff --git a/openspec/changes/feat-filter-chain-dispatch/design.md b/openspec/changes/feat-filter-chain-dispatch/design.md index 70a0d10..df15b2c 100644 --- a/openspec/changes/feat-filter-chain-dispatch/design.md +++ b/openspec/changes/feat-filter-chain-dispatch/design.md @@ -40,9 +40,9 @@ The PoC verify (`examples/poc-replace-text.php`) currently passes on a single-Fl ## Decisions -### D1 — Inline private dispatch helpers, not new public methods +### D1 — Inline protected-static dispatch helpers, not new public methods -We add two `private` helpers — `apply_filter_chain_decode(string $bytes, array $filters, array $decodeParms): string|false` and `apply_filter_chain_encode(string $bytes, array $filters, array $decodeParms): string|false` — both inside `PDFObject`. The public `get_stream` / `set_stream` shape stays identical; they normalise the `/Filter` value (string → 1-element array) and the parallel `/DecodeParms` (single value → 1-element array), then delegate. +We add two `protected static` helpers — `apply_filter_chain_decode($bytes, array $filters, array $params)` and `apply_filter_chain_encode($bytes, array $filters, array $params)` — both inside `PDFObject`. (`protected static` rather than `private` so test-suite subclasses can stub the helpers — the chain-ordering verification gate uses this — and matches the existing `FlateDecode` static helper's visibility.) The public `get_stream` / `set_stream` shape stays identical; they normalise the `/Filter` value (string → 1-element array) and the parallel `/DecodeParms` (single value → 1-element array), then delegate. **Alternative considered:** add a public `apply_filter()` method to `PDFObject`. **Rejected** because it would expose internal-only mechanics, widen the API surface, and create a separate code path that callers might bypass to inject malformed input. The two existing public methods already encapsulate the right level of abstraction. @@ -72,7 +72,15 @@ No new files, no signature changes on dependent classes (`PDFDoc`, `PDFUtilFnc`) ### D6 — Tests live as round-trip fixture assertions in `examples/` -The existing `examples/poc-replace-text.php` validation gate proves the single-FlateDecode path. We add a sibling `examples/poc-filter-chain-roundtrip.php` (introduced in this change) that builds a synthetic two-filter PDF (`[/ASCIIHexDecode /FlateDecode]`) using only FlateDecode-implemented machinery plus hand-crafted ASCIIHex envelope bytes (since ASCIIHexDecode itself doesn't ship until PR `#01`). The fixture asserts that the dispatcher correctly applies the chain even when only one filter is implemented — the ASCII-hex envelope gets handed off to a stub case that returns `false` and the test asserts that path is taken. +The existing `examples/poc-replace-text.php` validation gate proves the single-FlateDecode path. We add a sibling `examples/poc-filter-chain-roundtrip.php` (introduced in this change) that exercises four scenarios at the dispatcher contract level: + +1. A single-element `/Filter [/FlateDecode]` array-form round-trip (proves the array shape routes through the chain dispatcher, semantically equivalent to the string form). +2. An empty `/Filter []` pass-through (REQ-1's empty-chain scenario). +3. A string-form `/Filter /FlateDecode` round-trip + shape-preservation check (D2; REQ-4). +4. An unknown filter `/Filter [/SappTestUnknownFilter /FlateDecode]` chain-failure assertion: `get_stream(false)` returns `false`, `set_stream($_, false)` leaves `_stream` + `Length` untouched (REQ-5). +5. A `/DecodeParms` positional smoke test: single `/Filter [/FlateDecode]` + `/DecodeParms [<>]` round-trip (REQ-3 smoke; Predictor=1 keeps it lossless). + +A genuine two-filter `[/ /]` round-trip — one that would prove decode/encode ORDERING is correct in the dispatcher — requires a second implemented filter and is therefore deferred to upstream-PRs #01-#04. Each of those PRs' verification gates extends `poc-filter-chain-roundtrip.php` with a two-filter test pairing the new codec with `/FlateDecode`. The chain-ordering scenario in `spec.md` REQ-1 / REQ-2 is locked at the spec layer but un-exercised by this PR's gate alone; the four follow-up PRs are jointly responsible for proving the ordering by induction. **Alternative considered:** defer chain-aware tests until PR `#01` (when there's a real second filter to chain). **Rejected** — the dispatch behaviour is the entire contract of this PR; testing it via the stub path locks the contract in before any real filter depends on it. diff --git a/openspec/changes/feat-filter-chain-dispatch/specs/filter-chain-dispatch/spec.md b/openspec/changes/feat-filter-chain-dispatch/specs/filter-chain-dispatch/spec.md index fc30e43..3991267 100644 --- a/openspec/changes/feat-filter-chain-dispatch/specs/filter-chain-dispatch/spec.md +++ b/openspec/changes/feat-filter-chain-dispatch/specs/filter-chain-dispatch/spec.md @@ -28,10 +28,12 @@ When `PDFObject::set_stream($plaintext, $raw = false)` is called on an object wh - **WHEN** `set_stream("BT...ET", false)` is called on an object with `/Filter [/ASCIIHexDecode /FlateDecode]` - **THEN** the resulting `_stream` MUST equal `ascii_hex_envelope(gzcompress("BT...ET"))` and `_value['Length']` MUST equal `strlen($_stream)` after encoding -#### Scenario: Encode-decode round-trip is lossless +#### Scenario: Encode-decode round-trip is lossless (Predictor absent or = 1) -- **WHEN** `set_stream($P, false)` is called on an array-form-`/Filter` object, then `get_stream(false)` is called on the same object -- **THEN** the value returned by `get_stream(false)` MUST equal `$P` byte-for-byte +- GIVEN an array-form-`/Filter` object with `/DecodeParms` either absent OR carrying `Predictor` value 1 for every chain position (i.e. the no-op-predictor case) +- WHEN `set_stream($P, false)` is invoked on the object, then `get_stream(false)` is invoked on the same object +- THEN the value returned by `get_stream(false)` MUST equal `$P` byte-for-byte +- AND for chains that include a non-trivial predictor (`Predictor ≥ 10`), round-trip losslessness is **out of scope** of this change — the encode path applies `gzcompress` only and does not run the PNG filter encoder. A follow-up change (`feat-flate-predictor-encode`) ships the symmetric encode-side predictor application; until then, callers MUST NOT rely on byte-for-byte round-trip when a PNG predictor is in play. ### Requirement: `/DecodeParms` array SHALL be applied positionally per filter @@ -78,8 +80,9 @@ If any filter name in `/Filter` is not implemented, the dispatcher MUST log via - **AND** `$this->_value['Length']` MUST remain unchanged - **AND** the method MUST NOT throw -#### Scenario: Unknown filter on get_stream returns the raw stream +#### Scenario: Unknown filter on get_stream returns false -- **WHEN** `get_stream(false)` is called on an object with `/Filter [/UnknownFilter /FlateDecode]` -- **THEN** `p_error()` MUST be called identifying the unknown filter -- **AND** the return value MUST equal `$this->_stream` (the raw, undecoded bytes) \ No newline at end of file +- GIVEN an object with `/Filter [/UnknownFilter /FlateDecode]` +- WHEN `get_stream(false)` is invoked +- THEN `p_error()` MUST be called identifying the unknown filter +- AND the return value MUST be `false` (matching upstream's pre-refactor `return p_error(...)` semantics — `p_error` defaults to returning `false`) diff --git a/src/PDFObject.php b/src/PDFObject.php index 42e42c7..c2f9b62 100644 --- a/src/PDFObject.php +++ b/src/PDFObject.php @@ -299,8 +299,14 @@ protected static function normalise_decode_parms_chain($parmsValue, $chainLength */ protected static function build_flate_params($parmsForThisFilter) { $parms = ($parmsForThisFilter === null) ? [] : $parmsForThisFilter; + // PDF 1.7 §7.4.4.3 Table 8 defaults — `Columns=1`, NOT 0. The + // wrong default is hidden when Predictor=1 (the unfilter loop + // short-circuits before consulting Columns), but the moment a + // PNG-predicted stream lands without an explicit /Columns + // entry, `Columns=0` would drive the row-stride loop into an + // empty/infinite buffer. return [ - "Columns" => $parms['Columns'] ?? new PDFValueSimple(0), + "Columns" => $parms['Columns'] ?? new PDFValueSimple(1), "Predictor" => $parms['Predictor'] ?? new PDFValueSimple(1), "BitsPerComponent" => $parms['BitsPerComponent'] ?? new PDFValueSimple(8), "Colors" => $parms['Colors'] ?? new PDFValueSimple(1), @@ -407,11 +413,14 @@ public function get_stream($raw = true) { $decoded = self::apply_filter_chain_decode($this->_stream, $filters, $params); if ($decoded === false) { - // Chain failed (unknown filter or codec error). Mirror the - // pre-refactor behaviour: the failing arm already emitted - // `p_error`; we return the raw stream so callers can detect - // the mismatch without crashing. - return $this->_stream; + // Chain failed (unknown filter or codec error). The + // pre-refactor behaviour was `return p_error(...)`, and + // `p_error` defaults its `$retval` to `false` — so legacy + // callers using the `if ($decoded === false) continue;` + // idiom (e.g. `PDFDoc::replace_text_in_document`) get the + // same skip semantics they did before the dispatcher + // refactor. The failing arm already emitted `p_error`. + return false; } return $decoded;