fix(router): only strip basePath when it is a leading prefix (#149)

* fix(router): only strip basePath when it is a leading prefix

The router derived the route path by stripping basePath wherever it
appeared via pathname.split(basePath), which dropped any segments before
the first occurrence. A pathname like /x/api/test would then resolve onto
the internal route /test even though it is not mounted under basePath.

Strip basePath only when it is a leading, /-boundary prefix of the request
pathname; pathnames outside basePath now resolve to a 404. Trailing slashes
on basePath are normalized so /api/auth/ and /api/auth behave the same.

* refactor(router): derive base path once and flatten path resolution

Compute the trailing-slash-normalized base path once at router creation
instead of on every request, and resolve the request path with guard-clause
early returns. Behavior is unchanged. Add a regression test for a sibling
prefix that matches without a "/" boundary.

---------

Co-authored-by: Taesu <bytaesu@gmail.com>
(cherry picked from commit de59505)

Author: 2 people

packages/better-call/src/router.test.ts

@@ -1124,6 +1124,85 @@ describe("base path", () => {
 		const text = await response.text();
 		expect(text).toBe("hello world");
 	});
+
+	it("should not route a path where the base path is not a leading prefix", async () => {
+		const endpoint = createEndpoint(
+			"/test",
+			{
+				method: "GET",
+			},
+			async () => {
+				return "hello world";
+			},
+		);
+		const router = createRouter({ endpoint }, { basePath: "/api" });
+		// The base path appears mid-path, not as a leading prefix, so it should
+		// not be stripped and the request should not resolve onto "/test".
+		const response = await router.handler(
+			new Request("http://localhost/x/api/test"),
+		);
+		expect(response.status).toBe(404);
+	});
+
+	it("should not invoke a handler when the base path appears after another segment", async () => {
+		let called = false;
+		const endpoint = createEndpoint(
+			"/test",
+			{
+				method: "GET",
+			},
+			async () => {
+				called = true;
+				return "hello world";
+			},
+		);
+		const router = createRouter({ endpoint }, { basePath: "/api" });
+		const response = await router.handler(
+			new Request("http://localhost/foo/api/test"),
+		);
+		expect(response.status).toBe(404);
+		expect(called).toBe(false);
+	});
+
+	it("should normalize a base path with a trailing slash", async () => {
+		const endpoint = createEndpoint(
+			"/test",
+			{
+				method: "GET",
+			},
+			async () => {
+				return "hello world";
+			},
+		);
+		const router = createRouter({ endpoint }, { basePath: "/api/" });
+		const response = await router.handler(
+			new Request("http://localhost/api/test"),
+		);
+		expect(response.status).toBe(200);
+		expect(await response.text()).toBe("hello world");
+	});
+
+	it("should not route when the base path matches without a '/' boundary", async () => {
+		let called = false;
+		const endpoint = createEndpoint(
+			"/test",
+			{
+				method: "GET",
+			},
+			async () => {
+				called = true;
+				return "hello world";
+			},
+		);
+		const router = createRouter({ endpoint }, { basePath: "/admin" });
+		// "/admin" is a leading substring of "/administrator" but not a
+		// "/"-boundary prefix, so the request must not resolve onto "/test".
+		const response = await router.handler(
+			new Request("http://localhost/administrator/test"),
+		);
+		expect(response.status).toBe(404);
+		expect(called).toBe(false);
+	});
 });
 
 /**

packages/better-call/src/router.ts

@@ -157,31 +157,37 @@ export const createRouter = <
 		}
 	}
 
+	// Normalize the configured base path once. `basePath` is configuration, not
+	// per-request input, so trailing-slash normalization belongs here rather than
+	// in the request hot path. An empty result (unset, "/", or all slashes) means
+	// "no base path": route on the full pathname.
+	const basePath =
+		config?.basePath && config.basePath !== "/"
+			? config.basePath.replace(/\/+$/, "")
+			: "";
+
 	const processRequest = async (request: Request) => {
 		const url = new URL(request.url);
 		const pathname = url.pathname;
-		const path =
-			config?.basePath && config.basePath !== "/"
-				? pathname
-						.split(config.basePath)
-						.reduce((acc, curr, index) => {
-							if (index !== 0) {
-								if (index > 1) {
-									acc.push(`${config.basePath}${curr}`);
-								} else {
-									acc.push(curr);
-								}
-							}
-							return acc;
-						}, [] as string[])
-						.join("")
-				: url.pathname;
-		if (!path?.length) {
-			return new Response(null, { status: 404, statusText: "Not Found" });
+		// Strip `basePath` only when it is a leading, "/"-boundary prefix of the
+		// request pathname. A pathname that does not start with the configured
+		// basePath is outside this router and resolves to a 404, so a path like
+		// "/x/api/test" never reaches "/test". The "/" boundary also rejects a
+		// path where basePath is only a leading substring, not a full segment.
+		// The previous implementation stripped basePath wherever it occurred
+		// (`pathname.split(basePath)`).
+		let path: string;
+		if (basePath) {
+			if (!pathname.startsWith(`${basePath}/`)) {
+				return new Response(null, { status: 404, statusText: "Not Found" });
+			}
+			path = pathname.slice(basePath.length);
+		} else {
+			path = pathname;
 		}
 
-		// Reject paths with consecutive slashes
-		if (/\/{2,}/.test(path)) {
+		// Reject empty paths and paths with consecutive slashes.
+		if (path.length === 0 || /\/{2,}/.test(path)) {
 			return new Response(null, { status: 404, statusText: "Not Found" });
 		}