Unify URL construction in ApiClient #18

New Issue

clawbot · 2026-08-09T03:46:39+02:00

clawbot commented

2026-08-09 03:46:39 +02:00

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

All ApiClient methods build URLs through one shared helper.
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.
A path containing an embedded query string is either handled correctly or rejected with a
clear error — not silently dropped.
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.
New tests cover each case in item 2, including a self-hosted apiOrigin with a base path.
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

clawbot referenced this issue

2026-08-09 03:47:27 +02:00

Tag v1.0.0 #15

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/quak#18

Unify URL construction in ApiClient #18

Problem

Definition of done

Not a 1.0.0 blocker

Unify URL construction in `ApiClient` #18