Skip to content

fix: reject invalid UTF-8 enqueue bodies (#116) - #133

Merged
jonbaldie merged 1 commit into
mainfrom
fix/116-reject-invalid-utf8-final
Sep 21, 2026
Merged

jonbaldie merged 1 commit into
mainfrom
fix/116-reject-invalid-utf8-final

Conversation

@jonbaldie

Copy link
Copy Markdown
Owner

Summary

  • Decode enqueue request bytes with a fatal UTF-8 decoder.
  • Return the existing 400 Invalid JSON response for malformed UTF-8 before JSON parsing or enqueueing.
  • Add HTTP seam regression coverage for malformed bytes, no enqueue, and valid UTF-8 round trips.
  • Document malformed UTF-8 as a rejected request body.

Response.text() used replacement-mode decoding, so an invalid byte inside a JSON string became U+FFFD and the request incorrectly returned 200. The request bytes are now decoded with TextDecoder("utf-8", { fatal: true }).

Verification

  • Full Deno suite: 333 passed, 0 failed
  • npm run quality:unit -- http-handler
  • npm run quality:production
  • Live HTTP replay: malformed 0xe9 returns 400 Invalid JSON; dequeue returns 204
  • The regression test was red before the fix (200 instead of 400) and green after it.

Closes #116

Decode request bytes with a fatal UTF-8 decoder so malformed JSON strings return the existing 400 Invalid JSON response instead of being replaced with U+FFFD. Add HTTP seam coverage for rejection, no enqueue, and valid UTF-8 round trips.
@jonbaldie
jonbaldie merged commit 9ace31e into main Sep 21, 2026
4 checks passed
@jonbaldie
jonbaldie deleted the fix/116-reject-invalid-utf8-final branch September 21, 2026 17:12
jonbaldie added a commit that referenced this pull request Sep 22, 2026
* fix: make rate limiter checks independent of window size (#125)

isAllowed() filtered the caller's full timestamp window on every call,
including denied requests and pre-auth traffic, so per-request cost grew
linearly with RATE_LIMIT_REQUESTS and collapsed quadratically under load.

Timestamps are sorted ascending, so stale entries form a prefix: take an
O(1) fast path when the newest is fresh, otherwise binary search the
first fresh entry (O(log n)) and count the window without filtering or
copying. Drop the stale prefix only once it dominates the array so the
copy stays amortized O(1) per recorded request. All-stale entries are
still removed, and periodic cleanup and eviction semantics are unchanged.

* test: restore persist.ts mutation coverage below the 80% gate

The atomic-snapshot rewrite in #122 left persist.ts at 78-79% on
Stryker, failing the per-file threshold for any PR that touches tests.
Kill the surviving mutants through the public FileStore interface:
full truncation of stale temp content, rethrowing unusable-temp-path
errors, temp cleanup on failed rename, and multi-byte reassembly
across 4096-byte read boundaries.

* refactor: extract firstFreshIndex to keep isAllowed under complexity gate

* fix: return 400 for JSON nested beyond the parser's stack depth (#123) (#128)

* fix: return 400 for JSON nested beyond the parser's stack depth (#123)

Root cause: V8 applies a JSON.parse reviver recursively, one stack frame
per nesting level. The enqueue reviver (unsupported-number check) made
bodies nested ~3,100+ levels deep throw RangeError: Maximum call stack
size exceeded. enqueueErrorResponse only maps SyntaxError, so the
RangeError escaped as an uncaught 500. Plain JSON.parse and
JSON.stringify both handle 100,000+ levels, so the parse step was the
only point of failure.

parseJsonBody now converts a RangeError raised by JSON.parse into a
SyntaxError. The request gets the existing 400 "Invalid JSON" response
and the Queue is not changed. The catch covers only the parse call, so
RangeErrors from anywhere else still surface as 500s.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor: map parser depth overflow via a local error type

Throwing `new SyntaxError` added a module dependency and pushed
handler.ts to the CouplingBetweenObjects limit (13) in the production
quality gate. A local JsonNestingTooDeepError, mapped to the same
400 "Invalid JSON" response, follows the existing UnsupportedNumberError
pattern and keeps coupling at 12.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test: restore persist.ts mutation coverage below the 80% gate

The atomic-snapshot rewrite in #122 left persist.ts at 78-79% on
Stryker, failing the per-file threshold for any PR that touches tests.
Kill the surviving mutants through the public FileStore interface:
full truncation of stale temp content, rethrowing unusable-temp-path
errors, temp cleanup on failed rename, and multi-byte reassembly
across 4096-byte read boundaries.

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

* fix: reject invalid UTF-8 enqueue bodies (#116) (#133)

Decode request bytes with a fatal UTF-8 decoder so malformed JSON strings return the existing 400 Invalid JSON response instead of being replaced with U+FFFD. Add HTTP seam coverage for rejection, no enqueue, and valid UTF-8 round trips.

* fix: avoid per-value JSON reviver overhead (#134)

Parse request bodies natively, then scan the original JSON source to validate number literals. This preserves exact-number rejection while avoiding a reviver callback for every value in number-dense payloads.

Add public handler regressions for exact integers, nested metadata, JSON strings, and the explicit nesting limit.

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@jonbaldie jonbaldie mentioned this pull request Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enqueue silently replaces invalid UTF-8 in JSON strings with U+FFFD and returns 200

1 participant