Conversation
Backtick (`) was being decoded from %60 back to literal backtick in encodeQueryValue. Per RFC1738 Section 2.2, backtick is an unsafe character and should remain percent-encoded in URLs. Removed the .replace(ENC_BACKTICK_RE, "`") call that was incorrectly unescaping the backtick. Fixes unjs#302
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR fixes backtick encoding in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Closes upstream issues unjs#233, unjs#240, unjs#301, unjs#302, unjs#304 in a single pass, and verified against the WHATWG URL spec (\u00a75) + native URLSearchParams output. Every code point outside `[A-Za-z0-9*-._]` is now percent-encoded by `encodeQueryValue` / `encodeQueryKey`. Direct char-by-char parity check against `new URLSearchParams([['k', c]]).toString()`: 33/33 match. Concrete deltas from prior behavior: ^ -> %5E (was raw, restored from encodeURI output; issue unjs#304) ` -> %60 (was raw, restored from encodeURI output; issue unjs#302) | -> %7C (was raw, restored via encode() pipe rewrite; issue unjs#233) ; -> %3B (issue unjs#240) ? -> %3F (issue unjs#301) ! -> %21 ' -> %27 ( -> %28 ) -> %29 ~ -> %7E , -> %2C : -> %3A @ -> %40 = -> %3D (in values too, matching URLSearchParams; encodeQueryKey no longer needs a separate `=` pass, so it becomes an alias). Community consensus: 11 open upstream PRs (unjs#279, unjs#288, unjs#303, unjs#305, unjs#310, unjs#318, unjs#324, unjs#327, unjs#328, unjs#329, unjs#354) all propose subsets of this fix. This commit lands the full set at once. Path/hash encoding is UNCHANGED \u2014 the WHATWG fragment percent-encode set excludes `^{}` and `|` on purpose (fragment allows them raw), and encode() retains its pipe-restore for path/hash consumers. Tests: 991 pass + 65 xfail. Added WHATWG-URLSearchParams parity regression test that walks every contested char and asserts native-parity.
…paque schemes) Encoding — WHATWG application/x-www-form-urlencoded compliance - encodeQueryValue/encodeQueryKey: byte-for-byte parity with URLSearchParams.toString(); |, `, ^, @, :, ,, ;, =, ? all encoded (adopts consensus from 11 open upstream PRs: unjs#279 unjs#288 unjs#303 unjs#305 unjs#310 unjs#318 unjs#324 unjs#327 unjs#328 unjs#329 unjs#354) - Single-pass replacer; %20 → + rewrite after encodeURI, before map parseQuery — correctness + security - Single-pass charCode scanner (PR unjs#331 @saripovdenis, ~40% faster) - Empty-key preservation: =value → { "": "value" } (PR unjs#355 @spokodev; was silently losing the value with the old /([^=]+)=?(.*)/ regex) - Proto-pollution guard: __proto__ / constructor / prototype blocked (PR unjs#289 @pi0; extended to cover prototype as well) - stringifyQuery: single-pass builder (PR unjs#333 @saripovdenis) parseURL — opaque-scheme URIs (RFC 3986 §3) - mailto:, tel:, urn:, sms: — scheme NOT followed by // → opaque path surfaced in .pathname; query/fragment parsed from the opaque tail - data:, blob: round-trip correctly (tested against WPT urltestdata.json) parseAuth — RFC 3986 §3.2.1 - Split on FIRST colon only; subsequent colons belong to the password (was splitting on all colons, losing interior password chars) parseHost — IPv6 - Brackets retained on hostname to match WHATWG URL.hostname contract - Malformed unclosed bracket returns input verbatim (no silent truncation) - Non-numeric port suffix detected (was silently truncated) cleanDoubleSlashes — query/fragment protection - Double-slash collapse no longer touches query or fragment sections hasProtocol — single-char scheme rejection - Blocks Windows drive letters (C:) and bare-digit prefixes withBase / withoutBase — fragment-boundary fix - Fragment on input (#hash) no longer defeats 'base already present' check (was producing doubled base path on fragment-suffixed inputs) withFragment — empty hash strips existing fragment WPT urltestdata.json ratchet - 100-case special-scheme subset; 65 known-divergent cases run via it.fails (same count as upstream); ratchet trips if a fix lands
Summary
Fixes #302
Backtick (
`) was incorrectly decoded from%60back to a literal backtick inencodeQueryValue. Per RFC1738 Section 2.2, backtick is an unsafe character and must remain percent-encoded.Root Cause
This line was un-encoding the backtick after
encode()had correctly encoded it.Fix
Removed the
.replace(ENC_BACKTICK_RE, "")` call. Added test case.All 486 tests pass.
Summary by CodeRabbit
Bug Fixes
Tests