Skip to content

fix(web): show 404 for invalid browse paths - #1546

Open
brendan-kellam wants to merge 3 commits into
mainfrom
brendan-kellam/fix-SOU-1585
Open

fix(web): show 404 for invalid browse paths#1546
brendan-kellam wants to merge 3 commits into
mainfrom
brendan-kellam/fix-SOU-1585

Conversation

@brendan-kellam

@brendan-kellam brendan-kellam commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Fixes SOU-1585
Fixes WEB-8

Summary

  • treat malformed browse paths as invalid parser results instead of unhandled exceptions
  • validate catch-all browse params in a Server Component and route failures through the existing Next.js 404 page
  • pass validated browse params to client components through context
  • handle malformed URL encoding and reject partial browse type matches

Testing

  • yarn workspace @sourcebot/web lint (changed files)
  • yarn workspace @sourcebot/web test --run (95 files, 1111 tests)

Note

Low Risk
Browse URL parsing and Next.js layout routing only; no auth, data, or API contract changes beyond returning 404 for bad paths.

Overview
Invalid browse URLs now hit the normal 404 flow instead of surfacing as server errors.

Parser behavior: getBrowseParamsFromPathParam returns null for malformed paths (missing /-/, empty blob path, missing commit SHA, bad URL encoding, partial type prefixes like treehouse) instead of throwing. Decoding is wrapped so invalid % sequences are treated as invalid paths.

Routing/layout: Browse chrome is tied to [...path]/layout.tsx, which parses the path on the server and calls notFound() when parsing fails. The browse page does the same guard. Parsed params are passed into LayoutClient and exposed via BrowseParamsContext, so client hooks no longer re-parse the pathname (which could throw during invalid navigations).

Tests and changelog were updated for the new null-based invalid-path handling and commit/history URL cases.

Reviewed by Cursor Bugbot for commit a8d54d0. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Bug Fixes

    • Invalid browse URLs now display a standard 404 page instead of a service error.
    • Malformed paths, missing repository names, missing commit references, and invalid URL encoding are handled safely.
    • Valid commit and file browsing paths continue to work as expected.
  • Tests

    • Expanded coverage for valid commit paths and a range of invalid browse paths.
  • Documentation

    • Added an unreleased changelog entry for the improved invalid-path handling.

@github-actions

This comment has been minimized.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Invalid browse paths now return the 404 page instead of a service error. The parser returns null for malformed paths, and browse layouts pass valid parameters through context. Tests cover valid history paths and additional invalid inputs.

Changes

Browse path validation

