From 99513cb2e27e7171a2a918b04615b96eb68549cd Mon Sep 17 00:00:00 2001 From: kevin9327 <5299031+kevin9327@users.noreply.github.com> Date: Sun, 6 Sep 2026 21:13:53 +0900 Subject: [PATCH] Say which part of DATABASE_URL is not percent-encoded, instead of failing as URI error --- CHANGELOG.md | 11 +++++++++ server/src/db/client.ts | 34 ++++++++++++++++++++------ server/tests/db-client-address.test.ts | 31 +++++++++++++++++++++++ 3 files changed, 69 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c658b49..01ed78b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,17 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A password with a `%` in it says so, instead of failing as `URI error` + +`DATABASE_URL` is taken apart before it reaches Bun, and each part is percent-decoded. A part +holding a `%` that starts no escape -- `postgres://openbot:100%pure@host:5432/openbot`, which a +generated password produces often enough -- is a string `new URL` accepts and `decodeURIComponent` +rejects, so the server stopped with `URIError: URI error` and named neither the variable nor the +part. It now refuses with the same kind of sentence as every other malformed address: which part is +wrong, and that a literal `%` must be written `%25`. + +A correctly encoded password is unaffected. + ### The server connects to Postgres on Windows, and `localhost` is no longer a coin toss Two separate faults, both of which stop a deployment reaching its own database and neither of which diff --git a/server/src/db/client.ts b/server/src/db/client.ts index 5aa79640..a98687a9 100644 --- a/server/src/db/client.ts +++ b/server/src/db/client.ts @@ -3,11 +3,25 @@ import { drizzle } from "drizzle-orm/bun-sql"; import * as schema from "./schema"; /** - * `max` is exposed so tests can pin the pool to a single connection. Code that opens a transaction - * and then reads on a second connection deadlocks once every pooled connection is inside such a - * transaction; a pool of one turns that from a load-dependent production hang into an immediate, - * reproducible failure. + * One percent-decoded part of the address, or a refusal that names it. + * + * A password is where this bites. `postgres://openbot:100%pure@host:5432/openbot` is a string + * `new URL` accepts without complaint, and `decodeURIComponent` then rejects with `URIError: URI + * error` -- a message that names neither `DATABASE_URL` nor which part of it was wrong, thrown out + * of the one function whose whole job is to make a connection failure legible. A `%` that starts no + * escape is a common thing to find in a generated password, and every other malformed address here + * is answered with a sentence saying what to fix. */ +function decodePart(value: string, part: string): string { + try { + return decodeURIComponent(value); + } catch { + throw new TypeError( + `DATABASE_URL has a ${part} that is not percent-encoded. A literal "%" must be written "%25".`, + ); + } +} + /** * The address, taken apart, because Bun will not take it whole on every platform. * @@ -35,7 +49,7 @@ function addressOf(databaseUrl: string) { "DATABASE_URL names no host. Expected postgres://user:password@host:port/database.", ); } - const database = decodeURIComponent(url.pathname.replace(/^\//, "")); + const database = decodePart(url.pathname.replace(/^\//, ""), "database name"); if (database === "") { throw new TypeError( "DATABASE_URL names no database. Expected postgres://user:password@host:port/database.", @@ -55,13 +69,19 @@ function addressOf(databaseUrl: string) { adapter: "postgres" as const, hostname: url.hostname, port: url.port === "" ? 5432 : Number(url.port), - username: decodeURIComponent(url.username), - password: decodeURIComponent(url.password), + username: decodePart(url.username, "username"), + password: decodePart(url.password, "password"), database, ...(Object.keys(connection).length > 0 ? { connection } : {}), }; } +/** + * `max` is exposed so tests can pin the pool to a single connection. Code that opens a transaction + * and then reads on a second connection deadlocks once every pooled connection is inside such a + * transaction; a pool of one turns that from a load-dependent production hang into an immediate, + * reproducible failure. + */ export function createDatabase( databaseUrl: string, options: { max?: number } = {}, diff --git a/server/tests/db-client-address.test.ts b/server/tests/db-client-address.test.ts index 47bdca73..d4efca2b 100644 --- a/server/tests/db-client-address.test.ts +++ b/server/tests/db-client-address.test.ts @@ -46,6 +46,37 @@ describe("the database address", () => { ).toThrow(/names no database/); }); + test("refuses a password holding a percent that starts no escape, naming the part", () => { + /* + * `new URL` accepts this and `decodeURIComponent` does not, so the refusal used to be a bare + * `URIError: URI error` naming neither DATABASE_URL nor the password -- out of the one function + * whose job is to make a connection failure legible. A generated password is a common place to + * find a literal `%`. + */ + expect(() => + createDatabase("postgres://openbot:100%pure@127.0.0.1:5432/openbot"), + ).toThrow(/DATABASE_URL has a password that is not percent-encoded/); + }); + + test("refuses a username holding one too", () => { + expect(() => + createDatabase("postgres://open%bot:openbot@127.0.0.1:5432/openbot"), + ).toThrow(/DATABASE_URL has a username that is not percent-encoded/); + }); + + test("refuses a database name holding one too", () => { + expect(() => + createDatabase("postgres://openbot:openbot@127.0.0.1:5432/open%bot"), + ).toThrow(/DATABASE_URL has a database name that is not percent-encoded/); + }); + + test("still accepts a password that IS percent-encoded, decoding it", () => { + // The escape a correctly written password uses: `%40` is `@`, which cannot be written raw. + expect(() => + createDatabase("postgres://openbot:p%40ss@127.0.0.1:5432/openbot"), + ).not.toThrow(); + }); + test("still refuses pool options where the address belongs", () => { // @ts-expect-error the wrong-way-round call this guard exists for expect(() => createDatabase({ max: 1 })).toThrow(/connection string/);