GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix(router): decode params and match percent-encoded static segments by JosunLP · Pull Request #260 · bQuery/bQuery

Conversation

@JosunLP

@JosunLP JosunLP commented Sep 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • route.params values are now percent-decoded: /user/J%C3%BCrgen%20M gives { name: 'Jürgen M' }. Before this change they came back encoded.
  • Routes whose static text contains non-ASCII or other URL-encoded characters now match, e.g. /über and /a b/:id.
  • resolve() → navigation → route.params round-trips.
  • Param constraints are checked against the decoded value, the same value resolve() validates.

Motivation

Fixes #247. The router matched patterns against the raw, encoded location.pathname and had no decodeURIComponent anywhere. resolve() already encoded params, so named navigation with anything non-ASCII produced encoded params.

Changes included

  • src/router/match.ts:
    • Static route text is encoded the way the WHATWG URL parser encodes a pathname (C0/space/"#<>?\{}/DEL/non-ASCII). The result is cached per pattern. Params, their constraints and *` are left untouched.
    • Percent-escapes in the incoming path are upper-cased, so %c3%bc equals %C3%BC.
    • Captured params are decoded before the constraint test. A malformed escape is kept verbatim instead of failing navigation.
  • tests/router.test.ts: three regression tests covering param decoding, the resolve() round-trip with a constraint, and static non-ASCII / space routes including lower-case escapes.

Validation

  • bunx eslint + bunx prettier --check on changed files
  • bunx tsc --noEmit -p tsconfig.json
  • bun run build (not run)
  • bun test: 3520 pass, 0 fail. The three new tests fail on dev.

Checklist

  • I followed the contributing guidelines.
  • I added or updated tests where behavior changed.
  • I updated documentation when public behavior or developer workflow changed (the docs never described encoded params, so nothing to change).
  • I confirmed the change does not introduce security-sensitive regressions.

Notes for reviewers

  • Behaviour change: an app that worked around this by calling decodeURIComponent(route.params.x) itself will now decode twice. That only matters for values containing a literal %. Worth a line in the release notes.
  • route.path stays the encoded URL path on purpose, since it is a URL.
  • The server router has a similar static-segment issue; it has its own PR for [Bug]: Server routes containing non-ASCII or reserved characters never match (404) #248.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MdbZH1BQLrEGMoMD3gdBBo


Generated by Claude Code

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@claude
fix(router): decode params and match percent-encoded static segments
2ece15b
Matching ran against the raw, percent-encoded pathname and never decoded
anything: params came back encoded (J%C3%BCrgen), a route like /über could
never match (location.pathname reports /%C3%BCber), and resolve() -- which
encodes params -- did not round-trip. Constraints were also tested on the
encoded value during matching but on the decoded one in resolve().

Route statics are now encoded the way the URL parser encodes a pathname,
percent-escapes in the path are case-normalized, and captured params are
decoded (malformed escapes are kept verbatim) before constraint checks.

Fixes #247

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01MdbZH1BQLrEGMoMD3gdBBo
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 3b914e52-adab-4c68-a726-ead9f131efa7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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
  • X
  • Mastodon
  • Reddit
  • LinkedIn

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot added router Changes to the router module tests Chenges to the tests labels Sep 27, 2026
@JosunLP JosunLP mentioned this pull request Sep 27, 2026
fix(server): match static route segments with URL-encoded characters #261
Merged
7 of 8 tasks
@claude
fix(router): normalize raw paths and route statics to the same encoding
e3041b8
Encoding only the route statics broke callers that pass raw paths
(resolveRoute('/über/7'), hash mode with { or }), and escapes typed into
route statics (/caf%c3%a9) were no longer case-normalized. Normalize both
sides with the same URL path encoding + hex upper-casing.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01MdbZH1BQLrEGMoMD3gdBBo
JosunLP added a commit that referenced this pull request Sep 28, 2026
@JosunLP @claude
fix(server): match static route segments with URL-encoded characters (#…
54c4762
…261)

## Summary

- `app.get('/über', …)`, `app.get('/a b/:id', …)` and emoji segments
used to always return 404. They now match.
- Lower-case percent-escapes sent by a client (`/%c3%bcber`) match too.

## Motivation

Fixes #248. `compileRoutePath` built the RegExp from the literal route
string, but matching runs against `ctx.url.pathname`, which the URL
parser always percent-encodes. Param decoding was already correct; only
static segments were affected. File routes are affected the same way,
since they compile through the same function.

## Changes included

- `src/server/create-server.ts`:
- `encodeStaticSegment()` encodes static segments with the URL path
percent-encode set before regex-escaping them.
- `normalizePercentEncoding()` upper-cases escapes in the request path
inside `matchRoute`. `ctx.path` itself is not changed.
- `tests/server.test.ts`: regression test for an umlaut, lower-case
escapes, a space segment combined with a decoded param, an emoji
segment, and a negative case.

## Validation

- [x] `bunx eslint` + `bunx prettier` on changed files
- [x] `bunx tsc --noEmit -p tsconfig.json`
- [ ] `bun run build` (not run)
- [x] `bun test`: 3518 pass, 0 fail. The new test fails on `dev`.

## Checklist

- [x] I followed the contributing guidelines.
- [x] I added or updated tests where behavior changed.
- [x] I updated documentation when public behavior or developer workflow
changed (not needed).
- [x] I confirmed the change does not introduce security-sensitive
regressions. `%` is not in the encode set, so a pre-encoded route like
`/a%20b` still matches literally.

## Notes for reviewers

The client-router counterpart is #260 (#247).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01MdbZH1BQLrEGMoMD3gdBBo

---
_Generated by [Claude
Code](https://claude.ai/code/session_01MdbZH1BQLrEGMoMD3gdBBo)_

---------

Co-authored-by: Claude <[email protected]>
@JosunLP
Merge branch 'dev' into claude/fix-router-percent-decoding
5b74ce6
@JosunLP
JosunLP marked this pull request as ready for review September 28, 2026 00:03
@JosunLP
JosunLP merged commit bc58f93 into dev Sep 28, 2026
18 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@JosunLP
JosunLP deleted the claude/fix-router-percent-decoding branch September 28, 2026 00:03
This was referenced Sep 28, 2026
[Bug]: Client router never percent-decodes paths — params stay encoded and non-ASCII routes never match #247
Closed
release: 1.17.1 #272
Merged
JosunLP added a commit that referenced this pull request Sep 30, 2026
@JosunLP @claude
release: 1.17.1 (#272)
2d48260
## Summary

- Release PR for **1.17.1**. It brings everything on `dev` since
`1.17.0` into `main`.
- Security-and-correctness patch closing the 1.17.0 audit:
  - three security fixes in `server` and `ssr`;
- ten bug fixes across `core`, `router`, `server`, `reactive`, `forms`,
`store`, `component` and `view`.
- No API removals and no module status transitions. A few behaviour
changes are listed under **Notes for reviewers**.

## Motivation

`dev` is 13 PR merges ahead of `main` (#259–#271). Together they close
issues #246–#253, #256 and #258 (all closed) and the three privately
reported vulnerabilities fixed in #269, #270 and #271.

## Changes included

This PR targets `main`, so the diff shows the whole `1.17.0 → 1.17.1`
delta: **48 files, +2007/−100**. It has three commits on top of `dev`.

**Release commit (`d9d20de`)** contains only metadata and documentation:

- **Version:** `package.json` → `1.17.1`. `AGENT.md`, `llms.txt`,
`.github/copilot-instructions.md`, `.cursorrules` and `.clinerules` are
synced; `check:ai-guidance` passes. `SECURITY.md` already lists
`1.17.x`.
- **Changelog:** `CHANGELOG.md` gets a `[1.17.1] - 2026-09-28` entry
with Security / Fixed / Changed sections.
- **Release notes:** new `docs/release-notes/1.17.1.md`, plus a row in
the release-notes index.
- **Pre-existing doc fixes:**
  - The 1.17.0 release notes page was missing from the sidebar.
- Its date and CHANGELOG anchor said `2026-09-21`; the release date is
`2026-09-25`, so the anchor pointed nowhere.
- **Guides aligned with shipped behaviour:**
- `api-core.md` describes `show()`/`hide()`/`toggle()` and camelCase
`css()` as they work after #262/#263.
  - `router.md` documents decoded params (#260).

**Review-fix commits (`61198bc`, `640830a`)** address the findings from
the review on this PR:

- **Session-bound CSRF (#271):**
- Requests without a stored session use the signed CSRF cookie and never
write to the session. Anonymous traffic can no longer fill the capped
session store or evict real sessions from it.
- A session created during such a request adopts the cookie secret, so
forms rendered earlier still verify.
- A session created or regenerated with `$regenerate()` (a login) always
gets a fresh secret, so a planted cookie cannot reach the authenticated
session.
  - A session removed with `$destroy()` is not written back.
- For a stored session the secret is bound before `next()`, so a token
read later from a streamed body or `ctx.sse()` stays valid.
- **`toggle()` (#263):** `hidden="until-found"` counts as hidden even
with an inline `display`, because it hides through `content-visibility`.
The `api-core.md` list entry now names the inline-`display` exception.

Source changes by PR:

| PR | Change |
| --- | --- |
| #269 | Security: `serveStatic` `realpath` containment for
precompressed sidecars |
| #270 | Security: Node adapter answers handler errors with `500`
instead of crashing; cancels streams and aborts `request.signal` on
disconnect |
| #271 | Security: signed CSRF tokens bound to the session and rotated
on `$regenerate()` (refined in `61198bc`/`640830a`) |
| #259 | `deepEqual`/`isEqual` kind mismatches (#246) |
| #260 | Router: decoded params, encoded static segments (#247) |
| #261 | Server: encoded static route segments (#248) |
| #262 | `css()` camelCase (#249) |
| #263 | `toggle()`/`hide()`/`show()` (#250) |
| #264 | Already-aborted `AbortSignal` in `http` and `listen()` (#251) |
| #265 | Linear-time `email()` (#252) |
| #266 | Trusted Types for component render, `createTemplate()` and the
DOM sanitizer (#253) |
| #267 | `memoryStore()` sweeping, LRU eviction, default cap (#256) |
| #268 | `SECURITY.md` supported line and drift check (#258) |

## Validation

- [x] `bunx eslint .` + `bun run format:check`
- [x] `bun run build`, `check:publish`, `check:size`, `check:package`
- [x] `bun test`: 3568 pass, 0 fail; `test:types` and `lint:types` clean
- [x] `bun run test:coverage` (coverage floor), `bun run check` (incl.
`check:ai-guidance` → 1.17.1) and `bun run docs:build` (no dead links)
- [x] Each regression test added in the review-fix commits fails without
its fix.

## Checklist

- [x] I followed the contributing guidelines.
- [x] I added or updated tests where behavior changed. They were added
in the individual fix PRs and in the review-fix commits.
- [x] I updated documentation when public behavior or developer workflow
changed.
- [x] I confirmed the change does not introduce security-sensitive
regressions.

## Notes for reviewers

**Behaviour changes when upgrading** (also in the CHANGELOG and release
notes):

- **Router params are decoded.** Remove any manual
`decodeURIComponent()`.
- **CSRF with `session()` before `csrf({ secret })`:**
- requests with a stored session get no `bq.csrf` cookie; the secret
lives in the session;
  - visitors without a session keep the signed `bq.csrf` cookie;
- the token rotates on `$regenerate()`, so render a fresh
`csrfToken(ctx)` after login;
  - `bindToSession: false` opts out.
- **`listen({ signal })`** rejects when the signal is aborted before or
during binding.
- **The default session store** evicts beyond 10 000 live sessions.
- **`show()` without an argument** keeps the inline `display` of an
element that is not inline-hidden.
- **`email()`** rejects empty domain labels and addresses over 254
characters.

**Known limitation:** forms rendered to visitors without a session (for
example the login form) rely on the signed cookie, as before 1.17.1. The
server guide recommends a `__Host-` cookie name for that case.

**After merging:** tag `v1.17.1`, publish, and merge `main` back into
`dev`, as after 1.17.0. The review-fix commits exist only on this
branch, so that merge back is required. Consider publishing a GitHub
Security Advisory for ≤ 1.17.0 that covers #269–#271.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01MdbZH1BQLrEGMoMD3gdBBo

---
_Generated by [Claude
Code](https://claude.ai/code/session_01MdbZH1BQLrEGMoMD3gdBBo)_

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved protection against static-file sidecar paths escaping the
configured root and strengthened CSRF protection for session-backed
requests.
* Node server handlers now handle errors and client disconnects more
safely; server startup also responds correctly to cancelled requests.
* Fixed URL-encoded route matching, CSS property handling, visibility
toggling, equality checks, HTTP cancellation, and email validation.
* Expired sessions are cleaned up, and the default in-memory session
store now limits its capacity.

* **Documentation**
* Added version 1.17.1 release notes and updated security guidance and
feature documentation.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

router Changes to the router module tests Chenges to the tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@JosunLP @claude
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.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.