-
Notifications
You must be signed in to change notification settings - Fork 138
fix(web): Fix error when navigating to paths with percent symbols #485
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdjusts URL-decoding responsibilities in browse path parsing: page stops pre-decoding the catch-all path; utils now decodes the repo/revision segment. Adds tests covering percent-encoded characters (%) in both repo/revision and blob path. Updates changelog to document the fix. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant R as Next.js Route (page.tsx)
participant P as getBrowseParamsFromPathParam (utils.ts)
participant V as View (Code/Tree Panel)
U->>R: Navigate to /browse/...@REV/-/(blob|tree)/<encoded path>
note right of R: _No pre-decoding of catch-all path_
R->>P: parse(_rawPath.join('/')) (encoded)
note over P: decode repo/revision segment<br/>interpret path & pathType
P-->>R: { repoName, revisionName, path, pathType }
alt pathType == "blob"
R->>V: Render CodePreviewPanel
else pathType == "tree"
R->>V: Render TreePreviewPanel
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
This comment has been minimized.
This comment has been minimized.
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
CHANGELOG.md (1)
12-12
: Clarify the note (tiny wording nit).Consider “percent symbols (%)” and reference the issue for quick traceability.
-- Fix error when navigating to paths with percentage symbols. [#485](https://github.com/sourcebot-dev/sourcebot/pull/485) +- Fix SSR error when navigating to paths with percent symbols (%). [#485](https://github.com/sourcebot-dev/sourcebot/pull/485), fixes #482packages/web/src/app/[domain]/browse/hooks/utils.ts (2)
8-8
: Harden decoding to avoid URIError on malformed “%” sequences.If the router or upstream already decoded segments (leaving literal “%”),
decodeURIComponent
can throw and crash SSR. Use a safe decoder and apply it consistently here (and to the tree/blob path below).Apply within this hunk:
- const repoAndRevisionPart = decodeURIComponent(pathParam.substring(0, sentinelIndex)); + const repoAndRevisionPart = safeDecodeURIComponent(pathParam.substring(0, sentinelIndex));Also add (outside this hunk) and use for the tree/blob path:
// place near top of file const safeDecodeURIComponent = (s: string) => { try { return decodeURIComponent(s) } catch { return s } }; // replace decodeURIComponent(...) in both 'tree' and 'blob' cases with safeDecodeURIComponent(...)And add a test that passes an already-decoded percent path:
it('should handle already-decoded % in blob path', () => { const r = getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/-/blob/%hello%/world.c'); expect(r).toEqual({ repoName: 'github.com/sourcebot-dev/zoekt', revisionName: 'HEAD', path: '%hello%/world.c', pathType: 'blob', }); });
18-18
: Comment typo.“decodedURIComponent” → “decodeURIComponent”; “incase” → “in case”.
packages/web/src/app/[domain]/browse/hooks/utils.test.ts (1)
102-110
: Tests cover the core regressions—add one more for already-decoded “%”.The new cases are solid. Add a case where the blob path contains raw “%” to ensure SSR doesn’t blow up if upstream hands us decoded segments (pairs well with a safe decoder).
Example:
it('should not throw with raw % in path', () => { const r = getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/-/blob/%hello%/world.c'); expect(r.path).toBe('%hello%/world.c'); });Also applies to: 112-120
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
CHANGELOG.md
(1 hunks)packages/web/src/app/[domain]/browse/[...path]/page.tsx
(1 hunks)packages/web/src/app/[domain]/browse/hooks/utils.test.ts
(1 hunks)packages/web/src/app/[domain]/browse/hooks/utils.ts
(1 hunks)
🔇 Additional comments (1)
packages/web/src/app/[domain]/browse/[...path]/page.tsx (1)
22-22
: Good move—avoid premature decoding at the page level.Letting utils handle targeted decoding prevents double-decoding and URIError on literal “%”.
Fixes #482
Summary by CodeRabbit
Bug Fixes
Documentation
Tests