fix(converters): preserve query param encoding in getQueryFromSearchParams - #1197
Open
azuma317 wants to merge 1 commit into
Open
fix(converters): preserve query param encoding in getQueryFromSearchParams#1197azuma317 wants to merge 1 commit into
azuma317 wants to merge 1 commit into
Conversation
…arams getQueryFromSearchParams iterated over URLSearchParams.entries(), which decodes the values. Since convertToQueryString does not re-encode them (intentionally, since opennextjs#817), query values containing reserved characters (&, =, +, spaces, ...) were corrupted when req.url was rebuilt from the parsed query, silently truncating or mangling the query string. Keep the values percent-encoded on parse so they round-trip correctly through convertToQueryString. Fixes opennextjs/opennextjs-cloudflare#1134
🦋 Changeset detectedLatest commit: 6cb2732 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
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.
What
getQueryFromSearchParamsparsed query values by iterating overURLSearchParams.entries(), which decodes each value. However, since #817,convertToQueryStringintentionally does not re-encode values (it concatenates them raw to preserve their original encoding). This leaves the two sides of the round-trip inconsistent:getQueryFromSearchParams) → decoded valuesconvertToQueryString) → assumes already-encoded values, no re-encodingAs a result, any query value containing reserved characters (
&,=,+, spaces, …) is corrupted whenreq.urlis rebuilt from the parsed query inrequestHandler.ts:A decoded value such as
https://example.com/auth?a=1&b=2gets concatenated as-is, so the embedded&/=split the outer query string and everything after the first reserved char is lost or reinterpreted as separate params.Reproduction
This is the root cause of opennextjs/opennextjs-cloudflare#1134. It reliably breaks, for example, Sign in with Apple via
better-auth's Expo flow: the client hits…/expo-authorization-proxy?authorizationURL=<percent-encoded URL>, the encoded%26/%3DinsideauthorizationURLare decoded to&/=, the URL is truncated at the first&, thestateparam is lost, and the endpoint returns400 "Unexpected error". It only manifests in production (Workers / any deployment that goes through thisreq.urlreconstruction), not innext dev.Fix
Keep the parsed values percent-encoded so they round-trip correctly through
convertToQueryString. This completes the model established in #817 (query records hold encoded values end-to-end) rather than revertingconvertToQueryString— which would reintroduce the double-decode problem #817 fixed and break the existing "should respect existing query encoding" test.Multi-value grouping and the empty-query case are preserved.
Tests
Added unit tests for
getQueryFromSearchParamscovering the empty case, single value, repeated-key grouping, and the reserved-character round-trip from #1134. Full unit suite (604 passed),biome check, andtsc --noEmitall pass.