feat(filter-chain): array-form /Filter dispatch in PDFObject (PR #05 — foundation)

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.

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.

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.

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.

examples/poc-filter-chain-roundtrip.php

@@ -0,0 +1,225 @@
+<?php
+/**
+ * Round-trip verification gate for the filter chain dispatcher
+ * (`feat-filter-chain-dispatch` / docs/upstream-prs/05-filter-chaining/).
+ *
+ * This script asserts the dispatcher contract from the OpenSpec change's
+ * spec.md:
+ *
+ *   REQ-1 — Array-form /Filter SHALL decode in forward chain order
+ *   REQ-2 — Array-form /Filter SHALL encode in reverse chain order
+ *   REQ-3 — /DecodeParms array SHALL be applied positionally per filter
+ *   REQ-4 — String-form /Filter callers MUST observe unchanged behaviour
+ *   REQ-5 — Unknown filter names in a chain MUST fail safely
+ *
+ * Because the four follow-up filter PRs (#01-#04) haven't shipped yet,
+ * the chain-decode positive path is exercised with a single-element
+ * filter array `[/FlateDecode]` — semantically equivalent to the
+ * string form but routed through the chain dispatcher. The unknown-
+ * filter scenario verifies REQ-5 directly.
+ *
+ * Exit code 0 on success; 1 on any assertion failure.
+ *
+ * SPDX-License-Identifier: LGPL-3.0-or-later
+ */
+
+declare(strict_types=1);
+
+require_once __DIR__ . '/../vendor/autoload.php';
+
+use ddn\sapp\PDFObject;
+use ddn\sapp\pdfvalue\PDFValueObject;
+use ddn\sapp\pdfvalue\PDFValueList;
+use ddn\sapp\pdfvalue\PDFValueType;
+use ddn\sapp\pdfvalue\PDFValueSimple;
+
+$failures = [];
+
+/**
+ * Build a PDFObject with the given /Filter shape and stream bytes.
+ *
+ * The stream is stored raw (no encoding) so the test owns the byte
+ * layout entirely. Callers that want compression use `set_stream($_,
+ * false)` on the returned object to round-trip through the dispatcher.
+ */
+function buildObj($filterValue, string $rawStream = ''): PDFObject {
+    $value = new PDFValueObject();
+    if ($filterValue !== null) {
+        $value['Filter'] = $filterValue;
+    }
+    $obj = new PDFObject(1, $value);
+    $obj->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 [<</Predictor 1 /Columns 4>>] — 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);

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 [<</Predictor 1 /Columns 4>>]` round-trip (REQ-3 smoke; Predictor=1 keeps it lossless).
+
+A genuine two-filter `[/<filter-a> /<filter-b>]` 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.
 

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`)