From 4a5644d51e3235eb5355ea0d86016b6a45936d89 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Fri, 5 Jun 2026 11:53:26 -0700 Subject: [PATCH] refactor(ui): centralize proxy base URL resolution into tested resolver (#29793) * refactor(ui): centralize proxy base URL resolution into tested resolver The API base URL join logic was hand-rolled inside networking.tsx and re-derived inline at hundreds of call sites, with no test coverage and a latent double-slash bug when the base carried a trailing slash. This pulls the join into a single pure resolveApiBase() with full unit coverage and routes the existing resolution through it, also de-duplicating the env precedence ladder that was copied in two places. * test(ui): assert root-path redirect joins prefix exactly once The existing toContain check accepts a doubled separator; tighten it to a strict prefix match plus a no-double-slash assertion so a regression in the resolveApiBase origin+SERVER_ROOT_PATH join is caught end-to-end. --- .../login/serverRootPathRedirect.spec.ts | 8 +- .../src/components/networking.tsx | 39 +++------ .../src/lib/http/resolveApiBase.test.ts | 84 +++++++++++++++++++ .../src/lib/http/resolveApiBase.ts | 35 ++++++++ 4 files changed, 138 insertions(+), 28 deletions(-) create mode 100644 ui/litellm-dashboard/src/lib/http/resolveApiBase.test.ts create mode 100644 ui/litellm-dashboard/src/lib/http/resolveApiBase.ts diff --git a/ui/litellm-dashboard/e2e_tests/tests/login/serverRootPathRedirect.spec.ts b/ui/litellm-dashboard/e2e_tests/tests/login/serverRootPathRedirect.spec.ts index bdef8f85c4..37fea73961 100644 --- a/ui/litellm-dashboard/e2e_tests/tests/login/serverRootPathRedirect.spec.ts +++ b/ui/litellm-dashboard/e2e_tests/tests/login/serverRootPathRedirect.spec.ts @@ -28,5 +28,11 @@ test("unauth redirect preserves SERVER_ROOT_PATH prefix", async ({ page }) => { await page.waitForURL((url) => url.pathname.includes("/ui/login"), { timeout: 15_000 }); - expect(page.url()).toContain(`${ROOT_PATH}/ui/login`); + // The redirect target is built by joining proxyBaseUrl (assembled by + // resolveApiBase from the origin + SERVER_ROOT_PATH) with "/ui/login". A + // regression in that join surfaces as a doubled separator, which the loose + // toContain above would still accept, so assert the prefix joins exactly once. + const { pathname } = new URL(page.url()); + expect(pathname.startsWith(`${ROOT_PATH}/ui/login`)).toBe(true); + expect(pathname).not.toContain("//"); }); diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index 601840cc25..0503490823 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -77,6 +77,7 @@ import { EmailEventSettingsResponse, EmailEventSettingsUpdateRequest } from "./e import { jsonFields } from "./common_components/check_openapi_schema"; import NotificationsManager from "./molecules/notifications_manager"; import { createApiClient, deriveErrorMessage } from "@/lib/http/client"; +import { resolveApiBase } from "@/lib/http/resolveApiBase"; export { deriveErrorMessage }; export { ApiError } from "@/lib/http/client"; @@ -84,11 +85,13 @@ export { ApiError } from "@/lib/http/client"; const isLocal = process.env.NODE_ENV === "development"; // In dev, if NEXT_PUBLIC_USE_REWRITES=true the Next.js dev server proxies API calls // to the backend — use relative URLs (null) so rewrites can intercept them. -const defaultProxyBaseUrl = process.env.NEXT_PUBLIC_BASE_URL - ? process.env.NEXT_PUBLIC_BASE_URL - : isLocal && process.env.NEXT_PUBLIC_USE_REWRITES !== "true" - ? "http://localhost:4000" - : null; +const resolveDefaultBase = (fallback: string | null): string | null => + process.env.NEXT_PUBLIC_BASE_URL + ? process.env.NEXT_PUBLIC_BASE_URL + : isLocal && process.env.NEXT_PUBLIC_USE_REWRITES !== "true" + ? "http://localhost:4000" + : fallback; +const defaultProxyBaseUrl = resolveDefaultBase(null); const defaultServerRootPath = "/"; export let serverRootPath = defaultServerRootPath; const WORKER_URL_KEY = "litellm_worker_url"; @@ -128,28 +131,10 @@ const updateProxyBaseUrl = (serverRootPath: string, receivedProxyBaseUrl: string if (typeof window !== "undefined" && window.localStorage.getItem(WORKER_URL_KEY)) { return; } - const browserLocation = getWindowLocation(); - const resolvedDefaultProxyBaseUrl = process.env.NEXT_PUBLIC_BASE_URL - ? process.env.NEXT_PUBLIC_BASE_URL - : isLocal && process.env.NEXT_PUBLIC_USE_REWRITES !== "true" - ? "http://localhost:4000" - : browserLocation?.origin ?? null; - let initialProxyBaseUrl = receivedProxyBaseUrl || resolvedDefaultProxyBaseUrl; - console.log("proxyBaseUrl:", proxyBaseUrl); - console.log("serverRootPath:", serverRootPath); - - if (!initialProxyBaseUrl) { - proxyBaseUrl = proxyBaseUrl ?? null; - console.log("Updated proxyBaseUrl:", proxyBaseUrl); - return; - } - - if (serverRootPath.length > 0 && !initialProxyBaseUrl.endsWith(serverRootPath) && serverRootPath != "/") { - initialProxyBaseUrl += serverRootPath; - } - - proxyBaseUrl = initialProxyBaseUrl; - console.log("Updated proxyBaseUrl:", proxyBaseUrl); + proxyBaseUrl = resolveApiBase({ + explicitBase: receivedProxyBaseUrl || resolveDefaultBase(getWindowLocation()?.origin ?? null), + serverRootPath, + }); }; const updateServerRootPath = (receivedServerRootPath: string) => { diff --git a/ui/litellm-dashboard/src/lib/http/resolveApiBase.test.ts b/ui/litellm-dashboard/src/lib/http/resolveApiBase.test.ts new file mode 100644 index 0000000000..988b88cf07 --- /dev/null +++ b/ui/litellm-dashboard/src/lib/http/resolveApiBase.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, it } from "vitest"; +import { resolveApiBase } from "./resolveApiBase"; + +describe("resolveApiBase", () => { + describe("same-origin (no explicit base)", () => { + it("returns an empty base so requests stay relative", () => { + expect(resolveApiBase({})).toBe(""); + expect(resolveApiBase({ explicitBase: null, serverRootPath: "/" })).toBe(""); + expect(resolveApiBase({ explicitBase: "", serverRootPath: undefined })).toBe(""); + }); + + it("prefixes a relative base with the server root path", () => { + expect(resolveApiBase({ serverRootPath: "/litellm" })).toBe("/litellm"); + }); + }); + + describe("explicit base (split-origin / dev)", () => { + it("uses the explicit base verbatim when root path is mounted at /", () => { + expect(resolveApiBase({ explicitBase: "http://localhost:4000", serverRootPath: "/" })).toBe( + "http://localhost:4000", + ); + }); + + it("appends the server root path to the explicit base", () => { + expect(resolveApiBase({ explicitBase: "http://localhost:4000", serverRootPath: "/litellm" })).toBe( + "http://localhost:4000/litellm", + ); + }); + }); + + describe("dedup — base already includes the root path", () => { + it("does not double the root path", () => { + expect(resolveApiBase({ explicitBase: "http://localhost:4000/litellm", serverRootPath: "/litellm" })).toBe( + "http://localhost:4000/litellm", + ); + }); + }); + + describe("normalization", () => { + it("trims a trailing slash from the explicit base before appending", () => { + expect(resolveApiBase({ explicitBase: "http://localhost:4000/", serverRootPath: "/litellm" })).toBe( + "http://localhost:4000/litellm", + ); + }); + + it("trims a trailing slash from the explicit base when there is no root path", () => { + expect(resolveApiBase({ explicitBase: "http://localhost:4000/", serverRootPath: "/" })).toBe( + "http://localhost:4000", + ); + }); + + it("adds a leading slash to a root path that lacks one", () => { + expect(resolveApiBase({ explicitBase: "http://localhost:4000", serverRootPath: "litellm" })).toBe( + "http://localhost:4000/litellm", + ); + }); + + it("trims a trailing slash from the root path", () => { + expect(resolveApiBase({ explicitBase: "http://localhost:4000", serverRootPath: "/litellm/" })).toBe( + "http://localhost:4000/litellm", + ); + }); + + it("ignores whitespace around inputs", () => { + expect(resolveApiBase({ explicitBase: " http://localhost:4000 ", serverRootPath: " /litellm " })).toBe( + "http://localhost:4000/litellm", + ); + }); + }); + + describe("multi-segment root paths", () => { + it("appends a nested root path", () => { + expect(resolveApiBase({ explicitBase: "http://localhost:4000", serverRootPath: "/team/litellm" })).toBe( + "http://localhost:4000/team/litellm", + ); + }); + + it("dedups a nested root path", () => { + expect( + resolveApiBase({ explicitBase: "http://localhost:4000/team/litellm", serverRootPath: "/team/litellm" }), + ).toBe("http://localhost:4000/team/litellm"); + }); + }); +}); diff --git a/ui/litellm-dashboard/src/lib/http/resolveApiBase.ts b/ui/litellm-dashboard/src/lib/http/resolveApiBase.ts new file mode 100644 index 0000000000..8294ab315f --- /dev/null +++ b/ui/litellm-dashboard/src/lib/http/resolveApiBase.ts @@ -0,0 +1,35 @@ +export interface ApiBaseInputs { + /** + * Absolute API origin to target instead of same-origin. Comes from the + * NEXT_PUBLIC_BASE_URL build env (dev / split-origin) or the proxy_base_url + * the backend reports in its UI config. Empty/unset means same-origin. + */ + explicitBase?: string | null; + /** + * The proxy's mount path when it sits under a sub-path (e.g. "/litellm" + * behind a reverse proxy). "/" or empty means mounted at the root. + */ + serverRootPath?: string | null; +} + +const normalizeRootPath = (serverRootPath: string | null | undefined): string => { + const trimmed = (serverRootPath ?? "").trim(); + if (trimmed === "" || trimmed === "/") return ""; + const withLeadingSlash = trimmed.startsWith("/") ? trimmed : `/${trimmed}`; + return withLeadingSlash.replace(/\/+$/, ""); +}; + +/** + * Resolve the single API base string that every request is prefixed with. + * + * Same-origin is represented as "" so requests stay relative and work behind + * any domain or reverse proxy without hardcoding an origin. An explicit base is + * used verbatim (trailing slash trimmed). The server root path is appended + * unless the base already ends with it. + */ +export const resolveApiBase = ({ explicitBase, serverRootPath }: ApiBaseInputs): string => { + const base = (explicitBase ?? "").trim().replace(/\/+$/, ""); + const rootPath = normalizeRootPath(serverRootPath); + if (rootPath === "" || base.endsWith(rootPath)) return base; + return `${base}${rootPath}`; +};