Build every ApiClient request URL in one place (closes #18)

getJSON built its URL with new URL and then overwrote the host and
path, which dropped a base path in a self-hosted apiOrigin; postJSON,
putJSON and the file and thumbnail download URLs joined strings. All of
them now go through one function next to ApiClient that accepts a path
with or without a leading slash and an origin with or without a
trailing slash or base path, and percent-encodes query parameters. A
path containing "?" or "#" is rejected.

Model: opus-5-5

TODO.md

@@ -18,6 +18,11 @@ Tag v1.0.0.
 
 # Completed Steps
 
+- 2026-09-23: Every `ApiClient` request URL is now built by one function next to
+  the class (issue 18), so a self-hosted `apiOrigin` with a base path keeps it
+  on every request, a path works with or without a leading slash, and query
+  parameters are percent-encoded. A path containing `?` or `#` is rejected with
+  an error instead of being silently cut.
 - 2026-09-23: Hardened the JPEG EXIF scan behind `backup-metadata --exif` (issue
   11). Every segment length is checked against the remaining bytes and lengths
   under 2 stop the scan, so a truncated or corrupt original can neither throw

src/api/client.ts

@@ -98,6 +98,30 @@ const deadlineStream = (
     });
 };
 
+// The one place a request URL is built. `origin` may carry a base path (a
+// self-hosted server behind a prefix) and may end in a slash; `path` may or
+// may not start with one. Query parameters go only through `query`, which
+// percent-encodes them: a `?` or `#` in `path` is an error, because
+// `new URL` would otherwise quietly treat what follows as something else.
+const buildURL = (
+    origin: string,
+    path: string,
+    query?: Record<string, string | number | undefined>,
+): string => {
+    if (path.includes("?") || path.includes("#")) {
+        throw new Error(
+            `request path must not contain "?" or "#"; pass query parameters separately: ${path}`,
+        );
+    }
+    const url = new URL(
+        `${origin.replace(/\/+$/, "")}/${path.replace(/^\/+/, "")}`,
+    );
+    for (const [k, v] of Object.entries(query ?? {})) {
+        if (v !== undefined) url.searchParams.set(k, String(v));
+    }
+    return url.href;
+};
+
 export class ApiClient {
     private readonly apiOrigin: string;
     private readonly isCustomOrigin: boolean;
@@ -202,22 +226,10 @@ export class ApiClient {
         path: string,
         query?: Record<string, string | number | undefined>,
     ): Promise<T> {
-        const url = new URL(path, this.apiOrigin + "/");
-        // new URL with a base resolves relative paths; ensure we keep the
-        // origin from apiOrigin even when path starts with /
-        url.protocol = new URL(this.apiOrigin).protocol;
-        url.host = new URL(this.apiOrigin).host;
-        url.pathname = path;
-        if (query) {
-            for (const [k, v] of Object.entries(query)) {
-                if (v !== undefined) {
-                    url.searchParams.set(k, String(v));
-                }
-            }
-        }
+        const url = buildURL(this.apiOrigin, path, query);
         // A GET changes nothing, so it is retried under the full policy.
         return withRetry(async () => {
-            const resp = await this._fetch(url.href, {
+            const resp = await this._fetch(url, {
                 method: "GET",
                 headers: this.headers(),
                 signal: AbortSignal.timeout(this.requestTimeoutMs),
@@ -228,7 +240,7 @@ export class ApiClient {
     }
 
     async postJSON<T>(path: string, body: unknown): Promise<T> {
-        const url = `${this.apiOrigin}${path}`;
+        const url = buildURL(this.apiOrigin, path);
         // Not idempotent: a POST is replayed only when `isSafeToReplay`
         // says no request byte can have reached the server. The endpoints
         // this covers are listed in the README under "Endpoints used".
@@ -261,8 +273,8 @@ export class ApiClient {
         opts?: StreamOptions,
     ): Promise<ReadableStream<Uint8Array>> {
         const url = this.isCustomOrigin
-            ? `${this.apiOrigin}/files/download/${fileID}`
-            : `${this.filesOrigin}/?fileID=${fileID}`;
+            ? buildURL(this.apiOrigin, `/files/download/${fileID}`)
+            : buildURL(this.filesOrigin, "/", { fileID });
         return this.streamRequest(url, opts);
     }
 
@@ -304,7 +316,7 @@ export class ApiClient {
     }
 
     async putJSON<T>(path: string, body: unknown): Promise<T> {
-        const url = `${this.apiOrigin}${path}`;
+        const url = buildURL(this.apiOrigin, path);
         // Same replay and redirect rules as `postJSON`, for the same reasons.
         return withRetry(
             async () => {
@@ -340,8 +352,8 @@ export class ApiClient {
         opts?: StreamOptions,
     ): Promise<ReadableStream<Uint8Array>> {
         const url = this.isCustomOrigin
-            ? `${this.apiOrigin}/files/preview/${fileID}`
-            : `${this.thumbsOrigin}/?fileID=${fileID}`;
+            ? buildURL(this.apiOrigin, `/files/preview/${fileID}`)
+            : buildURL(this.thumbsOrigin, "/", { fileID });
         return this.streamRequest(url, opts);
     }
 

test/api/client.test.ts

@@ -385,6 +385,93 @@ describe("ApiClient custom origins", () => {
     });
 });
 
+describe("ApiClient request URLs", () => {
+    it("accepts a path with or without a leading slash", async () => {
+        const { fetch, calls } = recordingFetch(
+            jsonResponse({}),
+            jsonResponse({}),
+            jsonResponse({}),
+        );
+        const client = new ApiClient({ fetch });
+        await client.getJSON("health");
+        await client.postJSON("users/ott", {});
+        await client.putJSON("/files/thumbnail", {});
+
+        expect(calls.map((c) => c.url)).toEqual([
+            "https://api.ente.io/health",
+            "https://api.ente.io/users/ott",
+            "https://api.ente.io/files/thumbnail",
+        ]);
+    });
+
+    it("accepts an apiOrigin with a trailing slash", async () => {
+        const { fetch, calls } = recordingFetch(jsonResponse({}));
+        const client = new ApiClient({
+            fetch,
+            apiOrigin: "https://my-ente.example.com/",
+        });
+        await client.getJSON("/health");
+
+        expect(calls[0]!.url).toBe("https://my-ente.example.com/health");
+    });
+
+    it("keeps a base path in a self-hosted apiOrigin for every request", async () => {
+        const body = new Uint8Array([1]);
+        const { fetch, calls } = recordingFetch(
+            jsonResponse({}),
+            jsonResponse({}),
+            jsonResponse({}),
+            streamResponse(body),
+            streamResponse(body),
+        );
+        const client = new ApiClient({
+            fetch,
+            apiOrigin: "https://example.com/ente/",
+        });
+        await client.getJSON("/collections/v2", { sinceTime: 0 });
+        await client.postJSON("/users/ott", {});
+        await client.putJSON("/files/thumbnail", {});
+        await client.getFileStream(99);
+        await client.getThumbnailStream(77);
+
+        expect(calls.map((c) => c.url)).toEqual([
+            "https://example.com/ente/collections/v2?sinceTime=0",
+            "https://example.com/ente/users/ott",
+            "https://example.com/ente/files/thumbnail",
+            "https://example.com/ente/files/download/99",
+            "https://example.com/ente/files/preview/77",
+        ]);
+    });
+
+    it("percent-encodes query parameters and skips undefined ones", async () => {
+        const { fetch, calls } = recordingFetch(jsonResponse({}));
+        const client = new ApiClient({ fetch });
+        await client.getJSON("/search", {
+            q: "a&b=c/d é",
+            limit: 5,
+            cursor: undefined,
+        });
+
+        const url = new URL(calls[0]!.url);
+        expect(url.pathname).toBe("/search");
+        expect(url.search).toBe("?q=a%26b%3Dc%2Fd+%C3%A9&limit=5");
+        expect(url.searchParams.get("q")).toBe("a&b=c/d é");
+    });
+
+    it("rejects a path that carries its own query string", async () => {
+        const { fetch, calls } = recordingFetch();
+        const client = new ApiClient({ fetch });
+
+        await expect(client.getJSON("/diff?sinceTime=0")).rejects.toThrow(
+            /must not contain "\?"/,
+        );
+        await expect(client.postJSON("/users/ott?x=1", {})).rejects.toThrow(
+            /must not contain "\?"/,
+        );
+        expect(calls).toHaveLength(0);
+    });
+});
+
 describe("ApiError", () => {
     it("throws ApiError on 4xx with status, code, requestID", async () => {
         const { fetch } = recordingFetch(