diff --git a/backend/cli/src/server/server.ts b/backend/cli/src/server/server.ts index 8f697005..2976b49c 100644 --- a/backend/cli/src/server/server.ts +++ b/backend/cli/src/server/server.ts @@ -5,7 +5,7 @@ import { describeRoute, generateSpecs, validator, resolver, openAPIRouteHandler import { Hono } from "hono" import { cors } from "hono/cors" import { streamSSE } from "hono/streaming" -import { serveWebAsset } from "../web/serve" +import { serveWebAsset, wantsJson } from "../web/serve" import { isAllowedHost, isAllowedOrigin, isCrossOrigin } from "./host-guard" import { timingSafeEqual } from "../util/timing-safe" import { FolderResolveRoutes } from "./routes/folder-resolve" @@ -80,7 +80,7 @@ export namespace Server { return _server?.requestIP(req)?.address } - const app = new Hono() + const app = new Hono({ strict: false }) export const App: () => Hono = lazy( () => // TODO: Break server.ts into smaller route files to fix type inference @@ -640,6 +640,11 @@ export namespace Server { // try to JSON.parse `` and crash (#180/#197/#189). + */ +export function wantsJson(accept: string | null, contentTypeHeader: string | null): boolean { + if (contentTypeHeader?.includes("application/json")) return true + const a = accept ?? "" + return a.includes("application/json") && !a.includes("text/html") +} + export async function serveWebAsset(c: Context): Promise { if (!WEB_INDEX) return undefined diff --git a/backend/cli/test/server/spa-fallback.test.ts b/backend/cli/test/server/spa-fallback.test.ts new file mode 100644 index 00000000..c949bd64 --- /dev/null +++ b/backend/cli/test/server/spa-fallback.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, test } from "bun:test" +import path from "path" +import { Instance } from "../../src/project/instance" +import { Server } from "../../src/server/server" +import { Log } from "../../src/util/log" + +const projectRoot = path.join(__dirname, "../..") +Log.init({ print: false }) + +describe("spa fallback", () => { + test("unmatched API-shaped request under /settings gets a JSON 404, not SPA HTML", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const fetch = Server.internalFetch() + const response = await fetch("http://openscience.internal/settings/nonexistent", { + headers: { "content-type": "application/json" }, + }) + + expect(response.status).toBe(404) + expect(response.headers.get("content-type")).toContain("application/json") + + const text = await response.text() + expect(text.startsWith("<")).toBe(false) + expect(JSON.parse(text)).toEqual({ error: "not_found", path: "/settings/nonexistent" }) + }, + }) + }) + + test("browser navigation to an unmatched route still gets the SPA index.html", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const fetch = Server.internalFetch() + const response = await fetch("http://openscience.internal/definitely/not/a/route", { + headers: { accept: "text/html" }, + }) + + expect(response.status).toBe(200) + expect(response.headers.get("content-type")).toContain("text/html") + expect(response.headers.get("content-security-policy")).toContain("default-src 'self'") + + const text = await response.text() + expect(text.toLowerCase().startsWith(" { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const fetch = Server.internalFetch() + const response = await fetch("http://openscience.internal/api/also-nonexistent") + + expect(response.status).toBe(404) + }, + }) + }) + + test("GET /settings/local/ (trailing slash) resolves to the real route, not the catch-all", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const fetch = Server.internalFetch() + const response = await fetch("http://openscience.internal/settings/local/", { + headers: { "content-type": "application/json" }, + }) + + expect(response.status).toBe(200) + expect(response.headers.get("content-type")).toContain("application/json") + + const body = await response.json() + expect(Array.isArray(body.presets)).toBe(true) + }, + }) + }) +}) diff --git a/backend/cli/test/web/serve.test.ts b/backend/cli/test/web/serve.test.ts new file mode 100644 index 00000000..fdc7f974 --- /dev/null +++ b/backend/cli/test/web/serve.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, test } from "bun:test" +import { wantsJson } from "../../src/web/serve" + +describe("wantsJson", () => { + test("content-type application/json → true", () => { + expect(wantsJson(null, "application/json")).toBe(true) + }) + + test("accept includes application/json, no text/html → true", () => { + expect(wantsJson("application/json, text/plain", null)).toBe(true) + }) + + test("browser navigation accept header → false", () => { + expect(wantsJson("text/html,application/xhtml+xml,*/*", null)).toBe(false) + }) + + test("text/html present alongside application/json → treated as navigation, false", () => { + expect(wantsJson("text/html,application/json", null)).toBe(false) + }) + + test("wildcard accept → false", () => { + expect(wantsJson("*/*", null)).toBe(false) + }) + + test("both null → false", () => { + expect(wantsJson(null, null)).toBe(false) + }) +}) diff --git a/frontend/workspace/src/components/settings/LocalModels.tsx b/frontend/workspace/src/components/settings/LocalModels.tsx index f827542f..a73d7b17 100644 --- a/frontend/workspace/src/components/settings/LocalModels.tsx +++ b/frontend/workspace/src/components/settings/LocalModels.tsx @@ -54,7 +54,7 @@ const LocalModels: Component = () => { call<{ detected: Detected[] }>("/detect").then((r) => r.detected), ) const [configured, { refetch: refetchConfigured }] = createResource(() => - call<{ providers: Configured[] }>("/").then((r) => r.providers), + call<{ providers: Configured[] }>("").then((r) => r.providers), ) const [status, { refetch: refetchStatus }] = createResource(() => call<{ runtimes: Runtime[] }>("/status").then((r) => r.runtimes), @@ -80,7 +80,7 @@ const LocalModels: Component = () => { const addRuntime = (d: Detected) => guard( () => - call("/", { + call("", { method: "POST", body: JSON.stringify({ url: d.baseURL, id: d.id, name: `${d.name} (local)`, models: d.models }), }), @@ -106,7 +106,7 @@ const LocalModels: Component = () => { showToast({ title: `${rt.name} isn't installed`, description: `Install it, then start it here.` }) window.open(r.install ?? rt.install, "_blank", "noopener") } else if (r.running && r.models?.length) { - await call("/", { + await call("", { method: "POST", body: JSON.stringify({ url: rt.baseURL, id: rt.id, name: `${rt.name} (local)`, models: r.models }), }) @@ -126,7 +126,7 @@ const LocalModels: Component = () => { const addRunning = (rt: Runtime) => guard( () => - call("/", { + call("", { method: "POST", body: JSON.stringify({ url: rt.baseURL, id: rt.id, name: `${rt.name} (local)`, models: rt.models }), }), @@ -172,7 +172,7 @@ const LocalModels: Component = () => { guard(async () => { const models = [...selected()] if (!models.length) throw new Error("Select at least one model.") - await call("/", { + await call("", { method: "POST", body: JSON.stringify({ url: url().trim(), key: key().trim() || undefined, models }), }) diff --git a/frontend/workspace/src/components/settings/api.test.ts b/frontend/workspace/src/components/settings/api.test.ts new file mode 100644 index 00000000..4be2e714 --- /dev/null +++ b/frontend/workspace/src/components/settings/api.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, test } from "bun:test" +import { settingsApi } from "./api" + +const base = "http://x" +const path = "/settings/local" + +describe("settingsApi", () => { + test("throws a descriptive error when a 200 response is not JSON", async () => { + const fetchFn = (async () => + new Response("", { + status: 200, + headers: { "content-type": "text/html; charset=utf-8" }, + })) as unknown as typeof fetch + + const error = await settingsApi(base, fetchFn, path).catch((e: Error) => e) + + expect(error).toBeInstanceOf(Error) + expect(error.message).toContain("/settings/local") + expect(error.message).toContain("Expected JSON") + expect(error.message).not.toContain("Unexpected token") + }) + + test("resolves with parsed JSON on a normal 200 response", async () => { + const fetchFn = (async () => Response.json({ ok: true }, { status: 200 })) as unknown as typeof fetch + + expect(await settingsApi<{ ok: boolean }>(base, fetchFn, path)).toEqual({ ok: true }) + }) + + test("resolves with undefined on 204", async () => { + const fetchFn = (async () => new Response(null, { status: 204 })) as unknown as typeof fetch + + expect(await settingsApi(base, fetchFn, path)).toBeUndefined() + }) + + test("preserves the existing non-ok error path", async () => { + const fetchFn = (async () => new Response("boom", { status: 500 })) as unknown as typeof fetch + + await expect(settingsApi(base, fetchFn, path)).rejects.toThrow(/boom|500/) + }) +}) diff --git a/frontend/workspace/src/components/settings/api.ts b/frontend/workspace/src/components/settings/api.ts index 783a44f6..c490e998 100644 --- a/frontend/workspace/src/components/settings/api.ts +++ b/frontend/workspace/src/components/settings/api.ts @@ -17,5 +17,9 @@ export async function settingsApi( throw new Error(text || `${res.status} ${res.statusText}`) } if (res.status === 204) return undefined as T + const contentType = res.headers.get("content-type") ?? "" + if (!contentType.includes("application/json")) { + throw new Error(`Expected JSON from ${path}, but got ${res.status} (${contentType || "no content-type"})`) + } return (await res.json()) as T }