Layer / File(s) Summary
Parser contract and validation
packages/web/src/app/(app)/browse/hooks/utils.ts, packages/web/src/app/(app)/browse/hooks/utils.test.ts
The parser safely decodes components, validates required values, returns null for malformed paths, and covers valid commit paths and invalid inputs.
Browse route resolution
packages/web/src/app/(app)/browse/[...path]/layout.tsx, packages/web/src/app/(app)/browse/[...path]/page.tsx, CHANGELOG.md
The route layout and page call notFound() for invalid browse paths. Metadata uses Browse when parsing fails. The changelog records the fix.
Browse parameter context wiring
packages/web/src/app/(app)/browse/hooks/useBrowseParams.ts, packages/web/src/app/(app)/browse/layoutClient.tsx
LayoutClient provides parsed BrowseProps through BrowseParamsContext. useBrowseParams reads the context and throws when no provider exists.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: invalid browse paths now show a 404 page.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch brendan-kellam/fix-SOU-1585

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/web/src/app/`(app)/browse/[...path]/layout.tsx:
- Around line 17-18: Update the route handling around
getBrowseParamsFromPathParam so Next.js-decoded catch-all segments are not
decoded a second time. Use an entry point that accepts already-decoded path
data, or re-encode segments before joining them, while preserving literal
percent characters and encoded slash components.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 31c17ca9-177d-451e-88e2-6d76b210749a

📥 Commits

Reviewing files that changed from the base of the PR and between 0012be8 and a8d54d0.

📒 Files selected for processing (4)
  • packages/web/src/app/(app)/browse/[...path]/layout.tsx
  • packages/web/src/app/(app)/browse/hooks/useBrowseParams.ts
  • packages/web/src/app/(app)/browse/layout.tsx
  • packages/web/src/app/(app)/browse/layoutClient.tsx
💤 Files with no reviewable changes (1)
  • packages/web/src/app/(app)/browse/layout.tsx

Comment on lines +17 to +18
const { path } = await params;
const browseParams = getBrowseParamsFromPathParam(path.join('/'));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Repo files matching layout/params:"
git ls-files | rg '(^|/)layout\.tsx$|getBrowseParamsFromPathParam|browseParams|routeMatchers|route-matcher' || true

echo
echo "Target file excerpt:"
if [ -f packages/web/src/app/'(app)'/browse/[...path]/layout.tsx ]; then
  cat -n 'packages/web/src/app/(app)/browse/[...path]/layout.tsx'
else
  fd -a 'layout.tsx' packages/web/src/app | sed -n '1,80p'
fi

echo
echo "Find getBrowseParamsFromPathParam definitions/usages:"
rg -n "getBrowseParamsFromPathParam|BrowseParams|decodeURIComponent|encodeURIComponent|notFound\(" packages/web/src -S || true

echo
echo "Next.js version declarations:"
for f in package.json packages/web/package.json pnpm-lock.yaml package-lock.json yarn.lock bun.lockb; do
  [ -f "$f" ] && { echo "--- $f"; sed -n '1,180p' "$f" | rg -n '"next"|"`@next/`"|next@|next:' || true; }
done

Repository: sourcebot-dev/sourcebot

Length of output: 24523


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Source hook utils:"
cat -n 'packages/web/src/app/(app)/browse/hooks/utils.ts' | sed -n '1,120p'

echo
echo "Tests for decoding-related cases:"
cat -n 'packages/web/src/app/(app)/browse/hooks/utils.test.ts' | sed -n '1,130p'

echo
echo "Inspect Next.js route matcher from declared lockfile / source if available:"
if [ -f node_modules/next/package.json ]; then
  node - <<'JS'
const p = require('/app/node_modules/next/package.json');
console.log(JSON.stringify({next: p.version}, null, 2));
JS
  rg -n "decodeURIComponent|route-matcher|routeMatcher" node_modules/next -S -m 20 || true
else
  echo "node_modules/next/package.json not present"
fi

echo
echo "Try NPM registry source bundle for declared next patch version:"
tmp="$(mktemp -d)"
cd "$tmp"
set +e
npm pack next@16.2.6 --json 2>/dev/null | jq -r '.[0].filename' 2>/dev/null || true
tarball="$(npm pack next@16.2.6 2>/dev/null | tail -n 1)"
if [ -n "$tarball" ] && [ -f "$tarball" ]; then
  tar -xzf "$tarball"
  echo "--- package files matching route matcher/decode:"
  find package -type f | rg 'path/dist/shared/lib/router/utils/route-matcher|route-matcher|decodeURIComponent' || true
  echo "--- source contains decodeURIComponent:"
  rg -n "decodeURIComponent|decode =? function|decodeURIComponentOnce|decodeURI" package -S -m 40 || true
fi

Repository: sourcebot-dev/sourcebot

Length of output: 12150


🌐 Web query:

Next.js 16.2.11 routeMatcher parameter decodeURIComponent repeated catch-all route parameters source route-matcher

💡 Result:

In Next.js, including version 16.2.11, the route matcher process extracts dynamic route parameters from the URL path. These parameters are URL-encoded by nature when they originate from the request URL. To ensure that parameters are usable as expected (e.g., converting %20 back to a space), the framework applies decodeURIComponent to these matched segments. When handling catch-all routes (e.g., [...slug]), the framework processes each individual element of the catch-all array by applying decoding [1]. A known technical challenge in this process is the risk of double-encoding or decoding errors, particularly when a path segment is already decoded or contains characters that might cause decodeURIComponent to throw an error (such as malformed percent-encoded sequences or already decoded path delimiters) [2]. To handle these cases robustly, the standard implementation pattern in Next.js—often observed in its routing and middleware utilities—involves wrapping the decoding process in a try-catch block [3][4]: try { return decodeURIComponent(param) } catch { // If decoding fails (e.g., param is already decoded or contains invalid sequences), // return the parameter as-is to preserve the original value. return param } This defensive approach prevents application crashes due to malformed URI components while ensuring that properly encoded parameters are correctly normalized for use in the application, such as in router.query or useParams hooks [1]. Ongoing refinements in the Next.js routing layer continue to address edge cases where path delimiters (like slashes or encoded equivalents) interact with this decoding logic, especially regarding middleware behavior and base path rewrites [5][6].

Citations:


🌐 Web query:

github vercel next.js 16.2.6 packages next src shared lib router utils route matcher decodeURIComponent params

💡 Result:

In Next.js, the route-matcher utility is located in packages/next/src/shared/lib/router/utils/route-matcher.ts [1][2]. This utility is responsible for matching request paths against dynamic routes and extracting parameters [3][2]. The getRouteMatcher function inside this file defines a decode helper that uses decodeURIComponent to process path parameters captured by regular expressions [2]. When a dynamic route is matched, the matcher iterates through the identified groups (parameters) and applies this decode function to each value [2]. If a parameter is marked as a repeatable segment (using rest parameters like [...slug]), it splits the string by / and decodes each part individually [2]. Historically, this area of the codebase has been sensitive to how parameters are encoded, leading to issues such as double-decoding or failure when encountering special characters like % [4][5][6]. The implementation specifically includes a try-catch block around decodeURIComponent to handle scenarios where the input might be invalid, throwing a custom DecodeError: failed to decode param if decoding fails [2]. Next.js version 16.2.6, released in May 2026, was a significant security release that addressed multiple vulnerabilities, including those related to middleware/proxy bypass through dynamic route parameter injection (e.g., GHSA-492v-c6pp-mqqv) [7][8][9]. While version 16.2.6 does not focus solely on route-matcher, the routing and parameter handling logic remain foundational components of the App Router and Pages Router functionality within the next package [1][3].

Citations:


🌐 Web query:

github vercel next.js v16.2.11 routeMatcher.ts decodeURIComponent

💡 Result:

In Next.js, particularly within files like routeMatcher.ts, the use of decodeURIComponent is a frequent point of interaction for handling URL path segments [1][2][3]. The handling of decodeURIComponent in the Next.js router has evolved to address several critical issues: 1. Handling Non-ASCII Characters: Earlier updates introduced logic in route-matcher.ts to attempt decoding of pathnames to properly match route definitions, allowing for non-ASCII path names (e.g., Unicode characters) that might otherwise return 404 errors [1]. 2. Robustness Against Invalid Sequences: Next.js has implemented specific error handling for cases where a URL segment contains invalid percent-encoded sequences (e.g., %A0). Instead of allowing a URIError to propagate and cause a 500 server error, the router catches these cases and correctly returns a 404 Not Found status [2]. 3. Preventing Encoding Errors: There has been historical work to ensure that parameters are not double-encoded or improperly decoded. Developers have noted that applying decodeURIComponent directly can fail if the input is already partially decoded or contains characters like %25, leading to recommendations for safe, try-catch based approaches when manually handling these values in user-land code [3][4]. Regarding Next.js version 16.2.11, released on July 21, 2026, the primary focus was on addressing several high-severity security vulnerabilities, including issues related to Server Actions, Middleware, and SSRF [5][6][7]. While the core router files like routeMatcher.ts continue to refine how they handle URI decoding to maintain stability and security, version 16.2.11 is specifically characterized by these security patches rather than a direct, major overhaul of the route matcher's decoding logic [5][8][9].

Citations:


🌐 Web query:

raw.githubusercontent.com vercel/next.js v16.2.11 packages next src shared lib router utils route-matcher.ts decodeURIComponent decode function

💡 Result:

In the Next.js source code (such as in v16.2.11), the getRouteMatcher function located in packages/next/src/shared/lib/router/utils/route-matcher.ts uses the decodeURIComponent function to decode dynamic route parameters [1]. When a path is matched against a route, the getRouteMatcher creates a helper decode function that wraps decodeURIComponent in a try...catch block [1]. If decodeURIComponent fails (for example, due to a malformed percent-encoded sequence), it throws a DecodeError (custom error) with the message "failed to decode param" [1][2]. This mechanism ensures that captured dynamic segments—whether they are single parameters or elements of a catch-all route—are properly URL-decoded before being processed by the application [1][3]. If decoding fails, recent versions of Next.js are designed to handle the error, typically resulting in a 404 response rather than allowing the error to propagate as a 500 server error [2][4].

Citations:


🌐 Web query:

raw.githubusercontent.com vercel/next.js v16.2.6 packages next src shared lib router utils route-matcher.ts decodeURIComponent decode function

💡 Result:

In Next.js version 16.2.6, the route matcher logic found in packages/next/src/shared/lib/router/utils/route-matcher.ts uses decodeURIComponent to process URL path segments when matching dynamic routes [1][2]. The use of decodeURIComponent in this utility has historically been a source of errors when URLs contain invalid percent-encoded sequences (e.g., %A0) or when paths are double-encoded [3][4][5]. When decodeURIComponent encounters a malformed URI sequence, it throws a URIError. Within the Next.js router, this error is typically wrapped or handled to ensure the application does not crash with a 500 error [2][6]. Recent improvements and fixes in the Next.js routing infrastructure—specifically around the 16.x release cycle—have focused on making this decoding process more resilient [2][7][6]. Notably, if a segment cannot be decoded, modern versions of the router are designed to catch the resulting DecodeError and treat it as a non-match (resulting in a 404 Not Found response) rather than allowing the error to propagate or trigger an incorrect status code [2][6]. This prevents issues where invalid encoding in a URL segment could inadvertently cause server-side exceptions [2][6].

Citations:


🌐 Web query:

raw.githubusercontent.com vercel/next.js v16.2.6 packages/next/src/shared/lib/router/utils/route-matcher.ts "for (const segment of restSegments)" decodeURIComponent

💡 Result:

In Next.js v16.2.6, the file located at packages/next/src/shared/lib/router/utils/route-matcher.ts is responsible for matching URL path segments against defined routes [1][2]. The use of decodeURIComponent within this module—specifically when iterating over path segments—is a critical part of how Next.js handles dynamic route parameters [2]. Historically, this logic has been a focus for reliability improvements to prevent unhandled errors and handle edge cases in URI decoding [3][4][5]. Key technical context for this implementation includes: 1. Handling Invalid Encoding: When a URL segment contains a percent-encoded sequence that cannot be decoded (e.g., %A0), decodeURIComponent throws a URIError [3]. In modern versions of Next.js, this is often wrapped or handled to ensure the framework returns a 404 Not Found instead of a 500 error, as an un-decodable segment is treated as a non-match for the route [3]. 2. Preventing Double Decoding and Errors: Next.js has evolved to ensure that parameters are decoded safely [6][2]. Because the router might encounter segments that have already been processed, careless use of decodeURIComponent can lead to errors if it attempts to decode an already-decoded string or handles malformed input [6][5]. The implementation typically involves try-catch blocks around decodeURIComponent to allow the router to fall back gracefully if a segment is invalid or already decoded [6]. 3. Support for Special Characters: Using decodeURIComponent allows dynamic route parameters (like [slug]) to correctly capture and provide literal values for encoded characters (e.g., converting %20 to a space or %2F to a slash where appropriate), ensuring consistent behavior across different router configurations [2]. For exact source code visualization, you can view the repository at github.com/vercel/next.js/tree/v16.2.6, though direct access to internal source files via raw.githubusercontent.com URLs is subject to GitHub's raw file serving policies and specific repository branch structures [7][8].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

tmp="$(mktemp -d || true)"
if [ -n "${tmp:-}" ]; then
  echo "Checking next@16.2.6 package metadata/source fetch:"
  (set +e; npm view next@16.2.6 version dist.tarball dist.integrity 2>&1)
  tarball="$(npm pack next@16.2.6 2>/dev/null | tail -n 1)"
  if [ -n "${tarball:-}" ] && [ -f "$tarball" ]; then
    tar -xzf "$tarball"
    echo "--- route-matcher matches:"
    grep -RIn "decodeURIComponent|restSegments|for \\(const segment of restSegments\\)" package/dist package/src 2>/dev/null | head -80 || true
    echo "--- inspect candidate source:"
    for f in package/src/shared/lib/router/utils/route-matcher.ts package/dist/server/route-matchers/next.js package/dist/esm/shared/lib/router/utils/route-matcher.js; do
      if [ -f "$f" ]; then
        echo "### $f"
        sed -n '1,180p' "$f"
      fi
    done
    echo "--- inspect compiled route-matchers if present:"
    f="package/dist/server/route-matchers/route-matcher.js"
    if [ -f "$f" ]; then
      sed -n '1,220p' "$f"
    fi
  fi
fi

echo
echo "Programmatic probe of getBrowseParamsFromPathParam decoding behavior for route-param decoded shapes:"
node - <<'JS'
function decodeBrowsePathPart(pathPart) {
  try {
    return decodeURIComponent(pathPart);
  } catch {
    return null;
  }
}

function getDecodeShapeFor(urlOrEncoded) {
  // Simulate Next.js catch-all repeated segments decoded before rejoining plus slash normalization.
  // This is not full Next router matching; it only shows the decoded string the parser receives
  // for the stated examples in the review.
  if (urlOrEncoded.includes('/-/')) return urlOrEncoded;
  const match = urlOrEncoded.match(/^([^/]+)\/([^/]+)\/-(\/[a-zA-Z0-9-_\/]+)$/);
  if (!match) return null;
  const repoAndRevisionPart = decodeURIComponent(match[1]) + '/' + decodeURIComponent(match[2]) || '';
  const pathPart = '/' + decodeURIComponent(match[3]).replace(/\/+/g, '/').replace(/^\/+/, '');
  return repoAndRevisionPart + '/-/tree' + pathPart;
}

const tests = [
  '100%.txt',
  '/-/tree/100%.txt',
  'github.com/repo@HEAD/-/tree/100%.txt',
  'github.com/repo@HEAD/-/blob/%25hello%25%2Fworld.c',
];
for (const input of tests) {
  const shape = getDecodeShapeFor(input);
  console.log(JSON.stringify({ input, shape, parserParsed: getBrowseParamsFromPathParam(shape ? shape : input) })
    // Since getBrowseParamsFromPathParam is not exported to Node, just log parsing decision point inputs instead.
  );
}
JS

Repository: sourcebot-dev/sourcebot

Length of output: 4620


Preserve URL encoding at the Next.js route boundary.

Next.js decodes each catch-all route segment before returning params.path, but getBrowseParamsFromPathParam() decodes the joined string again. A path such as 100%.txt can then fail decodeURIComponent(), and legitimate %2F path components are decoded twice. Add an already-decoded parser entry point or encode before joining route segments for this layout/page path.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/web/src/app/`(app)/browse/[...path]/layout.tsx around lines 17 - 18,
Update the route handling around getBrowseParamsFromPathParam so Next.js-decoded
catch-all segments are not decoded a second time. Use an entry point that
accepts already-decoded path data, or re-encode segments before joining them,
while preserving literal percent characters and encoded slash components.

Source: MCP tools

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.

1 participant