fix(url_decode): stop malformed escapes failing the render - #970
josefguenther wants to merge 2 commits into
Conversation
url_decode passed its whole input to decodeURIComponent, so a single malformed escape (a "%" not followed by two hex digits, or an incomplete UTF-8 sequence) threw a RenderError and failed the render, discarding any valid escapes around it. It now decodes each run of well-formed escapes separately and leaves the rest as written, which is how Shopify's CGI.unescape treats a stray "%". "+" is still turned into a space before decoding, so %2B stays a literal plus. Fixes harttle#966 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
I find one discrepency: Not sure for other cases, but you can check with Shopify/liquid. |
Each run of escapes was decoded with decodeURIComponent and kept as written if that threw, so one byte that isn't valid UTF-8 left its whole run undecoded: "caf%C3%A9%ff" rendered as written rather than "café�". Each run is now decoded as UTF-8 bytes, and a byte sequence that forms no character becomes U+FFFD, as the URL Standard's percent-decoding does. A leading byte order mark is kept, as decodeURIComponent keeps it. The docs page now states how malformed input renders. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for checking against Ruby. You're right about those two rows, and they led me to a worse case the first commit got wrong: one bad byte left its whole run of escapes undecoded, so The invalid-UTF-8 rows are the same question as base64 in #971; I've written up there why I think the render should continue. |
| export const url_decode = (x: string) => stringify(x) | ||
| .replace(/\+/g, ' ') | ||
| .replace(rPercentEscapes, escapes => new TextDecoder('utf-8', { ignoreBOM: true }) | ||
| .decode(Uint8Array.from(escapes.slice(1).split('%'), hex => parseInt(hex, 16)))) |
There was a problem hiding this comment.
for the Uint8Array and TextDecoder, need add test on both Node and browser bundles.
There was a problem hiding this comment.
They're not as old and stable as decodeURIComponent
url_decode(src/filters/url.ts:3) passes its whole input todecodeURIComponent, so a single malformed escape throws and fails the render.{{ v | url_decode }}:vrender100%100%100%%zz%zz%zz%E2%9C%93 done 100%✓ done 100%✓ done 100%a%2Bb 100%a+b 100%a+b 100%%E2%9CLiquid error: invalid byte sequence in UTF-8�%ffLiquid error: invalid byte sequence in UTF-8�caf%C3%A9%ffLiquid error: invalid byte sequence in UTF-8café�caf%C3%A9,a+b%20c,1%2B1This decodes each run of
%XXescapes as UTF-8 bytes, so:%that isn't followed by two hex digits stays as written, as in Shopify;URLSearchParams) does.Shopify's
url_decoderaises on invalid UTF-8 instead. Its defaultrenderwrites that error inline and carries on, while LiquidJS fails the whole render, so this takes the URL Standard's answer.For input that decodes today the output is unchanged.
+still becomes a space before decoding, so #939's%2Bhandling stays. The docs page for the filter now states how malformed input renders.Four new tests in
test/integration/filters/url.spec.ts: a stray%, an incomplete sequence, and valid and invalid bytes in one run fail on master and pass here; a leading byte order mark passes on both and guards against the decoder dropping it.npm run build,npm test(1676 tests),npm run lintandnpm run build:docspass on Node 24.11.1.Fixes #966
Written with AI assistance (Claude Code); I have reviewed the change.