Unify URL construction in ApiClient #18

Closed
opened 2026-08-09 03:46:39 +02:00 by clawbot · 2 comments
Collaborator

Problem

ApiClient builds request URLs two different ways.

getJSON (src/api/client.ts:118-123) constructs a URL from a base and then overwrites
protocol, host and pathname in place. Assigning url.pathname = path silently discards
any query string embedded in path and does not percent-encode the value it is given.

postJSON (:140) and putJSON (:190) instead concatenate strings:
${this.apiOrigin}${path}, with no encoding and no handling of a missing or doubled slash.

Two strategies, neither of them obviously correct, is exactly how a path-handling bug gets
introduced later.

Definition of done

  1. All ApiClient methods build URLs through one shared helper.
  2. The helper handles: a path with or without a leading slash, an apiOrigin with or
    without a trailing slash, an apiOrigin that includes a base path (self-hosted servers
    behind a prefix), and query parameters that require percent-encoding.
  3. A path containing an embedded query string is either handled correctly or rejected with a
    clear error — not silently dropped.
  4. Existing behaviour for every current call site is unchanged; the tests in
    test/api/client.test.ts that assert on request URLs still pass untouched.
  5. New tests cover each case in item 2, including a self-hosted apiOrigin with a base path.
  6. make check green.

Not a 1.0.0 blocker

No known failure in production use today; this is hardening against a class of bug rather than
fixing an observed one.

## Problem `ApiClient` builds request URLs two different ways. `getJSON` (`src/api/client.ts:118-123`) constructs a `URL` from a base and then overwrites `protocol`, `host` and `pathname` in place. Assigning `url.pathname = path` silently discards any query string embedded in `path` and does not percent-encode the value it is given. `postJSON` (`:140`) and `putJSON` (`:190`) instead concatenate strings: `${this.apiOrigin}${path}`, with no encoding and no handling of a missing or doubled slash. Two strategies, neither of them obviously correct, is exactly how a path-handling bug gets introduced later. ## Definition of done 1. All `ApiClient` methods build URLs through one shared helper. 2. The helper handles: a `path` with or without a leading slash, an `apiOrigin` with or without a trailing slash, an `apiOrigin` that includes a base path (self-hosted servers behind a prefix), and query parameters that require percent-encoding. 3. A `path` containing an embedded query string is either handled correctly or rejected with a clear error — not silently dropped. 4. Existing behaviour for every current call site is unchanged; the tests in `test/api/client.test.ts` that assert on request URLs still pass untouched. 5. New tests cover each case in item 2, including a self-hosted `apiOrigin` with a base path. 6. `make check` green. ## Not a 1.0.0 blocker No known failure in production use today; this is hardening against a class of bug rather than fixing an observed one.
clawbot self-assigned this 2026-08-09 03:46:39 +02:00
Author
Collaborator

Implementer brief (branch next2)

The issue's line numbers are out of date. In src/api/client.ts today:

  • getJSON builds its URL at 205-214 with new URL and then overwrites protocol, host and pathname, which drops a base path and any query string in path.
  • postJSON (231) and putJSON (307) join strings with ${this.apiOrigin}${path}.
  • The download and preview URLs at 264-265 and 343-344 also join strings, against apiOrigin, filesOrigin and thumbsOrigin.
  • apiOrigin has its trailing slash stripped at 113.

Definition of done: as in the issue body. The one shared helper covers every site above, including the file-download and thumbnail URLs. A path containing ? is rejected with a clear error; query parameters go only through the helper's parameter argument. Keep the helper a plain function next to ApiClient, not a new class.

Model: opus-5-5

## Implementer brief (branch `next2`) The issue's line numbers are out of date. In `src/api/client.ts` today: - `getJSON` builds its URL at 205-214 with `new URL` and then overwrites `protocol`, `host` and `pathname`, which drops a base path and any query string in `path`. - `postJSON` (231) and `putJSON` (307) join strings with `${this.apiOrigin}${path}`. - The download and preview URLs at 264-265 and 343-344 also join strings, against `apiOrigin`, `filesOrigin` and `thumbsOrigin`. - `apiOrigin` has its trailing slash stripped at 113. Definition of done: as in the issue body. The one shared helper covers every site above, including the file-download and thumbnail URLs. A `path` containing `?` is rejected with a clear error; query parameters go only through the helper's parameter argument. Keep the helper a plain function next to `ApiClient`, not a new class. Model: opus-5-5
Author
Collaborator

Implemented in #87 (base next2).

Model: opus-5-5

Implemented in https://git.eeqj.de/sneak/quak/pulls/87 (base `next2`). Model: opus-5-5
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/quak#18