diff --git a/CHANGELOG.md b/CHANGELOG.md index 504bff969..60b791069 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Upgraded `socket.io-parser` to `^4.2.7`. [#1542](https://github.com/sourcebot-dev/sourcebot/pull/1542) - Upgraded `fast-uri` to `^3.1.5`. [#1541](https://github.com/sourcebot-dev/sourcebot/pull/1541) - Upgraded `ip-address` to `^10.4.0`. [#1540](https://github.com/sourcebot-dev/sourcebot/pull/1540) +- Fixed invalid browse paths returning service errors instead of the 404 page. [#1546](https://github.com/sourcebot-dev/sourcebot/pull/1546) ## [5.1.5] - 2026-07-31 diff --git a/packages/web/src/app/(app)/browse/[...path]/layout.tsx b/packages/web/src/app/(app)/browse/[...path]/layout.tsx new file mode 100644 index 000000000..38fe246f8 --- /dev/null +++ b/packages/web/src/app/(app)/browse/[...path]/layout.tsx @@ -0,0 +1,32 @@ +import { getConfiguredLanguageModelsInfo } from "@/features/chat/utils.server"; +import { notFound } from "next/navigation"; +import { getBrowseParamsFromPathParam } from "../hooks/utils"; +import { LayoutClient } from "../layoutClient"; + +interface LayoutProps { + children: React.ReactNode; + params: Promise<{ + path: string[]; + }>; +} + +export default async function Layout({ + children, + params, +}: LayoutProps) { + const { path } = await params; + const browseParams = getBrowseParamsFromPathParam(path.join('/')); + if (!browseParams) { + notFound(); + } + + const languageModels = await getConfiguredLanguageModelsInfo(); + return ( + 0} + > + {children} + + ) +} diff --git a/packages/web/src/app/(app)/browse/[...path]/page.tsx b/packages/web/src/app/(app)/browse/[...path]/page.tsx index a051e5bb2..5ca466670 100644 --- a/packages/web/src/app/(app)/browse/[...path]/page.tsx +++ b/packages/web/src/app/(app)/browse/[...path]/page.tsx @@ -7,6 +7,7 @@ import { CommitsPanel } from "./components/commitHistoryPanel/commitsPanel"; import { Loader2 } from "lucide-react"; import { TreePreviewPanel } from "./components/treePreviewPanel/treePreviewPanel"; import { Metadata } from "next"; +import { notFound } from "next/navigation"; import { TrackRepoVisit } from "./components/trackRepoVisit"; import { auth } from "@/auth"; @@ -24,6 +25,10 @@ const parsePathForTitle = (path: string[]): string => { const pathParam = path.join('/'); const browseProps = getBrowseParamsFromPathParam(pathParam); + if (!browseProps) { + return 'Browse'; + } + const { repoName, revisionName, path: filePath } = browseProps; // Build the base repository and revision string. @@ -104,6 +109,10 @@ export default async function BrowsePage(props: BrowsePageProps) { const rawPath = _rawPath.join('/'); const browseProps = getBrowseParamsFromPathParam(rawPath); + if (!browseProps) { + notFound(); + } + const { repoName, revisionName, path } = browseProps; const page = Math.max(1, parseInt(searchParams.page ?? '1', 10) || 1); @@ -166,4 +175,3 @@ export default async function BrowsePage(props: BrowsePageProps) { ) } - diff --git a/packages/web/src/app/(app)/browse/hooks/useBrowseParams.ts b/packages/web/src/app/(app)/browse/hooks/useBrowseParams.ts index d7917f722..39bd9d442 100644 --- a/packages/web/src/app/(app)/browse/hooks/useBrowseParams.ts +++ b/packages/web/src/app/(app)/browse/hooks/useBrowseParams.ts @@ -1,18 +1,13 @@ -import { usePathname } from "next/navigation"; -import { useMemo } from "react"; -import { getBrowseParamsFromPathParam } from "./utils"; +import { createContext, useContext } from "react"; +import type { BrowseProps } from "./utils"; -export const useBrowseParams = () => { - const pathname = usePathname(); +export const BrowseParamsContext = createContext(null); - return useMemo(() => { - const startIndex = pathname.indexOf('/browse/'); - if (startIndex === -1) { - throw new Error(`Invalid browse pathname: "${pathname}" - expected to contain "/browse/"`); - } +export const useBrowseParams = () => { + const browseParams = useContext(BrowseParamsContext); + if (!browseParams) { + throw new Error('useBrowseParams must be used within a BrowseParamsContext provider'); + } - const rawPath = pathname.substring(startIndex + '/browse/'.length); - return getBrowseParamsFromPathParam(rawPath); - }, [pathname]); + return browseParams; } - diff --git a/packages/web/src/app/(app)/browse/hooks/utils.test.ts b/packages/web/src/app/(app)/browse/hooks/utils.test.ts index a50214c82..1164125da 100644 --- a/packages/web/src/app/(app)/browse/hooks/utils.test.ts +++ b/packages/web/src/app/(app)/browse/hooks/utils.test.ts @@ -152,6 +152,29 @@ describe('getBrowseParamsFromPathParam', () => { }); }); + describe('history paths', () => { + it('should parse a commits path', () => { + const result = getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/-/commits/packages/web'); + expect(result).toEqual({ + repoName: 'github.com/sourcebot-dev/zoekt', + revisionName: 'HEAD', + path: 'packages/web', + pathType: 'commits', + }); + }); + + it('should parse a commit path', () => { + const result = getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt/-/commit/a1b2c3d'); + expect(result).toEqual({ + repoName: 'github.com/sourcebot-dev/zoekt', + revisionName: undefined, + path: '', + pathType: 'commit', + commitSha: 'a1b2c3d', + }); + }); + }); + describe('edge cases', () => { it('should handle repo name with multiple @ symbols', () => { const result = getBrowseParamsFromPathParam('gitlab.com/user@domain/repo@main/-/tree/'); @@ -175,40 +198,44 @@ describe('getBrowseParamsFromPathParam', () => { }); describe('error cases', () => { - it('should throw error for blob path with trailing slash and no path', () => { - expect(() => { - getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/-/blob/'); - }).toThrow(); + it('should return null for blob path with trailing slash and no path', () => { + expect(getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/-/blob/')).toBeNull(); + }); + + it('should return null for blob path without trailing slash and no path', () => { + expect(getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/-/blob')).toBeNull(); + }); + + it('should return null for invalid pattern - missing /-/', () => { + expect(getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/tree/')).toBeNull(); + }); + + it('should return null for invalid pattern - missing tree/blob', () => { + expect(getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/-/invalid/')).toBeNull(); + }); + + it('should return null for completely invalid format', () => { + expect(getBrowseParamsFromPathParam('invalid-path')).toBeNull(); }); - it('should throw error for blob path without trailing slash and no path', () => { - expect(() => { - getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/-/blob'); - }).toThrow(); + it('should return null for empty string', () => { + expect(getBrowseParamsFromPathParam('')).toBeNull(); }); - it('should throw error for invalid pattern - missing /-/', () => { - expect(() => { - getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/tree/'); - }).toThrow(); + it('should return null for an empty repository name', () => { + expect(getBrowseParamsFromPathParam('/-/tree')).toBeNull(); }); - it('should throw error for invalid pattern - missing tree/blob', () => { - expect(() => { - getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/-/invalid/'); - }).toThrow(); + it('should return null for a commit path without a SHA', () => { + expect(getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt/-/commit')).toBeNull(); }); - it('should throw error for completely invalid format', () => { - expect(() => { - getBrowseParamsFromPathParam('invalid-path'); - }).toThrow(); + it('should return null for invalid URL encoding', () => { + expect(getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/-/blob/%')).toBeNull(); }); - it('should throw error for empty string', () => { - expect(() => { - getBrowseParamsFromPathParam(''); - }).toThrow(); + it('should return null when the browse type is only a prefix', () => { + expect(getBrowseParamsFromPathParam('github.com/sourcebot-dev/zoekt@HEAD/-/treehouse')).toBeNull(); }); }); -}); \ No newline at end of file +}); diff --git a/packages/web/src/app/(app)/browse/hooks/utils.ts b/packages/web/src/app/(app)/browse/hooks/utils.ts index 7ea1863c6..6a2b2eb8c 100644 --- a/packages/web/src/app/(app)/browse/hooks/utils.ts +++ b/packages/web/src/app/(app)/browse/hooks/utils.ts @@ -59,65 +59,68 @@ export type BrowsePathType = BrowseProps['pathType']; // both map to the empty path. const normalizeRepoPath = (path: string): string => path.replace(/^\/+/, ''); -export const getBrowseParamsFromPathParam = (pathParam: string): BrowseProps => { - // @note: order matters — `commits` must come before `commit` so the regex - // engine doesn't greedily match `commit` against `/-/commits/...`. - const sentinelIndex = pathParam.search(/\/-\/(tree|blob|commits|commit)/); - if (sentinelIndex === -1) { - throw new Error(`Invalid browse pathname: "${pathParam}" - expected to contain "/-/(tree|blob|commits|commit)/" pattern`); +const decodeBrowsePathPart = (pathPart: string): string | null => { + try { + return decodeURIComponent(pathPart); + } catch { + return null; + } +}; + +export const getBrowseParamsFromPathParam = (pathParam: string): BrowseProps | null => { + const sentinelMatch = pathParam.match(/\/-\/(tree|blob|commits|commit)(?:\/|$)/); + if (!sentinelMatch || sentinelMatch.index === undefined) { + return null; + } + + const sentinelIndex = sentinelMatch.index; + const repoAndRevisionPart = decodeBrowsePathPart(pathParam.substring(0, sentinelIndex)); + if (repoAndRevisionPart === null) { + return null; } - const repoAndRevisionPart = decodeURIComponent(pathParam.substring(0, sentinelIndex)); const lastAtIndex = repoAndRevisionPart.lastIndexOf('@'); const repoName = lastAtIndex === -1 ? repoAndRevisionPart : repoAndRevisionPart.substring(0, lastAtIndex); const revisionName = lastAtIndex === -1 ? undefined : repoAndRevisionPart.substring(lastAtIndex + 1); + if (!repoName) { + return null; + } - const tail = pathParam.substring(sentinelIndex + '/-/'.length); - const pathType = ((): BrowsePathType => { - if (tail.startsWith('tree')) { - return 'tree'; - } - else if (tail.startsWith('commits')) { - return 'commits'; - } - else if (tail.startsWith('commit')) { - return 'commit'; - } - - return 'blob'; - })(); + const pathType = sentinelMatch[1] as BrowsePathType; + const tail = pathParam.substring(sentinelIndex + '/-/'.length + pathType.length); + const pathPart = tail.startsWith('/') ? tail.substring(1) : tail; + const decodedPathPart = decodeBrowsePathPart(pathPart); + if (decodedPathPart === null) { + return null; + } - // @note: decodeURIComponent is needed in case the path contains a space. switch (pathType) { case 'tree': { - const rest = tail.startsWith('tree/') ? tail.substring('tree/'.length) : tail.substring('tree'.length); return { repoName, revisionName, pathType, - path: normalizeRepoPath(decodeURIComponent(rest)), + path: normalizeRepoPath(decodedPathPart), }; } case 'commits': { - const rest = tail.startsWith('commits/') ? tail.substring('commits/'.length) : tail.substring('commits'.length); return { repoName, revisionName, pathType, - path: normalizeRepoPath(decodeURIComponent(rest)), + path: normalizeRepoPath(decodedPathPart), }; } case 'commit': { // Path suffix on /-/commit// is no longer used, but we // keep the slash-split here so legacy URLs still resolve to the // commit (we just ignore everything after the SHA). - const rest = tail.startsWith('commit/') ? tail.substring('commit/'.length) : tail.substring('commit'.length); - const firstSlash = rest.indexOf('/'); - const commitSha = decodeURIComponent(firstSlash === -1 ? rest : rest.substring(0, firstSlash)); + const firstSlash = decodedPathPart.indexOf('/'); + const commitSha = firstSlash === -1 ? decodedPathPart : decodedPathPart.substring(0, firstSlash); if (!commitSha) { - throw new Error(`Invalid browse pathname: "${pathParam}" - expected to contain a commit SHA for commit type`); + return null; } return { @@ -129,11 +132,10 @@ export const getBrowseParamsFromPathParam = (pathParam: string): BrowseProps => }; } case 'blob': { - const rest = tail.startsWith('blob/') ? tail.substring('blob/'.length) : tail.substring('blob'.length); - const path = normalizeRepoPath(decodeURIComponent(rest)); + const path = normalizeRepoPath(decodedPathPart); if (path === '') { - throw new Error(`Invalid browse pathname: "${pathParam}" - expected to contain a path for blob type`); + return null; } return { diff --git a/packages/web/src/app/(app)/browse/layout.tsx b/packages/web/src/app/(app)/browse/layout.tsx deleted file mode 100644 index d9b555c0a..000000000 --- a/packages/web/src/app/(app)/browse/layout.tsx +++ /dev/null @@ -1,17 +0,0 @@ -import { LayoutClient } from "./layoutClient"; -import { getConfiguredLanguageModelsInfo } from "@/features/chat/utils.server"; - -interface LayoutProps { - children: React.ReactNode; -} - -export default async function Layout({ - children, -}: LayoutProps) { - const languageModels = await getConfiguredLanguageModelsInfo(); - return ( - 0}> - {children} - - ) -} diff --git a/packages/web/src/app/(app)/browse/layoutClient.tsx b/packages/web/src/app/(app)/browse/layoutClient.tsx index fb0922d6e..a9cd00359 100644 --- a/packages/web/src/app/(app)/browse/layoutClient.tsx +++ b/packages/web/src/app/(app)/browse/layoutClient.tsx @@ -5,7 +5,8 @@ import { BottomPanel } from "./components/bottomPanel"; import { AnimatedResizableHandle } from "@/components/ui/animatedResizableHandle"; import { BrowseStateProvider } from "./browseStateProvider"; import { FileTreePanel } from "./components/fileTreePanel"; -import { useBrowseParams } from "./hooks/useBrowseParams"; +import { BrowseParamsContext } from "./hooks/useBrowseParams"; +import type { BrowseProps } from "./hooks/utils"; import { FileSearchCommandDialog } from "./components/fileSearchCommandDialog"; import { SearchBar } from "../components/searchBar"; import escapeStringRegexp from "escape-string-regexp"; @@ -13,65 +14,69 @@ import { Separator } from "@/components/ui/separator"; interface LayoutProps { children: React.ReactNode; + browseParams: BrowseProps; isSearchAssistSupported: boolean; } export function LayoutClient({ children, + browseParams, isSearchAssistSupported, }: LayoutProps) { - const { repoName, revisionName, pathType } = useBrowseParams(); + const { repoName, revisionName, pathType } = browseParams; return ( - -
-
-
- + + +
+
+
+ +
+
- -
- - + + - + - - - - {children} - - {(pathType === 'blob' || pathType === 'tree') && ( - <> - - - - )} - - - -
- - + + {children} + + {(pathType === 'blob' || pathType === 'tree') && ( + <> + + + + )} + + + +
+ + + ); }