Sitelet https://github.com/unjs/ufo/pull/328
Skip to content

fix(encoding): keep backtick encoded in query values - #328

Open
mvtandas wants to merge 1 commit into
unjs:mainfrom
mvtandas:fix/backtick-encoding
Open

mvtandas wants to merge 1 commit into
unjs:mainfrom
mvtandas:fix/backtick-encoding

Conversation

@mvtandas

@mvtandas mvtandas commented Apr 14, 2026 •

Copy link
Copy Markdown

Summary

Fixes #302

Backtick (`) was incorrectly decoded from %60 back to a literal backtick in encodeQueryValue. Per RFC1738 Section 2.2, backtick is an unsafe character and must remain percent-encoded.

Root Cause

// src/encoding.ts
.replace(ENC_BACKTICK_RE, "`")  // ← decodes %60 back to `

This line was un-encoding the backtick after encode() had correctly encoded it.

Fix

Removed the .replace(ENC_BACKTICK_RE, "")` call. Added test case.

encodeQueryValue("a`b") // before: "a`b" ❌ | after: "a%60b" ✅

All 486 tests pass.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed query value encoding to properly retain percent-encoded backticks in accordance with RFC standards.
  • Tests

    • Added test case for backtick encoding in query values.

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
@coderabbitai

coderabbitai Bot commented Apr 14, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2dc78f96-9e6a-4a60-a023-7882075410d2

📥 Commits

Reviewing files that changed from the base of the PR and between f729916 and b5565cf.

📒 Files selected for processing (2)
  • src/encoding.ts
  • test/encoding.test.ts

📝 Walkthrough

Walkthrough

The PR fixes backtick encoding in encodeQueryValue to keep backticks percent-encoded as %60 per RFC1738 instead of decoding them to literal characters. A test case validates this corrected behavior.

Changes

Cohort / File(s) Summary
Backtick Encoding Fix
src/encoding.ts, test/encoding.test.ts
Removed backtick decode replacement in encodeQueryValue to keep backticks percent-encoded as %60 per RFC1738; added test case validating correct encoding of backticks.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A backtick once danced so free,
Encoded then decoded, wild and spry,
But RFC said "please, let it be,"
Stay as %60—that's no lie,
Now hop with proper encoding, you and I! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: preventing backtick from being decoded from %60 in query values, which is the primary objective.
Linked Issues check ✅ Passed The PR successfully addresses issue #302 by removing the backtick decode replacement, ensuring encodeQueryValue('a`b') returns 'a%60b' per RFC1738, and includes a test case.
Out of Scope Changes check ✅ Passed All changes are in-scope: the encoding fix targets the reported backtick issue, the test case validates the fix, and only comments are updated for clarity.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

silverfish2525 added a commit to silverfish2525/better-ufo that referenced this pull request Jul 1, 2026
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.
silverfish2525 added a commit to silverfish2525/better-ufo that referenced this pull request Jul 1, 2026
…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
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.

Wrong back tick encoding in encodeQueryValue

1 participant