From 501e8f592712f3833ca4196395182c392947777b Mon Sep 17 00:00:00 2001 From: Bendik Aagaard Lynghaug Date: Tue, 8 Sep 2026 16:50:34 +0200 Subject: [PATCH] fix(core): URL normalization that survives non-ASCII and malformed input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace nanorouter's regex+decodeURI normalization with the WHATWG URL parser. Path segments are percent-decoded exactly once in the trie, with a keep-raw fallback instead of the URIError that crashed v7 on a literal '%'. Route definitions and locations are NFC-normalized so 'café' matches regardless of composition. Hash-to-slash rewriting now handles every hash, not just the first, and state.href is decoded for humans (raw on failure). Behavior changes from v7: params are no longer double-decoded (%2540 → '%40', not '@'), and unmatchable encodings fall back to raw segments instead of silently 404ing. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014NgfSjHE11oFpoSnLVKLXd --- packages/core/index.js | 12 +++- packages/core/lib/router.js | 98 ++++++++++++++++------------ packages/core/test/url.test.js | 114 +++++++++++++++++++++++++++++++++ 3 files changed, 181 insertions(+), 43 deletions(-) create mode 100644 packages/core/test/url.test.js diff --git a/packages/core/index.js b/packages/core/index.js index 92ee634..2638631 100644 --- a/packages/core/index.js +++ b/packages/core/index.js @@ -14,6 +14,16 @@ import { ok, equal, notEqual } from './lib/assert.js' const HISTORY_OBJECT = {} +// state.href is for reading: decode it for humans ('/caf%C3%A9' → '/café'), +// but never throw on malformed input — keep the raw string instead. +function safeDecode (location) { + try { + return decodeURI(location).normalize('NFC') + } catch (e) { + return location + } +} + export class Choo { constructor (opts) { const timing = nanotiming('choo.constructor') @@ -247,7 +257,7 @@ export class Choo { } const matched = this.router.match(location) this._handler = matched.cb - state.href = location + state.href = safeDecode(location) state.query = nanoquery(queryString) state.route = matched.route state.params = matched.params diff --git a/packages/core/lib/router.js b/packages/core/lib/router.js index 5a0f6eb..a43a2e7 100644 --- a/packages/core/lib/router.js +++ b/packages/core/lib/router.js @@ -1,7 +1,14 @@ // Ported from nanorouter 4.0.0 and wayfarer 7.0.1 with its trie (MIT) // https://github.com/choojs/nanorouter — https://github.com/yoshuawuyts/wayfarer // Consolidated into a single module: trie-based router with :params and -// * wildcards, plus nanorouter's URL normalization on top. +// * wildcards, plus URL normalization on top. +// +// Normalization differs deliberately from v7: locations are parsed with the +// WHATWG URL parser and each path segment is percent-decoded exactly once, +// falling back to the raw segment instead of throwing on malformed input +// (v7's decodeURI crashed on a literal '%'). Both route definitions and +// incoming segments are NFC-normalized so 'café' matches 'café' regardless +// of how the é was composed. import { ok, equal, notEqual } from './assert.js' @@ -9,6 +16,16 @@ function has (object, property) { return Object.prototype.hasOwnProperty.call(object, property) } +// Decode a path segment exactly once; keep the raw segment when it isn't +// valid percent-encoding. Always NFC-normalize. +export function decodeSegment (segment) { + try { + return decodeURIComponent(segment).normalize('NFC') + } catch (e) { + return segment.normalize('NFC') + } +} + class Trie { constructor () { this.trie = { nodes: {} } @@ -40,11 +57,16 @@ class Trie { } trie.name = thisRoute.replace(/^:|^\*/, '') - } else if (!has(trie.nodes, thisRoute)) { - node = { nodes: {} } - trie.nodes[thisRoute] = node } else { - node = trie.nodes[thisRoute] + // Literal segment: store it decoded + NFC-normalized so route + // definitions and incoming URLs compare in one canonical form. + const key = decodeSegment(thisRoute) + if (!has(trie.nodes, key)) { + node = { nodes: {} } + trie.nodes[key] = node + } else { + node = trie.nodes[key] + } } return createNode(index + 1, node) @@ -63,27 +85,23 @@ class Trie { function search (index, trie) { // either there's no match, or we're done searching if (trie === undefined) return undefined - const thisRoute = routes[index] - if (thisRoute === undefined) return trie + if (routes[index] === undefined) return trie + + // Segments arrive percent-encoded from the URL parser; decode each + // exactly once (create() stored literal keys in the same form). + const thisRoute = decodeSegment(routes[index]) if (has(trie.nodes, thisRoute)) { // match regular routes first return search(index + 1, trie.nodes[thisRoute]) } else if (trie.name) { // match named routes - try { - params[trie.name] = decodeURIComponent(thisRoute) - } catch (e) { - return search(index, undefined) - } + params[trie.name] = thisRoute return search(index + 1, trie.nodes.$$) } else if (trie.wildcard) { - // match wildcards - try { - params.wildcard = decodeURIComponent(routes.slice(index).join('/')) - } catch (e) { - return search(index, undefined) - } + // match wildcards; decode per segment so an encoded '/' can't + // change the segment structure mid-decode + params.wildcard = routes.slice(index).map(decodeSegment).join('/') // return early, or else search may keep recursing through the wildcard return trie.nodes.$$ } else { @@ -183,30 +201,26 @@ function wayfarer (dft) { } } -// electron support -const isLocalFile = /file:\/\//.test( - typeof window === 'object' && - window.location && - window.location.origin -) +// Reduce a location (path, full URL, or electron file:// URL) to a +// matchable path: pathname plus any hash segments rewritten to slashes, +// query dropped. Percent-decoding is NOT done here — the trie decodes per +// segment — so a malformed sequence can never throw during routing. +function pathname (routename) { + let url + try { + url = new URL(routename, 'http://localhost') + } catch (e) { + return routename + } -/* eslint-disable no-useless-escape */ -const electron = '^(file:\/\/|\/)(.*\.html?\/?)?' -const protocol = '^(http(s)?(:\/\/))?(www\.)?' -const domain = '[a-zA-Z0-9-_\.]+(:[0-9]{1,5})?(\/{1})?' -const qs = '[\?].*$' -/* eslint-enable no-useless-escape */ + let path = url.pathname + // electron support: file:///path/to/index.html routes as '/' + if (url.protocol === 'file:') { + path = path.replace(/\/[^/]*\.html?$/, '') || '/' + } -const stripElectron = new RegExp(electron) -const prefix = new RegExp(protocol + domain) -const normalize = new RegExp('#') -const suffix = new RegExp(qs) - -// replace everything in a route but the pathname and hash -function pathname (routename, isElectron) { - if (isElectron) routename = routename.replace(stripElectron, '') - else routename = routename.replace(prefix, '') - return decodeURI(routename.replace(suffix, '').replace(normalize, '/')) + const hash = url.hash.replace(/^#/, '/').replaceAll('#', '/') + return path + hash } export default class Nanorouter { @@ -223,13 +237,13 @@ export default class Nanorouter { emit (routename) { equal(typeof routename, 'string') - routename = pathname(routename, isLocalFile) + routename = pathname(routename) return this.router.emit(routename) } match (routename) { equal(typeof routename, 'string') - routename = pathname(routename, isLocalFile) + routename = pathname(routename) return this.router.match(routename) } } diff --git a/packages/core/test/url.test.js b/packages/core/test/url.test.js new file mode 100644 index 0000000..c8bf826 --- /dev/null +++ b/packages/core/test/url.test.js @@ -0,0 +1,114 @@ +// URL normalization: the non-ASCII and malformed-percent cases that made +// v7 routing crash or mismatch. See lib/router.js header for the rules. + +import { test } from 'node:test' +import assert from 'node:assert' + +import html from '@choojs/html' +import choo from '@choojs/core' + +function view (state) { + return html`
` +} + +test('a literal % in the path does not crash routing', () => { + const app = choo() + let seen + app.route('/deals/:label', (state) => { + seen = state + return view(state) + }) + app.toString('/deals/50%off') + assert.strictEqual(seen.params.label, '50%off', 'raw segment kept when not decodable') + assert.strictEqual(seen.href, '/deals/50%off', 'href kept raw when not decodable') +}) + +test('percent-encoded UTF-8 params decode exactly once', () => { + const app = choo() + let seen + app.route('/user/:name', (state) => { + seen = state + return view(state) + }) + app.toString('/user/%F0%9F%9A%82') + assert.strictEqual(seen.params.name, '🚂', 'param decoded') + assert.strictEqual(seen.href, '/user/🚂', 'href decoded for humans') +}) + +test('double-encoded input is not double-decoded', () => { + const app = choo() + let seen + app.route('/user/:name', (state) => { + seen = state + return view(state) + }) + // %2540 is '%40' encoded once; v7 double-decoded it all the way to '@' + app.toString('/user/%2540') + assert.strictEqual(seen.params.name, '%40', 'decoded exactly once') +}) + +test('non-ASCII literal routes match their encoded locations', () => { + const app = choo() + let calls = 0 + app.route('/café', (state) => { + calls++ + return view(state) + }) + app.toString('/caf%C3%A9') + assert.strictEqual(calls, 1, 'encoded location matched unencoded route') +}) + +test('NFD input matches an NFC route definition', () => { + const app = choo() + let calls = 0 + app.route('/café', (state) => { // composed é + calls++ + return view(state) + }) + app.toString('/café') // decomposed e + combining acute + assert.strictEqual(calls, 1, 'unicode-normalized before matching') +}) + +test('unencoded non-ASCII locations route fine', () => { + const app = choo() + let seen + app.route('/user/:name', (state) => { + seen = state + return view(state) + }) + app.toString('/user/日本語') + assert.strictEqual(seen.params.name, '日本語') +}) + +test('wildcards decode per segment', () => { + const app = choo() + let seen + app.route('/files/*', (state) => { + seen = state + return view(state) + }) + app.toString('/files/caf%C3%A9/na%C3%AFve.txt') + assert.strictEqual(seen.params.wildcard, 'café/naïve.txt') +}) + +test('hash routing survives multiple hashes', () => { + const app = choo({ hash: true }) + let calls = 0 + app.route('/docs/api/intro', (state) => { + calls++ + return view(state) + }) + app.toString('/docs#api#intro') + assert.strictEqual(calls, 1, 'every hash became a slash') +}) + +test('query strings decode + and percent-encoding via URLSearchParams', () => { + const app = choo() + let seen + app.route('/', (state) => { + seen = state + return view(state) + }) + app.toString('/?q=caf%C3%A9+au+lait&tags=a&tags=b') + assert.deepStrictEqual(seen.query, { q: 'café au lait', tags: ['a', 'b'] }) +})