Tag path params as UTF-8 instead of leaving them binary - #2839
Open
ericproulx wants to merge 1 commit into
Open
Conversation
Mustermann decodes path captures out of PATH_INFO, which Rack hands over
tagged ASCII-8BIT, and nothing re-tagged the result. Query and body params
arrive UTF-8 because Rack tags those itself, so the same value reached the
endpoint with a different encoding depending on where it came from.
That made an API's declarations disagree with themselves — a binary string
never equals the UTF-8 literal it was written as:
params { requires :id, type: String, values: ['café'] }
GET /?id=café -> 200
GET /café -> 400 "id does not have a valid value"
The same held for same_as, except_values and any comparison an endpoint made
against a non-ASCII literal. It also leaked into serialization: a non-ASCII
path param rendered into a JSON response drew an encoding warning from the
json gem, which that gem says will become an error in json 3.0.
Re-tag in Route#params_for, the single funnel for path-extracted values, the
way Rails does after unescaping a path. Only the encoding changes: the bytes
are untouched, so an invalid sequence stays invalid and is still caught
downstream instead of being silently scrubbed into something the client never
sent. Unnamed splats capture into an Array, so those are walked too.
No extra allocations — the captures are freshly built and unfrozen, so the
re-tag happens in place.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ericproulx
force-pushed
the
fix/path-param-encoding
branch
from
July 29, 2026 20:59
b3d9d12 to
f70f13c
Compare
Danger ReportNo issues found. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Mustermann decodes path captures out of
PATH_INFO, which Rack hands over taggedASCII-8BIT, and nothing re-tagged the result. Query and body params arriveUTF-8because Rack tags those itself — so the same value reached the endpoint with a different encoding depending on where it came from.That made an API's own declarations disagree with themselves, since a binary string never equals the UTF-8 literal it was written as:
GET /?id=café(query)GET /café(path)"id does not have a valid value"The same held for
same_as,except_values, and any comparison an endpoint made against a non-ASCII literal. It also leaked into serialization: a non-ASCII path param rendered into a JSON response drewwarning: JSON.generate: UTF-8 string passed as BINARY, which thejsongem says will become an error in json 3.0.Approach
Re-tag in
Route#params_for, the single funnel for path-extracted values, the way Rails does after unescaping a path.Only the encoding changes — the bytes are untouched, so an invalid sequence stays invalid and is still caught downstream rather than being silently scrubbed into something the client never sent. Unnamed splats capture into an
Array({"splat" => ["a", "b"]}), so those are walked too.Backward compatibility
UPGRADING entry added. Comparisons against pure-ASCII strings are unaffected, which is the overwhelming majority of code. Code that relied on a path param being binary — concatenating one with genuinely binary data — can now raise
Encoding::CompatibilityErrorand should call.bon the param. An app that already worked around this with its ownforce_encodingneeds no change; that call becomes a no-op.Perf
No extra allocations: the captures are freshly built and unfrozen, so
+valuereturns self and the re-tag happens in place. Measured onparams_forin isolation (two captures, 200 calls):~0.24 µs per matched request, in a method that is itself a small slice of the request. I tried folding the tagging into a single
each_with_objectpass to avoid the extra traversal; it was slower than keeping the C-optimizedcompact/symbolize_keys/transform_values!chain (1.45M vs 1.57M i/s on the hash transform alone), so the straightforward form stayed.Test plan
api_spec.rbcovering single/multiple/splat/unnamed-splat captures, thevalues:equality case, and a byte-preservation invariant; verified the behavioural ones fail without thelib/change.🤖 Generated with Claude Code