fix(scenes): guard scene URL handling against malformed input - #70254
Conversation
Malformed URLs crash scene rendering because sceneLogic decodes and routes them without guarding the input. - PersonScene's paramsToProps called decodeURIComponent on the raw URL id, which throws URIError on a stray `%` (e.g. a distinct id like `50%off`). Use the existing tryDecodeURIComponent helper so it falls back to the raw id instead of crashing the scene. - sceneLogic's urlToAction handlers build redirect URLs and dispatch scene loads synchronously while kea-router matches the route. A malformed or whitespace-containing URL could throw there with nothing to catch it, failing the whole page. Wrap the handlers so a throw is captured and the route falls back to Error404. Generated-By: PostHog Code Task-Id: 6e5d3078-ffbd-4039-becb-607e900c2d42
🤖 CI report
|
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.21 MiB · 22 files | no change | ███░░░░░░░ 28.2% of 4.29 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.10 MiB · 2,974 files | 🔺 +247 B (+0.0%) | █████████░ 87.5% of 9.25 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 668 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 278.2 KiB | ../node_modules/.pnpm/posthog-js@1.399.2/node_modules/posthog-js/dist/rrweb.js |
| 266.9 KiB | ../node_modules/.pnpm/@posthog+icons@0.37.4_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 234.9 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 221.5 KiB | ../node_modules/.pnpm/posthog-js@1.399.2/node_modules/posthog-js/dist/module.js |
| 164.0 KiB | src/queries/validators.js |
| 154.2 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 105.9 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
⚠️ Dist folder size — 🔺 +8.7 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1283.94 MiB · 🔺 +8.7 KiB (+0.0%)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69655d0c92
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kea-router runs decodeURI(pathname) while matching routes, before any scene loads. A stray `%` that isn't a valid escape (e.g. a direct visit or popstate to `/person/50%off`) makes decodeURI throw URIError synchronously inside the router, crashing the whole app before the route handler or PersonScene ever run. The urlToAction handler guard added earlier sits downstream of this throw, so it can't catch it. Add `ensureRoutablePathname` and apply it in `transformPathInActions`, which kea-router runs on the pathname before decodeURI on every navigation (initial load, push/replace, popstate). Escaping a lone `%` keeps the path decodable so routing continues to the matched scene or falls through to 404 instead of throwing. Also tighten the guardRoute catch to use the non-optional `payload.method` directly. Generated-By: PostHog Code Task-Id: 6e5d3078-ffbd-4039-becb-607e900c2d42
|
stamphog does not review bot-authored PRs — removing the |
1 participant
Problem
Malformed URLs crash scene rendering because
sceneLogicdecodes and routes them without guarding the input. Two separate first-observation error tracking crashes trace back to the same weak spot:PersonScene'sparamsToPropscallsdecodeURIComponent(rawUrlId)directly, which throwsURIError: URI malformedon a stray%(think a distinct id like50%off).argument must not contain whitespace) inside the route-mapping arrows that build redirect URLs and dispatchopenScene/loadScene.Both run synchronously inside kea-router's route dispatch, so nothing catches the throw and the page fails to render. Blast radius is small (both are new, low-occurrence issues), but they are real crashes rather than cosmetic glitches.
Why: raised from an inbox report about the two crashes above. The fix hardens the scene-loading path so an oddly-encoded URL degrades gracefully instead of taking down the whole app.
Changes
PersonScene.tsx: decode the URL id with the existingtryDecodeURIComponenthelper fromlib/utils/url, which falls back to the raw id onURIErrorinstead of throwing.sceneLogic.tsx: wrap theurlToActionredirect and scene-dispatch handlers in aguardRoutehelper. If building a redirect URL or dispatching a scene throws, it captures the exception and falls back toError404so the app renders a 404 instead of crashing.Note
The catch-all
/*handler already loadsError404, so it is left unwrapped.No frontend visual changes.
How did you test this code?
Automated tests I (Claude) actually ran:
PersonScene.test.tsx: a parameterized test overscene.paramsToPropscovering a valid encoding (50%25off→50%off), a malformed%that must fall back to the raw id (50%off→50%off), and whitespace passthrough (foo bar). This locks in the decode fallback — it fails if someone reverts to a baredecodeURIComponent. No existing test exercisedPersonScene'sparamsToProps.sceneLogic.test.ts/.tsxsuites (24 tests) — all pass.I did not manually exercise the malformed URLs in a running browser.
Automatic notifications
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Authored by Claude (Claude Code) acting on a PostHog inbox report. I traced both crashes to the scene-routing path in
sceneLogic.tsxand the directdecodeURIComponentinPersonScene.tsx.Decisions along the way:
tryDecodeURIComponenthelper instead of adding a new try/catch, since it already does exactly the raw-id fallback.Error404matches howsceneLogicalready handles malformed scene params elsewhere (setScene's logic-builder catch).sceneLogicguard:guardRouteis a local closure and triggering the real throw needs kea-router internals, which would be a brittle, expensive test. Invoked the/writing-testsskill to make that call.Created with PostHog Code from an inbox report