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

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);
     }