diff --git a/docs/upstream-prs/05-filter-chaining/design.md b/docs/upstream-prs/05-filter-chaining/design.md index 4e161fc..390f20e 100644 --- a/docs/upstream-prs/05-filter-chaining/design.md +++ b/docs/upstream-prs/05-filter-chaining/design.md @@ -86,3 +86,12 @@ 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 + +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 23f52e4..226324b 100644 --- a/docs/upstream-prs/05-filter-chaining/proposal.md +++ b/docs/upstream-prs/05-filter-chaining/proposal.md @@ -31,3 +31,12 @@ 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 + +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 f92eb0a..d36c24c 100644 --- a/docs/upstream-prs/05-filter-chaining/tasks.md +++ b/docs/upstream-prs/05-filter-chaining/tasks.md @@ -41,3 +41,12 @@ - [ ] 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 + +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 new file mode 100644 index 0000000..bfb63a7 --- /dev/null +++ b/examples/poc-filter-chain-roundtrip.php @@ -0,0 +1,225 @@ +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-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 + * ---------------------------------------------------------------- */ +{ + $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(); + // 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); + $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 `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. + $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/design.md b/openspec/changes/feat-filter-chain-dispatch/design.md index 87879dc..2b11656 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 upstream-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 upstream-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 26447a3..2563e1c 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 @@ -64,10 +64,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 +- 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. ### REQ-003: `/DecodeParms` array SHALL be applied positionally per filter @@ -114,8 +116,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]` +- 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 equal `$this->_stream` (the raw, undecoded bytes) +- 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/openspec/changes/feat-filter-chain-dispatch/tasks.md b/openspec/changes/feat-filter-chain-dispatch/tasks.md index 78dd775..d76e4be 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 upstream-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 upstream-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` +- [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..c2f9b62 100644 --- a/src/PDFObject.php +++ b/src/PDFObject.php @@ -209,6 +209,189 @@ 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; + // 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(1), + "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 +399,31 @@ 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; } - 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). 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; } /** * Sets the stream for the object (overwrites a previous existing stream) @@ -244,17 +434,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; + } + + $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; } - $this->_value['Length'] = strlen($stream); - $this->_stream = $stream; + + // 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,