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.
This commit is contained in:
parent
a4f57032e0
commit
4a5644d51e
@ -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("//");
|
||||
});
|
||||
|
||||
@ -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) => {
|
||||
|
||||
84
ui/litellm-dashboard/src/lib/http/resolveApiBase.test.ts
Normal file
84
ui/litellm-dashboard/src/lib/http/resolveApiBase.test.ts
Normal file
@ -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");
|
||||
});
|
||||
});
|
||||
});
|
||||
35
ui/litellm-dashboard/src/lib/http/resolveApiBase.ts
Normal file
35
ui/litellm-dashboard/src/lib/http/resolveApiBase.ts
Normal file
@ -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}`;
|
||||
};
|
||||
Loading…
Reference in New Issue
Block a user