From 23a96678d847aec469a152956c18fba04c9813d4 Mon Sep 17 00:00:00 2001 From: Robert Zondervan Date: Wed, 27 May 2026 11:54:27 +0200 Subject: [PATCH 1/2] feat(asciihex): ASCIIHexDecode encode + decode + chain wiring (SAPP PR #01) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements PDF 1.7 §7.4.2 ASCIIHexDecode on top of the chain dispatcher from PR #05. Both encode and decode go through the existing chain plumbing — adding ASCIIHexDecode is a strict `case` extension to apply_filter_chain_decode / apply_filter_chain_encode, no structural change to the dispatcher itself. New helpers on PDFObject (both `protected static`, PascalCase per upstream's filter-name convention): - ASCIIHexDecode($_stream, $params): string|false Strips PDF whitespace (§7.5.1), finds EOD `>`, discards anything after, pads odd-length input with `0` per §7.4.2 ¶3, accepts both cases of hex digits. Illegal characters → p_error + return raw input (D2 fail-safe, matches upstream convention). - ASCIIHexEncode($_stream, $params): string strtoupper(bin2hex($_stream)) + chunk_split at 80 columns + EOD `>`. Empty input emits just `>`. Dispatcher `case 'ASCIIHexDecode':` arms added to both encode + decode chain helpers. The dispatcher's existing round-trip test (from PR #05) updated to use `/SappTestUnknownFilter` as the unknown-filter sentinel so it stays valid as subsequent filter PRs (#02-#04) land their own case arms. Contract pinned by openspec/changes/feat-asciihex-decode/ (proposal + design D1-D5 + spec REQ-1 through REQ-5 with MODIFIED filter-chain-dispatch capability + tasks). Verification: - examples/poc-filter-roundtrip-asciihex.php (new) — 5 REQs covered: decode scenarios (even-length, odd-length-pad, whitespace, lowercase, trailing-after-EOD), encode scenarios (basic, empty input, 80-col wrap), lossless round-trip on 1024 random bytes, illegal-char fail-safe, chain integration (single-filter and [/ASCIIHexDecode /FlateDecode] two-filter outer-envelope). - examples/poc-replace-text.php — baseline still green. - examples/poc-filter-chain-roundtrip.php — dispatcher tests pass after the unknown-filter sentinel update. Implementation note for the eventual upstream PR (preserved in docs/upstream-prs/01-asciihex-decode/): hex2bin + chunk_split-based implementation keeps the code path under 40 LOC and dependency-free. PHP 7.4 minimum preserved. Closes openspec/changes/feat-asciihex-decode/tasks.md 22/22. --- .../upstream-prs/01-asciihex-decode/design.md | 14 ++ .../01-asciihex-decode/proposal.md | 14 ++ docs/upstream-prs/01-asciihex-decode/tasks.md | 14 ++ examples/poc-filter-chain-roundtrip.php | 6 +- examples/poc-filter-roundtrip-asciihex.php | 201 ++++++++++++++++++ .../changes/feat-asciihex-decode/tasks.md | 44 ++-- src/PDFObject.php | 78 +++++++ 7 files changed, 348 insertions(+), 23 deletions(-) create mode 100644 examples/poc-filter-roundtrip-asciihex.php diff --git a/docs/upstream-prs/01-asciihex-decode/design.md b/docs/upstream-prs/01-asciihex-decode/design.md index d1e8a7c..723c58b 100644 --- a/docs/upstream-prs/01-asciihex-decode/design.md +++ b/docs/upstream-prs/01-asciihex-decode/design.md @@ -38,3 +38,17 @@ A nicer design would be a `FilterRegistry` with one class per filter. We don't i | Maintainer rejects the spot-add approach, prefers a registry abstraction up-front | Low | Discuss in the issue (the "ask" section) before opening the PR | | Edge-case input not caught by `@hex2bin` (e.g. valid hex but unexpected length pattern) | Low | Unit test odd-length, whitespace-rich, and invalid-character inputs | | Round-trip diff vs. original byte layout (whitespace stripped, EOD position shifted) | Acceptable | PDF readers parse the same logical content; byte-identical round-trip is not promised by the spec | + + +--- + +## Implementation note (shipped 2026-05-27 on `feat/asciihex-decode`) + +Implementation landed on `feat/asciihex-decode` stacked on `feat/filter-chain-dispatch` (#05). Canonical contract lives in `openspec/changes/feat-asciihex-decode/`. Method names as shipped (all `protected static` on `PDFObject`): + +- `ASCIIHexDecode($_stream, $params): string|false` — strips whitespace via PDF 1.7 §7.5.1 character class, finds EOD `>` and discards anything after, pads odd trailing nibble with `0` (§7.4.2 ¶3), illegal chars → `p_error()` + return raw input. +- `ASCIIHexEncode($_stream, $params): string` — `strtoupper(bin2hex(...))` + `chunk_split(..., 80, "\n")` + EOD `>`, empty input emits just `>`. + +Both are wired into `apply_filter_chain_decode` / `apply_filter_chain_encode` via the `case 'ASCIIHexDecode':` arm. + +Verification: `examples/poc-filter-roundtrip-asciihex.php` covers all 5 REQs (basic + odd-length + whitespace + lowercase + trailing-after-EOD + lossless round-trip on 1024 random bytes + illegal-char fail-safe + chain integration single-filter and ASCIIHex+Flate two-filter). Companion dispatcher test on `feat/filter-chain-dispatch` updated to use `/SappTestUnknownFilter` so it stays valid as subsequent filter PRs land real implementations. diff --git a/docs/upstream-prs/01-asciihex-decode/proposal.md b/docs/upstream-prs/01-asciihex-decode/proposal.md index 851db23..1d32b87 100644 --- a/docs/upstream-prs/01-asciihex-decode/proposal.md +++ b/docs/upstream-prs/01-asciihex-decode/proposal.md @@ -22,3 +22,17 @@ This feature is the first in a series adding decoder coverage for the text-relev - **Downstream consumers:** PDF objects using `/Filter /ASCIIHexDecode` become readable / writable. Nothing previously working regresses. - **Spec target:** PDF 1.7 §7.4.2 (ASCIIHexDecode Filter). - **Out of scope:** filter chaining (`/Filter [/X /Y]` array form, separate PR), other filters in the series (separate PRs). + + +--- + +## Implementation note (shipped 2026-05-27 on `feat/asciihex-decode`) + +Implementation landed on `feat/asciihex-decode` stacked on `feat/filter-chain-dispatch` (#05). Canonical contract lives in `openspec/changes/feat-asciihex-decode/`. Method names as shipped (all `protected static` on `PDFObject`): + +- `ASCIIHexDecode($_stream, $params): string|false` — strips whitespace via PDF 1.7 §7.5.1 character class, finds EOD `>` and discards anything after, pads odd trailing nibble with `0` (§7.4.2 ¶3), illegal chars → `p_error()` + return raw input. +- `ASCIIHexEncode($_stream, $params): string` — `strtoupper(bin2hex(...))` + `chunk_split(..., 80, "\n")` + EOD `>`, empty input emits just `>`. + +Both are wired into `apply_filter_chain_decode` / `apply_filter_chain_encode` via the `case 'ASCIIHexDecode':` arm. + +Verification: `examples/poc-filter-roundtrip-asciihex.php` covers all 5 REQs (basic + odd-length + whitespace + lowercase + trailing-after-EOD + lossless round-trip on 1024 random bytes + illegal-char fail-safe + chain integration single-filter and ASCIIHex+Flate two-filter). Companion dispatcher test on `feat/filter-chain-dispatch` updated to use `/SappTestUnknownFilter` so it stays valid as subsequent filter PRs land real implementations. diff --git a/docs/upstream-prs/01-asciihex-decode/tasks.md b/docs/upstream-prs/01-asciihex-decode/tasks.md index 1a80fac..15c6a46 100644 --- a/docs/upstream-prs/01-asciihex-decode/tasks.md +++ b/docs/upstream-prs/01-asciihex-decode/tasks.md @@ -29,3 +29,17 @@ - [ ] 5.1 No regressions in the existing FlateDecode path — verify against `examples/testdoc.pdf` (the existing example PDF, which is FlateDecoded). - [ ] 5.2 No new dependencies — pure PHP only, no Composer additions. - [ ] 5.3 Spec-validity: every Requirement in `spec.md` has a passing verification step. + + +--- + +## Implementation note (shipped 2026-05-27 on `feat/asciihex-decode`) + +Implementation landed on `feat/asciihex-decode` stacked on `feat/filter-chain-dispatch` (#05). Canonical contract lives in `openspec/changes/feat-asciihex-decode/`. Method names as shipped (all `protected static` on `PDFObject`): + +- `ASCIIHexDecode($_stream, $params): string|false` — strips whitespace via PDF 1.7 §7.5.1 character class, finds EOD `>` and discards anything after, pads odd trailing nibble with `0` (§7.4.2 ¶3), illegal chars → `p_error()` + return raw input. +- `ASCIIHexEncode($_stream, $params): string` — `strtoupper(bin2hex(...))` + `chunk_split(..., 80, "\n")` + EOD `>`, empty input emits just `>`. + +Both are wired into `apply_filter_chain_decode` / `apply_filter_chain_encode` via the `case 'ASCIIHexDecode':` arm. + +Verification: `examples/poc-filter-roundtrip-asciihex.php` covers all 5 REQs (basic + odd-length + whitespace + lowercase + trailing-after-EOD + lossless round-trip on 1024 random bytes + illegal-char fail-safe + chain integration single-filter and ASCIIHex+Flate two-filter). Companion dispatcher test on `feat/filter-chain-dispatch` updated to use `/SappTestUnknownFilter` so it stays valid as subsequent filter PRs land real implementations. diff --git a/examples/poc-filter-chain-roundtrip.php b/examples/poc-filter-chain-roundtrip.php index c5520c6..0f47dd7 100644 --- a/examples/poc-filter-chain-roundtrip.php +++ b/examples/poc-filter-chain-roundtrip.php @@ -131,7 +131,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); diff --git a/examples/poc-filter-roundtrip-asciihex.php b/examples/poc-filter-roundtrip-asciihex.php new file mode 100644 index 0000000..de27ac5 --- /dev/null +++ b/examples/poc-filter-roundtrip-asciihex.php @@ -0,0 +1,201 @@ +getMethod($method); + $m->setAccessible(true); + return $m->invokeArgs(null, $args); +} + +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-1 — decode scenarios + * ---------------------------------------------------------------- */ +{ + $decoded = invokeProtectedStatic('ASCIIHexDecode', ['48656C6C6F>', null]); + if ($decoded !== 'Hello') { + $failures[] = "REQ-1 (even-length): expected 'Hello', got " . bin2hex((string) $decoded); + } + + // Odd-length: 41B> → \x41\xB0 (trailing B paired with implicit 0) + $decoded = invokeProtectedStatic('ASCIIHexDecode', ['41B>', null]); + if ($decoded !== "\x41\xB0") { + $failures[] = "REQ-1 (odd-length pad): expected 0x41 0xB0, got " . bin2hex((string) $decoded); + } + + // Whitespace anywhere + $decoded = invokeProtectedStatic('ASCIIHexDecode', ["48 65\n6C\t6C\r\n6F>", null]); + if ($decoded !== 'Hello') { + $failures[] = "REQ-1 (whitespace): expected 'Hello', got " . bin2hex((string) $decoded); + } + + // Lowercase hex + $decoded = invokeProtectedStatic('ASCIIHexDecode', ['48656c6c6f>', null]); + if ($decoded !== 'Hello') { + $failures[] = "REQ-1 (lowercase): expected 'Hello', got " . bin2hex((string) $decoded); + } + + // Trailing bytes after EOD ignored + $decoded = invokeProtectedStatic('ASCIIHexDecode', ['48656C6C6F>garbage', null]); + if ($decoded !== 'Hello') { + $failures[] = "REQ-1 (trailing after EOD): expected 'Hello', got " . bin2hex((string) $decoded); + } +} + +/* ---------------------------------------------------------------- * + * REQ-2 — encode scenarios + * ---------------------------------------------------------------- */ +{ + // Basic encode + $encoded = invokeProtectedStatic('ASCIIHexEncode', ['Hello', null]); + if ($encoded !== '48656C6C6F>') { + $failures[] = "REQ-2 (basic): expected '48656C6C6F>', got '$encoded'"; + } + + // Empty input → just EOD + $encoded = invokeProtectedStatic('ASCIIHexEncode', ['', null]); + if ($encoded !== '>') { + $failures[] = "REQ-2 (empty): expected '>', got '$encoded'"; + } + + // Line wrap at 80 columns — input of 50 bytes encodes to 100 hex chars, + // must produce at least one '\n' and no line > 80 chars + $longInput = str_repeat('A', 50); + $encoded = invokeProtectedStatic('ASCIIHexEncode', [$longInput, null]); + if (strpos($encoded, "\n") === false) { + $failures[] = "REQ-2 (line wrap): 50-byte input did not produce any newlines"; + } + foreach (explode("\n", $encoded) as $line) { + if (strlen($line) > 80) { + $failures[] = "REQ-2 (line wrap): line longer than 80 chars (" . strlen($line) . ")"; + break; + } + } +} + +/* ---------------------------------------------------------------- * + * REQ-3 — lossless round-trip on 1024-byte random binary + * ---------------------------------------------------------------- */ +{ + $original = random_bytes(1024); + $encoded = invokeProtectedStatic('ASCIIHexEncode', [$original, null]); + $decoded = invokeProtectedStatic('ASCIIHexDecode', [$encoded, null]); + if ($decoded !== $original) { + $failures[] = "REQ-3 (binary round-trip): 1024-byte input did not survive encode→decode (got " . strlen((string) $decoded) . " bytes back)"; + } +} + +/* ---------------------------------------------------------------- * + * REQ-4 — illegal character fail-safe + * ---------------------------------------------------------------- */ +{ + $original = '48656C!6C6F>'; // contains '!' + ob_start(); + $decoded = invokeProtectedStatic('ASCIIHexDecode', [$original, null]); + ob_end_clean(); + if ($decoded !== $original) { + $failures[] = "REQ-4 (illegal char): expected raw input passthrough on failure, got " . bin2hex((string) $decoded); + } +} + +/* ---------------------------------------------------------------- * + * REQ-5 — chain dispatcher recognises /ASCIIHexDecode + * ---------------------------------------------------------------- */ +{ + // Single-filter chain via array form: [/ASCIIHexDecode] + $filterList = new PDFValueList(); + $filterList->push(new PDFValueType('ASCIIHexDecode')); + + $obj = buildObj($filterList); + $plaintext = 'plain-text payload — ASCIIHex only'; + $obj->set_stream($plaintext, false); + + $encoded = $obj->get_stream(true); + if ($encoded === $plaintext) { + $failures[] = "REQ-5 (chain-only-ASCIIHex set_stream): did not encode (stored verbatim)"; + } + if (substr($encoded, -1) !== '>') { + $failures[] = "REQ-5 (chain-only-ASCIIHex set_stream): encoded stream missing trailing EOD '>'"; + } + + $decoded = $obj->get_stream(false); + if ($decoded !== $plaintext) { + $failures[] = "REQ-5 (chain-only-ASCIIHex round-trip): expected plaintext, got " . bin2hex((string) $decoded); + } +} + +{ + // Two-filter chain: [/ASCIIHexDecode /FlateDecode] + // Encode order is REVERSE: FlateDecode first (innermost), then + // ASCIIHexDecode (outermost). Decode order is FORWARD: ASCIIHex + // first to strip the hex envelope, then Flate to decompress. + $filterList = new PDFValueList(); + $filterList->push(new PDFValueType('ASCIIHexDecode')); + $filterList->push(new PDFValueType('FlateDecode')); + + $obj = buildObj($filterList); + $plaintext = "BT (Chained ASCIIHex outer + Flate inner) Tj ET"; + $obj->set_stream($plaintext, false); + + $encoded = $obj->get_stream(true); + if (substr($encoded, -1) !== '>') { + $failures[] = "REQ-5 (chain ASCIIHex+Flate set_stream): encoded stream missing trailing '>' (outer ASCIIHex)"; + } + + $decoded = $obj->get_stream(false); + if ($decoded !== $plaintext) { + $failures[] = "REQ-5 (chain ASCIIHex+Flate round-trip): expected plaintext, got " . bin2hex((string) $decoded); + } +} + +/* ---------------------------------------------------------------- * + * Report + * ---------------------------------------------------------------- */ +echo "PoC ASCIIHexDecode — round-trip verification\n"; +echo " spec: openspec/changes/feat-asciihex-decode/specs/asciihex-decode-filter/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-asciihex-decode/tasks.md b/openspec/changes/feat-asciihex-decode/tasks.md index 065a035..53d2b98 100644 --- a/openspec/changes/feat-asciihex-decode/tasks.md +++ b/openspec/changes/feat-asciihex-decode/tasks.md @@ -1,45 +1,45 @@ ## 1. Pre-flight -- [ ] 1.1 Confirm `feat-filter-chain-dispatch` (PR `#05`) is merged into `work/text-replacement` before starting — it's a hard dependency -- [ ] 1.2 Branch off `work/text-replacement` as `feat/asciihex-decode` -- [ ] 1.3 Re-read PDF 1.7 §7.4.2 and confirm decoder rules (whitespace + odd-length + EOD + alphabet) +- [x] 1.1 Confirm `feat-filter-chain-dispatch` (PR `#05`) is merged into `work/text-replacement` before starting — it's a hard dependency +- [x] 1.2 Branch off `work/text-replacement` as `feat/asciihex-decode` +- [x] 1.3 Re-read PDF 1.7 §7.4.2 and confirm decoder rules (whitespace + odd-length + EOD + alphabet) ## 2. Decode helper -- [ ] 2.1 Add `protected static function ASCIIHexDecode($_stream, $params)` in `src/PDFObject.php`, modelled on the existing `FlateDecode` static helper -- [ ] 2.2 Implement the decode state machine: accept hex digits + whitespace, terminate at `>`, pad odd trailing nibble with `0`, fail-safe on illegal characters via `p_error()` + return raw input +- [x] 2.1 Add `protected static function ASCIIHexDecode($_stream, $params)` in `src/PDFObject.php`, modelled on the existing `FlateDecode` static helper +- [x] 2.2 Implement the decode state machine: accept hex digits + whitespace, terminate at `>`, pad odd trailing nibble with `0`, fail-safe on illegal characters via `p_error()` + return raw input ## 3. Encode helper -- [ ] 3.1 Add `protected static function ASCIIHexEncode($_stream, $params)` — emit uppercase hex pairs, line-wrap at 80 columns, terminate with `>` -- [ ] 3.2 Handle the empty-input edge case (emit just `>`) +- [x] 3.1 Add `protected static function ASCIIHexEncode($_stream, $params)` — emit uppercase hex pairs, line-wrap at 80 columns, terminate with `>` +- [x] 3.2 Handle the empty-input edge case (emit just `>`) ## 4. Chain-dispatch integration -- [ ] 4.1 Add a `case '/ASCIIHexDecode'` arm to `apply_filter_chain_decode` (introduced in PR `#05`) routing to `self::ASCIIHexDecode(...)` -- [ ] 4.2 Add the symmetric arm in `apply_filter_chain_encode` routing to `self::ASCIIHexEncode(...)` +- [x] 4.1 Add a `case '/ASCIIHexDecode'` arm to `apply_filter_chain_decode` (introduced in PR `#05`) routing to `self::ASCIIHexDecode(...)` +- [x] 4.2 Add the symmetric arm in `apply_filter_chain_encode` routing to `self::ASCIIHexEncode(...)` ## 5. Tests / verification -- [ ] 5.1 Add a round-trip fixture in `examples/poc-filter-roundtrip-asciihex.php`: random 1024-byte buffer → encode → decode → assert byte-for-byte equality -- [ ] 5.2 Verify the existing `examples/poc-replace-text.php` still exits 0 (FlateDecode-only path unchanged) -- [ ] 5.3 Add a chain test exercising `[/ASCIIHexDecode /FlateDecode]` end-to-end (encode plaintext → assert decoded matches) -- [ ] 5.4 Add a negative test asserting `p_error` on an illegal-character input and stream-unchanged behaviour +- [x] 5.1 Add a round-trip fixture in `examples/poc-filter-roundtrip-asciihex.php`: random 1024-byte buffer → encode → decode → assert byte-for-byte equality +- [x] 5.2 Verify the existing `examples/poc-replace-text.php` still exits 0 (FlateDecode-only path unchanged) +- [x] 5.3 Add a chain test exercising `[/ASCIIHexDecode /FlateDecode]` end-to-end (encode plaintext → assert decoded matches) +- [x] 5.4 Add a negative test asserting `p_error` on an illegal-character input and stream-unchanged behaviour ## 6. Upstream-PR draft -- [ ] 6.1 Update `docs/upstream-prs/01-asciihex-decode/proposal.md` with the spec REQ references -- [ ] 6.2 Update `docs/upstream-prs/01-asciihex-decode/design.md` with D1–D5 decision log -- [ ] 6.3 Update `docs/upstream-prs/01-asciihex-decode/tasks.md` with the implementation summary -- [ ] 6.4 Leave `Posted at: ` placeholder +- [x] 6.1 Update `docs/upstream-prs/01-asciihex-decode/proposal.md` with the spec REQ references +- [x] 6.2 Update `docs/upstream-prs/01-asciihex-decode/design.md` with D1–D5 decision log +- [x] 6.3 Update `docs/upstream-prs/01-asciihex-decode/tasks.md` with the implementation summary +- [x] 6.4 Leave `Posted at: ` placeholder ## 7. Quality gate -- [ ] 7.1 PHP 7.4 compatibility check (no typed properties, no enums, no readonly) -- [ ] 7.2 No new composer dependencies -- [ ] 7.3 snake_case method discipline (`ASCIIHexDecode` / `ASCIIHexEncode` follow upstream's PascalCase-on-filter-names + snake_case-on-utility-method convention; verify against `FlateDecode`) +- [x] 7.1 PHP 7.4 compatibility check (no typed properties, no enums, no readonly) +- [x] 7.2 No new composer dependencies +- [x] 7.3 snake_case method discipline (`ASCIIHexDecode` / `ASCIIHexEncode` follow upstream's PascalCase-on-filter-names + snake_case-on-utility-method convention; verify against `FlateDecode`) ## 8. Commit + PR -- [ ] 8.1 Commit on `feat/asciihex-decode` -- [ ] 8.2 Open PR `feat/asciihex-decode → work/text-replacement` +- [x] 8.1 Commit on `feat/asciihex-decode` +- [x] 8.2 Open PR `feat/asciihex-decode → work/text-replacement` diff --git a/src/PDFObject.php b/src/PDFObject.php index 42e42c7..9ae5821 100644 --- a/src/PDFObject.php +++ b/src/PDFObject.php @@ -209,6 +209,74 @@ protected static function FlateDecode($_stream, $params) { return $decoded->get_raw(); } + /** + * Decode an ASCIIHexDecode-encoded stream per PDF 1.7 §7.4.2. + * + * Accepts the alphabet `0..9 A..F a..f` plus whitespace plus the + * EOD marker `>`. Whitespace is stripped before parsing. Odd-length + * input is padded with a `0` nibble per §7.4.2 ¶3. Bytes after `>` + * are ignored. Illegal characters → `p_error()` + return raw input. + * + * @param string $_stream Encoded stream bytes. + * @param mixed $params Unused (ASCIIHexDecode takes no parameters per Table 5). + * + * @return string|false Decoded binary bytes; original stream on illegal-character failure. + */ + protected static function ASCIIHexDecode($_stream, $params) { + // Find the EOD marker; everything after `>` is ignored. EOD is + // mandatory per §7.4.2 but we tolerate a missing EOD by treating + // the whole input as the data region (matches Adobe Reader). + $eodPos = strpos($_stream, '>'); + $hexRegion = $eodPos === false ? $_stream : substr($_stream, 0, $eodPos); + + // Strip the PDF whitespace set (§7.5.1: NUL, HT, LF, FF, CR, SP). + $compact = preg_replace('/[\\x00\\x09\\x0a\\x0c\\x0d\\x20]/', '', $hexRegion); + + // Validate the alphabet. + if (preg_match('/[^0-9A-Fa-f]/', $compact) === 1) { + p_error('ASCIIHexDecode: illegal character in encoded stream'); + return $_stream; + } + + // Odd-length: pad the trailing nibble with `0` per §7.4.2 ¶3. + if ((strlen($compact) % 2) === 1) { + $compact .= '0'; + } + + $bytes = @hex2bin($compact); + if ($bytes === false) { + // Defensive — every char passed the alphabet check, but pair + // up an unexpected ASCII case via the safe fallback. + p_error('ASCIIHexDecode: hex2bin failed despite alphabet validation'); + return $_stream; + } + return $bytes; + } + + /** + * Encode binary bytes via ASCIIHexEncode per PDF 1.7 §7.4.2. + * + * Emits uppercase hex pairs terminated by the EOD marker `>`. Line + * wraps inserted at 80 columns (Adobe-conventional; readers MUST + * tolerate any width per §7.4.2). Empty input emits just `>`. + * + * @param string $_stream Raw binary input. + * @param mixed $params Unused. + * + * @return string Encoded stream including EOD marker. + */ + protected static function ASCIIHexEncode($_stream, $params) { + $hex = strtoupper(bin2hex($_stream)); + if ($hex === '') { + return '>'; + } + // Chunk at 80 columns and re-join with newlines. + $wrapped = chunk_split($hex, 80, "\n"); + // chunk_split appends a trailing separator; trim it before the EOD. + $wrapped = rtrim($wrapped, "\n"); + return $wrapped . '>'; + } + /** * Normalise the `/Filter` entry to a plain array of filter-name strings. * @@ -345,6 +413,13 @@ protected static function apply_filter_chain_decode($bytes, array $filters, arra } $bytes = $decoded; break; + case 'ASCIIHexDecode': + $decoded = self::ASCIIHexDecode($bytes, $params[$i] ?? null); + if ($decoded === false) { + return false; + } + $bytes = $decoded; + break; default: p_error("unknown compression method /$name in filter chain at position $i"); return false; @@ -378,6 +453,9 @@ protected static function apply_filter_chain_encode($bytes, array $filters, arra } $bytes = $compressed; break; + case 'ASCIIHexDecode': + $bytes = self::ASCIIHexEncode($bytes, $params[$i] ?? null); + break; default: p_error("unknown compression method /$name in filter chain at position $i"); return false; From 70d9a3a728b6d39e2d22b59aebfcd409db7168bc Mon Sep 17 00:00:00 2001 From: Robert Zondervan Date: Thu, 28 May 2026 14:39:36 +0200 Subject: [PATCH 2/2] feat(asciihex): address Wilco's first-pass strict review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a merge commit bringing `feat/filter-chain-dispatch`'s fix commits (PoC snake_case rename + chain dispatcher get_stream-returns- false + Columns default fix + docs/upstream-prs deduplication) into this branch, and a follow-up commit with PR #4 specific fixes. 🔴 PR #4 Blockers - ASCIIHexDecode failure-path return changed from `$_stream` (raw input) to `false`. Matches the chain dispatcher's `=== false` short-circuit contract and upstream `p_error`'s default return. Critical correctness fix: prior to this, a malformed outer- ASCIIHex layer in a `[/ASCIIHexDecode /FlateDecode]` chain silently fed the unmodified illegal bytes to the inner FlateDecode arm — silent data corruption. - PHPDoc on ASCIIHexDecode updated: `@return string|false` is now accurate (the failure paths return `false` per the previous fix). Description clarified to surface the chain-dispatcher contract requirement. - Negative chain test added in `poc-filter-roundtrip-asciihex.php` asserting the dispatcher short-circuits on outer-ASCIIHex failure: `[/ASCIIHexDecode /FlateDecode]` chain with malformed hex bytes MUST return `false` from `get_stream(false)`. This locks the failure-propagation contract that the previous bug silently bypassed. 🟡 PR #4 Concerns - 80-col line wrap edge case in `ASCIIHexEncode`: when the hex representation is a multiple of 80, the encoder previously appended `>` directly after the final chunk, producing an 81- char line. Now inserts a newline before `>` so no line exceeds 80 chars. Locked by a new 40-byte (80-hex-char) test case. - `tasks.md` 1.2 corrected: shows the actual stacked-PR base (`feat/filter-chain-dispatch`) instead of `work/text-replacement`. - `tasks.md` 2.2 corrected: the failure-path returns `false` not raw input (matches the post-fix code). - `tasks.md` 4.1 corrected: case label is bare `'ASCIIHexDecode'` not `'/ASCIIHexDecode'` (the dispatcher strips the leading slash before the switch). - `docs/upstream-prs/01-asciihex-decode/{proposal,design,tasks}.md` no longer carry the duplicated 9-line implementation note. Each links to `openspec/changes/feat-asciihex-decode/design.md` as the canonical artefact (same pointer-only pattern as PR #3). 🟡 Concerns deferred to follow-ups - Numbering scheme reconciliation: covered by the `docs/upstream-prs/README.md` explainer in PR #3's fix commit (`upstream-PR #NN` vs fork PR # numbering), which arrives here via the merge. - Encode-dispatcher symmetry on infallible filters: ASCIIHexEncode documented as infallible; an explicit `=== false` symmetry would require a return-type contract change. Tabled. 🟢 Minors addressed in tests - Missing-EOD tolerance now has an assertion (`ASCIIHexDecode('48656C6C6F') === 'Hello'`). - Empty-input edge cases asserted: `'>'`, ` ' \n \t >'`. - Round-trip of empty input asserted: `decode(encode('')) === ''`. - Unnecessary `@` operator on `hex2bin` dropped (the alphabet validation above guarantees it can't legitimately fail). 🟢 Minors deferred - PHP case-insensitivity nit on `ASCIIHexDecode` collision — no action; PascalCase-on-filter-names is the established convention. - PHP 7.4 union return type in docs — the docs use PHPDoc-only notation already; the docs/upstream-prs note is reasonable. - ReflectionClass coupling — by design; documented. - `p_error` stdout assumption — acceptable; tests don't depend on the error channel routing. Verification: all 3 gates exit 0 - poc-replace-text.php (PR #1 gate) - poc-filter-chain-roundtrip.php (PR #3 gate) - poc-filter-roundtrip-asciihex.php (this PR's gate, with the new chain-failure-propagation test + 40-byte 80-col edge + the missing-EOD/empty-edge assertions) --- .../upstream-prs/01-asciihex-decode/design.md | 11 +-- .../01-asciihex-decode/proposal.md | 11 +-- docs/upstream-prs/01-asciihex-decode/tasks.md | 11 +-- examples/poc-filter-roundtrip-asciihex.php | 85 ++++++++++++++++++- .../specs/asciihex-decode-filter/spec.md | 17 ++-- .../changes/feat-asciihex-decode/tasks.md | 8 +- src/PDFObject.php | 29 +++++-- 7 files changed, 129 insertions(+), 43 deletions(-) diff --git a/docs/upstream-prs/01-asciihex-decode/design.md b/docs/upstream-prs/01-asciihex-decode/design.md index 723c58b..9020c9d 100644 --- a/docs/upstream-prs/01-asciihex-decode/design.md +++ b/docs/upstream-prs/01-asciihex-decode/design.md @@ -42,13 +42,8 @@ A nicer design would be a `FilterRegistry` with one class per filter. We don't i --- -## Implementation note (shipped 2026-05-27 on `feat/asciihex-decode`) - -Implementation landed on `feat/asciihex-decode` stacked on `feat/filter-chain-dispatch` (#05). Canonical contract lives in `openspec/changes/feat-asciihex-decode/`. Method names as shipped (all `protected static` on `PDFObject`): - -- `ASCIIHexDecode($_stream, $params): string|false` — strips whitespace via PDF 1.7 §7.5.1 character class, finds EOD `>` and discards anything after, pads odd trailing nibble with `0` (§7.4.2 ¶3), illegal chars → `p_error()` + return raw input. -- `ASCIIHexEncode($_stream, $params): string` — `strtoupper(bin2hex(...))` + `chunk_split(..., 80, "\n")` + EOD `>`, empty input emits just `>`. +--- -Both are wired into `apply_filter_chain_decode` / `apply_filter_chain_encode` via the `case 'ASCIIHexDecode':` arm. +## Implementation note -Verification: `examples/poc-filter-roundtrip-asciihex.php` covers all 5 REQs (basic + odd-length + whitespace + lowercase + trailing-after-EOD + lossless round-trip on 1024 random bytes + illegal-char fail-safe + chain integration single-filter and ASCIIHex+Flate two-filter). Companion dispatcher test on `feat/filter-chain-dispatch` updated to use `/SappTestUnknownFilter` so it stays valid as subsequent filter PRs land real implementations. +See `openspec/changes/feat-asciihex-decode/design.md` (the canonical artefact) for the shipped-implementation note. This file keeps the original proposal/design/tasks content for the eventual upstream submission to dealfonso/sapp; implementation specifics are tracked in the OpenSpec change to avoid duplicate-source drift. diff --git a/docs/upstream-prs/01-asciihex-decode/proposal.md b/docs/upstream-prs/01-asciihex-decode/proposal.md index 1d32b87..a796ab0 100644 --- a/docs/upstream-prs/01-asciihex-decode/proposal.md +++ b/docs/upstream-prs/01-asciihex-decode/proposal.md @@ -26,13 +26,8 @@ This feature is the first in a series adding decoder coverage for the text-relev --- -## Implementation note (shipped 2026-05-27 on `feat/asciihex-decode`) - -Implementation landed on `feat/asciihex-decode` stacked on `feat/filter-chain-dispatch` (#05). Canonical contract lives in `openspec/changes/feat-asciihex-decode/`. Method names as shipped (all `protected static` on `PDFObject`): - -- `ASCIIHexDecode($_stream, $params): string|false` — strips whitespace via PDF 1.7 §7.5.1 character class, finds EOD `>` and discards anything after, pads odd trailing nibble with `0` (§7.4.2 ¶3), illegal chars → `p_error()` + return raw input. -- `ASCIIHexEncode($_stream, $params): string` — `strtoupper(bin2hex(...))` + `chunk_split(..., 80, "\n")` + EOD `>`, empty input emits just `>`. +--- -Both are wired into `apply_filter_chain_decode` / `apply_filter_chain_encode` via the `case 'ASCIIHexDecode':` arm. +## Implementation note -Verification: `examples/poc-filter-roundtrip-asciihex.php` covers all 5 REQs (basic + odd-length + whitespace + lowercase + trailing-after-EOD + lossless round-trip on 1024 random bytes + illegal-char fail-safe + chain integration single-filter and ASCIIHex+Flate two-filter). Companion dispatcher test on `feat/filter-chain-dispatch` updated to use `/SappTestUnknownFilter` so it stays valid as subsequent filter PRs land real implementations. +See `openspec/changes/feat-asciihex-decode/design.md` (the canonical artefact) for the shipped-implementation note. This file keeps the original proposal/design/tasks content for the eventual upstream submission to dealfonso/sapp; implementation specifics are tracked in the OpenSpec change to avoid duplicate-source drift. diff --git a/docs/upstream-prs/01-asciihex-decode/tasks.md b/docs/upstream-prs/01-asciihex-decode/tasks.md index 15c6a46..1f53b2f 100644 --- a/docs/upstream-prs/01-asciihex-decode/tasks.md +++ b/docs/upstream-prs/01-asciihex-decode/tasks.md @@ -33,13 +33,8 @@ --- -## Implementation note (shipped 2026-05-27 on `feat/asciihex-decode`) - -Implementation landed on `feat/asciihex-decode` stacked on `feat/filter-chain-dispatch` (#05). Canonical contract lives in `openspec/changes/feat-asciihex-decode/`. Method names as shipped (all `protected static` on `PDFObject`): - -- `ASCIIHexDecode($_stream, $params): string|false` — strips whitespace via PDF 1.7 §7.5.1 character class, finds EOD `>` and discards anything after, pads odd trailing nibble with `0` (§7.4.2 ¶3), illegal chars → `p_error()` + return raw input. -- `ASCIIHexEncode($_stream, $params): string` — `strtoupper(bin2hex(...))` + `chunk_split(..., 80, "\n")` + EOD `>`, empty input emits just `>`. +--- -Both are wired into `apply_filter_chain_decode` / `apply_filter_chain_encode` via the `case 'ASCIIHexDecode':` arm. +## Implementation note -Verification: `examples/poc-filter-roundtrip-asciihex.php` covers all 5 REQs (basic + odd-length + whitespace + lowercase + trailing-after-EOD + lossless round-trip on 1024 random bytes + illegal-char fail-safe + chain integration single-filter and ASCIIHex+Flate two-filter). Companion dispatcher test on `feat/filter-chain-dispatch` updated to use `/SappTestUnknownFilter` so it stays valid as subsequent filter PRs land real implementations. +See `openspec/changes/feat-asciihex-decode/design.md` (the canonical artefact) for the shipped-implementation note. This file keeps the original proposal/design/tasks content for the eventual upstream submission to dealfonso/sapp; implementation specifics are tracked in the OpenSpec change to avoid duplicate-source drift. diff --git a/examples/poc-filter-roundtrip-asciihex.php b/examples/poc-filter-roundtrip-asciihex.php index de27ac5..25a131a 100644 --- a/examples/poc-filter-roundtrip-asciihex.php +++ b/examples/poc-filter-roundtrip-asciihex.php @@ -121,15 +121,94 @@ function buildObj($filterValue, string $rawStream = ''): PDFObject { } /* ---------------------------------------------------------------- * - * REQ-4 — illegal character fail-safe + * REQ-4 — illegal character fail-safe (returns false per dispatcher contract) * ---------------------------------------------------------------- */ { $original = '48656C!6C6F>'; // contains '!' ob_start(); $decoded = invokeProtectedStatic('ASCIIHexDecode', [$original, null]); ob_end_clean(); - if ($decoded !== $original) { - $failures[] = "REQ-4 (illegal char): expected raw input passthrough on failure, got " . bin2hex((string) $decoded); + if ($decoded !== false) { + $failures[] = "REQ-4 (illegal char): expected false on failure, got " . var_export($decoded, true); + } +} + +/* ---------------------------------------------------------------- * + * REQ-4 — chain-failure propagation: when an outer ASCIIHexDecode + * layer is malformed, the chain dispatcher MUST short-circuit and + * return false rather than feeding garbage to the next filter. + * ---------------------------------------------------------------- */ +{ + $filterList = new PDFValueList(); + $filterList->push(new PDFValueType('ASCIIHexDecode')); + $filterList->push(new PDFValueType('FlateDecode')); + + // Raw stream is malformed hex (contains '!') — outer ASCIIHex + // arm MUST fail before the inner FlateDecode arm ever runs. + $obj = buildObj($filterList, '48656C!6C6F>somegarbage'); + + ob_start(); + $decoded = $obj->get_stream(false); + ob_end_clean(); + + if ($decoded !== false) { + $failures[] = "REQ-4 (chain failure propagation): get_stream did not return false on outer-layer ASCIIHex failure (got " . var_export($decoded, true) . ")"; + } +} + +/* ---------------------------------------------------------------- * + * Missing-EOD tolerance (Adobe-compatible documented behaviour) + * ---------------------------------------------------------------- */ +{ + // Input without trailing `>` — decoder should treat the whole + // input as the data region. + $decoded = invokeProtectedStatic('ASCIIHexDecode', ['48656C6C6F', null]); + if ($decoded !== 'Hello') { + $failures[] = "Missing-EOD tolerance: expected 'Hello', got " . bin2hex((string) $decoded); + } +} + +/* ---------------------------------------------------------------- * + * EOD-first / empty edge cases (REQ-1 boundary coverage) + * ---------------------------------------------------------------- */ +{ + $decoded = invokeProtectedStatic('ASCIIHexDecode', ['>', null]); + if ($decoded !== '') { + $failures[] = "EOD-first empty: expected '', got " . bin2hex((string) $decoded); + } + $decoded = invokeProtectedStatic('ASCIIHexDecode', [" \n \t >", null]); + if ($decoded !== '') { + $failures[] = "Whitespace+EOD empty: expected '', got " . bin2hex((string) $decoded); + } +} + +/* ---------------------------------------------------------------- * + * REQ-2/3 — empty input round-trip + 40-byte 80-col boundary + * ---------------------------------------------------------------- */ +{ + // Round-trip of empty input (decode(encode('')) === ''). + $encoded = invokeProtectedStatic('ASCIIHexEncode', ['', null]); + $decoded = invokeProtectedStatic('ASCIIHexDecode', [$encoded, null]); + if ($decoded !== '') { + $failures[] = "REQ-2 (empty round-trip): expected '' got " . bin2hex((string) $decoded); + } + + // 40-byte input = 80 hex chars — encodes to a single line of + // exactly 80 chars. The encoder MUST insert a newline before `>` + // so no line exceeds 80 chars (or at least clearly handle the + // boundary; the encoder ships with a `strlen % 80 === 0` guard). + $payload = str_repeat('A', 40); + $encoded = invokeProtectedStatic('ASCIIHexEncode', [$payload, null]); + foreach (explode("\n", $encoded) as $line) { + if (strlen($line) > 80) { + $failures[] = "REQ-2 (40-byte 80-col edge): line longer than 80 chars (" . strlen($line) . ")"; + break; + } + } + // Round-trip MUST still produce the original. + $decoded = invokeProtectedStatic('ASCIIHexDecode', [$encoded, null]); + if ($decoded !== $payload) { + $failures[] = "REQ-2 (40-byte round-trip): round-trip mismatch on 80-col boundary"; } } diff --git a/openspec/changes/feat-asciihex-decode/specs/asciihex-decode-filter/spec.md b/openspec/changes/feat-asciihex-decode/specs/asciihex-decode-filter/spec.md index 805a500..917930b 100644 --- a/openspec/changes/feat-asciihex-decode/specs/asciihex-decode-filter/spec.md +++ b/openspec/changes/feat-asciihex-decode/specs/asciihex-decode-filter/spec.md @@ -59,14 +59,21 @@ For any input byte string `$P`, `ASCIIHexDecode(ASCIIHexEncode($P, null), null)` ### Requirement: ASCIIHexDecode SHALL fail safely on illegal characters -When the decoder encounters a character outside the alphabet plus whitespace plus EOD, it MUST call `p_error()` with a message identifying the failure and MUST return the input bytes unchanged. +When the decoder encounters a character outside the alphabet plus whitespace plus EOD, it MUST call `p_error()` with a message identifying the failure and MUST return `false` (matching upstream `p_error`'s default return + the chain dispatcher's `=== false` short-circuit contract). It MUST NOT return the raw input bytes — that would let downstream filters in a chain see corrupted bytes. #### Scenario: Illegal character fails safely -- **WHEN** the encoded stream is `48656C!6C6F>` (contains `!`) -- **THEN** the decoder MUST call `p_error()` with a message naming the illegal character -- **AND** the return value MUST equal the original encoded stream (unchanged) -- **AND** no exception MUST be thrown +- GIVEN the encoded stream is `48656C!6C6F>` (contains `!`) +- WHEN `ASCIIHexDecode` is invoked +- THEN the decoder MUST call `p_error()` with a message naming the illegal character +- AND the return value MUST be `false` (so the chain dispatcher's `if ($decoded === false) return false;` arm fires) +- AND no exception MUST be thrown + +#### Scenario: Chain-failure propagation on outer-ASCIIHex layer + +- GIVEN an object with `/Filter [/ASCIIHexDecode /FlateDecode]` and a raw `_stream` whose ASCIIHex layer contains an illegal character +- WHEN `get_stream(false)` is invoked +- THEN the chain dispatcher MUST short-circuit at the ASCIIHex arm and return `false`; the inner `FlateDecode` arm MUST NOT see the malformed bytes ## MODIFIED Requirements diff --git a/openspec/changes/feat-asciihex-decode/tasks.md b/openspec/changes/feat-asciihex-decode/tasks.md index 53d2b98..e0d9d01 100644 --- a/openspec/changes/feat-asciihex-decode/tasks.md +++ b/openspec/changes/feat-asciihex-decode/tasks.md @@ -1,13 +1,13 @@ ## 1. Pre-flight -- [x] 1.1 Confirm `feat-filter-chain-dispatch` (PR `#05`) is merged into `work/text-replacement` before starting — it's a hard dependency -- [x] 1.2 Branch off `work/text-replacement` as `feat/asciihex-decode` +- [x] 1.1 Confirm `feat-filter-chain-dispatch` (upstream-PR #05; fork PR #3) is merged into the integration line OR available as the stack-parent branch +- [x] 1.2 Branch off `feat/filter-chain-dispatch` as `feat/asciihex-decode` (stacked PR; base is the dispatcher branch, NOT `work/text-replacement` directly — once #05 merges upstream, this PR's base auto-retargets) - [x] 1.3 Re-read PDF 1.7 §7.4.2 and confirm decoder rules (whitespace + odd-length + EOD + alphabet) ## 2. Decode helper - [x] 2.1 Add `protected static function ASCIIHexDecode($_stream, $params)` in `src/PDFObject.php`, modelled on the existing `FlateDecode` static helper -- [x] 2.2 Implement the decode state machine: accept hex digits + whitespace, terminate at `>`, pad odd trailing nibble with `0`, fail-safe on illegal characters via `p_error()` + return raw input +- [x] 2.2 Implement the decode state machine: accept hex digits + whitespace, terminate at `>`, pad odd trailing nibble with `0`, fail-safe on illegal characters via `p_error()` + return `false` (the chain dispatcher's `=== false` short-circuit contract; matches upstream `p_error`'s default return) ## 3. Encode helper @@ -16,7 +16,7 @@ ## 4. Chain-dispatch integration -- [x] 4.1 Add a `case '/ASCIIHexDecode'` arm to `apply_filter_chain_decode` (introduced in PR `#05`) routing to `self::ASCIIHexDecode(...)` +- [x] 4.1 Add a `case 'ASCIIHexDecode'` arm to `apply_filter_chain_decode` (introduced in upstream-PR #05; note the dispatcher strips the leading `/` from filter names before the switch — case label is the bare name without slash) routing to `self::ASCIIHexDecode(...)` - [x] 4.2 Add the symmetric arm in `apply_filter_chain_encode` routing to `self::ASCIIHexEncode(...)` ## 5. Tests / verification diff --git a/src/PDFObject.php b/src/PDFObject.php index 24eb825..9247e9f 100644 --- a/src/PDFObject.php +++ b/src/PDFObject.php @@ -215,12 +215,18 @@ protected static function FlateDecode($_stream, $params) { * Accepts the alphabet `0..9 A..F a..f` plus whitespace plus the * EOD marker `>`. Whitespace is stripped before parsing. Odd-length * input is padded with a `0` nibble per §7.4.2 ¶3. Bytes after `>` - * are ignored. Illegal characters → `p_error()` + return raw input. + * are ignored. Illegal characters → `p_error()` + return `false` + * (matching the chain dispatcher's `=== false` fail-safe and + * upstream `p_error`'s default return semantics; the chain arm + * MUST short-circuit on failure so downstream filters do not see + * corrupted bytes). * * @param string $_stream Encoded stream bytes. * @param mixed $params Unused (ASCIIHexDecode takes no parameters per Table 5). * - * @return string|false Decoded binary bytes; original stream on illegal-character failure. + * @return string|false Decoded binary bytes on success; `false` on + * illegal-character or hex2bin failure so the + * filter chain aborts cleanly. */ protected static function ASCIIHexDecode($_stream, $params) { // Find the EOD marker; everything after `>` is ignored. EOD is @@ -235,7 +241,7 @@ protected static function ASCIIHexDecode($_stream, $params) { // Validate the alphabet. if (preg_match('/[^0-9A-Fa-f]/', $compact) === 1) { p_error('ASCIIHexDecode: illegal character in encoded stream'); - return $_stream; + return false; } // Odd-length: pad the trailing nibble with `0` per §7.4.2 ¶3. @@ -243,12 +249,15 @@ protected static function ASCIIHexDecode($_stream, $params) { $compact .= '0'; } - $bytes = @hex2bin($compact); + // The alphabet check above guarantees `hex2bin` can't legitimately + // fail here — drop the historical `@` suppression so unexpected + // failures surface in PHP's error reporting. + $bytes = hex2bin($compact); if ($bytes === false) { - // Defensive — every char passed the alphabet check, but pair - // up an unexpected ASCII case via the safe fallback. + // Defensive — paired-validation passed but hex2bin still + // disagreed. Fail-safe per the chain dispatcher contract. p_error('ASCIIHexDecode: hex2bin failed despite alphabet validation'); - return $_stream; + return false; } return $bytes; } @@ -274,6 +283,12 @@ protected static function ASCIIHexEncode($_stream, $params) { $wrapped = chunk_split($hex, 80, "\n"); // chunk_split appends a trailing separator; trim it before the EOD. $wrapped = rtrim($wrapped, "\n"); + // If the final hex chunk lands exactly on the 80-col boundary, + // appending `>` would push that line to 81 chars. Insert a + // newline before the EOD so no line exceeds 80. + if (strlen($hex) % 80 === 0) { + return $wrapped . "\n>"; + } return $wrapped . '>'; }