Skip to content

Commit 71b1b67

Browse files
committed
fix(cli): match pgconn TLS/password/multi-host parity for test db connections
Address three connection-string parity gaps in the test db port: - prefer/unset sslmode must not fall back to plaintext. Go's ConnectByUrl strips every non-TLS fallback when the primary is TLS (connect.go:156-168), so a default remote connection fails rather than downgrading. legacySslConfigsFor modeled pgconn's raw configTLS list and wrongly kept the plaintext fallback. - Honor pgconn's empty-password precedence: a connection-string password (even an explicit empty user:@host / ?password= / password=) overrides PGPASSWORD and then falls through to .pgpass. resolveLibpqPassword centralizes this; the URL userinfo ":" is detected from the raw string since WHATWG URL cannot. - Support libpq multi-host failover strings (postgres://h1:5432,h2:5433/db and host=h1,h2 port=...). LegacyPgConnInput gains a fallbacks list; the connect layer dials the primary then each fallback, reusing the first port per pgconn.
1 parent 5738da6 commit 71b1b67

6 files changed

Lines changed: 433 additions & 70 deletions

apps/cli/src/legacy/commands/test/db/SIDE_EFFECTS.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,15 @@ command (exit 1).
9696
`?options=reference=<ref>`) parameters are preserved on both forms. A malformed URL or
9797
percent escape surfaces as a redacted `failed to parse connection string` error, never
9898
an unhandled defect.
99+
- Multi-host failover connection strings (`postgres://h1:5432,h2:5433/db`,
100+
`host=h1,h2 port=5432,5433`) are supported on both forms, matching pgconn
101+
(`config.go:326-362`): the primary host is dialed first, then each fallback in order,
102+
reusing the first port when a host omits one.
103+
- Password precedence matches pgconn/libpq (`config.go:264-379`): a password supplied by
104+
the connection string — **even an explicit empty one** (`user:@host`, `?password=`,
105+
`password=`) — overrides `PGPASSWORD`; an empty resolved value then falls through to
106+
`.pgpass`. A connection string with no password key at all uses `PGPASSWORD` then
107+
`.pgpass`.
99108
- `--dns-resolver https` (global flag, Go's `utils.DNSResolver`): for remote connections
100109
the DB host is resolved via Cloudflare DNS-over-HTTPS (`https://1.1.1.1/dns-query`)
101110
before dialing, mirroring Go's `cc.LookupFunc = FallbackLookupIP` (`connect.go:211`).

apps/cli/src/legacy/shared/legacy-db-config.parse.ts

Lines changed: 204 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,93 @@ function unbracketIpv6(host: string): string {
6262
return host.startsWith("[") && host.endsWith("]") ? host.slice(1, -1) : host;
6363
}
6464

65+
/**
66+
* Resolve a libpq password with pgconn's precedence (`mergeSettings` plus the
67+
* `config.Password == ""` `.pgpass` fallback, `config.go:264-379`): a password
68+
* supplied by the connection string — **even an explicit empty one**
69+
* (`user:@host`, `?password=`, `password=`) — overrides `PGPASSWORD`, because the
70+
* connection-string settings are merged over the env settings; an absent password
71+
* falls back to `PGPASSWORD`. Either way, an empty resolved value then falls
72+
* through to `.pgpass`. `connStringPassword` is `undefined` only when the string
73+
* did not specify a password key at all. `host`/`port` are the primary host:
74+
* pgconn keys `.pgpass` off `config.Host` (the first fallback host).
75+
*/
76+
function resolveLibpqPassword(
77+
connStringPassword: string | undefined,
78+
host: string,
79+
port: number,
80+
database: string,
81+
user: string,
82+
): string {
83+
const resolved = connStringPassword ?? libpqEnv("PGPASSWORD") ?? "";
84+
return resolved.length > 0 ? resolved : legacyPgpassPassword(host, port, database, user);
85+
}
86+
87+
/**
88+
* Zip a comma-separated host list with a comma-separated port list into the
89+
* ordered dial targets, mirroring pgconn's per-host fallback expansion
90+
* (`config.go:326-362`): hosts and ports are split independently, and a host with
91+
* no matching port reuses the first port (`ports[0]`). A non-numeric (or empty)
92+
* port is a `parsePort` error, surfaced as `undefined` so the caller rejects the
93+
* DSN. `hostString`/`portString` carry the bare hosts and ports only — for a URL,
94+
* the structural `host:port` segments are pre-split by `parseHostPortSegment`.
95+
*/
96+
function buildLegacyHostList(
97+
hostString: string,
98+
portString: string,
99+
): Array<{ host: string; port: number }> | undefined {
100+
const hosts = hostString.split(",");
101+
const ports = portString.split(",");
102+
const list: Array<{ host: string; port: number }> = [];
103+
for (let i = 0; i < hosts.length; i++) {
104+
const portRaw = i < ports.length ? ports[i]! : ports[0]!;
105+
if (!/^\d+$/.test(portRaw)) return undefined;
106+
list.push({ host: hosts[i]!, port: Number(portRaw) });
107+
}
108+
return list;
109+
}
110+
111+
/** Extract a URL's authority (between `://` and the first `/`, `?`, or `#`). */
112+
function legacyUrlAuthority(url: string): string {
113+
const schemeEnd = url.indexOf("://");
114+
const rest = schemeEnd === -1 ? url : url.slice(schemeEnd + 3);
115+
const end = rest.search(/[/?#]/);
116+
return end === -1 ? rest : rest.slice(0, end);
117+
}
118+
119+
/** Split a `host:port,host:port` list on top-level commas, respecting `[ipv6]`. */
120+
function splitHostPortList(value: string): string[] {
121+
const segments: string[] = [];
122+
let depth = 0;
123+
let current = "";
124+
for (const ch of value) {
125+
if (ch === "[") depth++;
126+
else if (ch === "]") depth = Math.max(0, depth - 1);
127+
if (ch === "," && depth === 0) {
128+
segments.push(current);
129+
current = "";
130+
} else {
131+
current += ch;
132+
}
133+
}
134+
segments.push(current);
135+
return segments;
136+
}
137+
138+
/** Parse one `host`, `host:port`, `[ipv6]`, or `[ipv6]:port` authority segment. */
139+
function parseHostPortSegment(segment: string): { host: string; port: string } {
140+
if (segment.startsWith("[")) {
141+
const close = segment.indexOf("]");
142+
if (close === -1) return { host: segment, port: "" };
143+
const after = segment.slice(close + 1);
144+
return { host: segment.slice(1, close), port: after.startsWith(":") ? after.slice(1) : "" };
145+
}
146+
const colon = segment.lastIndexOf(":");
147+
return colon === -1
148+
? { host: segment, port: "" }
149+
: { host: segment.slice(0, colon), port: segment.slice(colon + 1) };
150+
}
151+
65152
/**
66153
* Parse a Postgres connection string into a `LegacyPgConnInput`. Mirrors Go's
67154
* `pgconn.ParseConfig` (`apps/cli-go/internal/utils/flags/db_url.go:64`), which
@@ -89,9 +176,35 @@ export function parseLegacyConnectionString(value: string): LegacyPgConnInput |
89176

90177
/** Parse the WHATWG `postgres(ql)://` URL form. */
91178
function parseUrlConnectionString(value: string): LegacyPgConnInput | undefined {
179+
const trimmed = value.trim();
180+
// pgconn accepts libpq multi-host failover URLs (`postgres://h1:5432,h2:5433/db`,
181+
// `config.go:166,326-362`), which WHATWG `new URL()` rejects (the comma'd
182+
// host:port is not a valid authority). Hand-extract the authority so we can split
183+
// the host list ourselves, then normalize the URL down to its first host so
184+
// `new URL()` still parses the userinfo, path, and query exactly as before.
185+
const authority = legacyUrlAuthority(trimmed);
186+
// Go's `net/url` splits userinfo from host on the last `@`; literal `@` in a
187+
// password must be percent-encoded, so the last `@` is the real boundary.
188+
const atIdx = authority.lastIndexOf("@");
189+
const userinfoRaw = atIdx === -1 ? "" : authority.slice(0, atIdx);
190+
const hostPortRaw = atIdx === -1 ? authority : authority.slice(atIdx + 1);
191+
const segments = splitHostPortList(hostPortRaw);
192+
const multiHost = segments.length > 1;
193+
194+
let normalized = trimmed;
195+
if (multiHost) {
196+
const authorityStart = trimmed.indexOf("://") + 3;
197+
const newAuthority =
198+
atIdx === -1 ? segments[0]! : `${authority.slice(0, atIdx + 1)}${segments[0]!}`;
199+
normalized =
200+
trimmed.slice(0, authorityStart) +
201+
newAuthority +
202+
trimmed.slice(authorityStart + authority.length);
203+
}
204+
92205
let url: URL;
93206
try {
94-
url = new URL(value);
207+
url = new URL(normalized);
95208
} catch {
96209
return undefined;
97210
}
@@ -115,13 +228,6 @@ function parseUrlConnectionString(value: string): LegacyPgConnInput | undefined
115228
// `mergeSettings(defaultSettings, envSettings, connStringSettings)`.
116229
const rawUser = queryOrElse("user", decodeURIComponent(url.username));
117230
const user = rawUser.length > 0 ? rawUser : defaultOsUser();
118-
const rawPassword = queryOrElse("password", decodeURIComponent(url.password));
119-
// WHATWG `URL.hostname` keeps the brackets around an IPv6 literal (`[::1]`),
120-
// but `net`/node-postgres and `PGHOST` expect the bare address. Go's
121-
// `url.Hostname()` returns the unbracketed host and only re-adds brackets when
122-
// formatting a URL (`ToPostgresURL`), so strip them here.
123-
const rawHost = queryOrElse("host", unbracketIpv6(url.hostname));
124-
const rawDatabase = queryOrElse("dbname", decodeURIComponent(url.pathname.replace(/^\//, "")));
125231
// libpq fills `sslmode` from `PGSSLMODE` when the connection string omits it
126232
// (pgconn's `parseEnvSettings`), before the TLS-mode default.
127233
const sslmode = url.searchParams.get("sslmode") ?? libpqEnv("PGSSLMODE") ?? null;
@@ -131,6 +237,30 @@ function parseUrlConnectionString(value: string): LegacyPgConnInput | undefined
131237
// libpq `sslrootcert` (query or `PGSSLROOTCERT`) pins the server CA.
132238
const sslrootcert = url.searchParams.get("sslrootcert") ?? libpqEnv("PGSSLROOTCERT") ?? null;
133239
const options = url.searchParams.get("options");
240+
241+
// Structural hosts/ports become pgconn's comma-joined `settings["host"]` /
242+
// `settings["port"]`. WHATWG `URL.hostname` keeps the brackets around an IPv6
243+
// literal (`[::1]`); Go's `url.Hostname()` returns the unbracketed host (only
244+
// re-adding brackets when formatting via `ToPostgresURL`), so strip them. For a
245+
// multi-host URL the per-segment host/port were already split out by hand.
246+
const structuralHosts = multiHost
247+
? segments.map((s) => parseHostPortSegment(s).host).filter((h) => h.length > 0)
248+
: url.hostname.length > 0
249+
? [unbracketIpv6(url.hostname)]
250+
: [];
251+
const structuralPorts = multiHost
252+
? segments.map((s) => parseHostPortSegment(s).port).filter((p) => p.length > 0)
253+
: url.port.length > 0
254+
? [url.port]
255+
: [];
256+
257+
const hostQuery = query.get("host");
258+
const hostString =
259+
hostQuery !== null && hostQuery.length > 0
260+
? hostQuery
261+
: structuralHosts.length > 0
262+
? structuralHosts.join(",")
263+
: (libpqEnv("PGHOST") ?? defaultLibpqHost());
134264
// pgconn merges a `?port=` query setting over the structural port and then
135265
// parses it, so an explicit query port — even empty (`?port=`) or non-numeric
136266
// — that is not a valid number is a parse error. A bad `PGPORT` fallback is
@@ -139,25 +269,51 @@ function parseUrlConnectionString(value: string): LegacyPgConnInput | undefined
139269
if (portQuery !== null && !/^\d+$/.test(portQuery)) {
140270
return undefined;
141271
}
142-
const rawPort = portQuery ?? url.port;
143-
const port = rawPort.length > 0 ? Number(rawPort) : libpqPort(libpqEnv("PGPORT"));
144-
if (port === undefined) {
272+
let portString: string;
273+
if (portQuery !== null) {
274+
portString = portQuery;
275+
} else if (structuralPorts.length > 0) {
276+
portString = structuralPorts.join(",");
277+
} else {
278+
const envPort = libpqPort(libpqEnv("PGPORT"));
279+
if (envPort === undefined) return undefined;
280+
portString = String(envPort);
281+
}
282+
283+
const hostList = buildLegacyHostList(hostString, portString);
284+
if (hostList === undefined || hostList.length === 0) {
145285
return undefined;
146286
}
147-
const host = rawHost.length > 0 ? rawHost : (libpqEnv("PGHOST") ?? defaultLibpqHost());
287+
const primary = hostList[0]!;
288+
289+
const rawDatabase = queryOrElse("dbname", decodeURIComponent(url.pathname.replace(/^\//, "")));
148290
// Absent database → PGDATABASE, then the resolved user (libpq default).
149291
const database = rawDatabase.length > 0 ? rawDatabase : (libpqEnv("PGDATABASE") ?? user);
150-
// Password precedence (pgconn): connection string → PGPASSWORD → `.pgpass`.
151-
const password =
152-
rawPassword.length > 0
153-
? rawPassword
154-
: (libpqEnv("PGPASSWORD") ?? legacyPgpassPassword(host, port, database, user));
292+
293+
// Password precedence (pgconn): the query loop runs last, so `?password=`
294+
// overrides the userinfo password. A `:` in the raw userinfo marks a present
295+
// (possibly empty) userinfo password — `user:@host` — which WHATWG `url.password`
296+
// cannot distinguish from an absent one (`user@host`), so detect it from the
297+
// raw string. `resolveLibpqPassword` then applies the PGPASSWORD/`.pgpass` rules.
298+
const connStringPassword = query.has("password")
299+
? (query.get("password") ?? "")
300+
: userinfoRaw.includes(":")
301+
? decodeURIComponent(url.password)
302+
: undefined;
303+
const password = resolveLibpqPassword(
304+
connStringPassword,
305+
primary.host,
306+
primary.port,
307+
database,
308+
user,
309+
);
155310
return {
156-
host,
157-
port,
311+
host: primary.host,
312+
port: primary.port,
158313
user,
159314
password,
160315
database,
316+
...(hostList.length > 1 ? { fallbacks: hostList.slice(1) } : {}),
161317
...(options !== null && options.length > 0 ? { options } : {}),
162318
...(sslmode !== null && sslmode.length > 0 ? { sslmode } : {}),
163319
...(sslrootcert !== null && sslrootcert.length > 0 ? { sslrootcert } : {}),
@@ -211,13 +367,24 @@ function parseKeywordValueDsn(value: string): LegacyPgConnInput | undefined {
211367
}
212368
// Omitted fields fall back to libpq `PG*` env vars and then the libpq defaults,
213369
// matching pgconn's `mergeSettings(defaultSettings, envSettings, connStringSettings)`.
214-
const host =
370+
// A libpq DSN also accepts comma-separated multi-host failover
371+
// (`host=h1,h2 port=5432,5433`, `config.go:326-362`), zipped by `buildLegacyHostList`.
372+
const hostString =
215373
params.get("host") ?? params.get("hostaddr") ?? libpqEnv("PGHOST") ?? defaultLibpqHost();
216-
const portRaw = params.get("port");
217-
const port =
218-
portRaw !== undefined && portRaw.length > 0 ? Number(portRaw) : libpqPort(libpqEnv("PGPORT"));
219-
// Explicit non-numeric `port=` → NaN; a bad `PGPORT` fallback → undefined.
220-
if (port === undefined || Number.isNaN(port)) return undefined;
374+
// Explicit empty/non-numeric `port=` is a parse error (pgconn's `parsePort`); an
375+
// absent `port` falls back to `PGPORT` and then the libpq default.
376+
const portParam = params.get("port");
377+
let portString: string;
378+
if (portParam !== undefined) {
379+
portString = portParam;
380+
} else {
381+
const envPort = libpqPort(libpqEnv("PGPORT"));
382+
if (envPort === undefined) return undefined;
383+
portString = String(envPort);
384+
}
385+
const hostList = buildLegacyHostList(hostString, portString);
386+
if (hostList === undefined || hostList.length === 0) return undefined;
387+
const primary = hostList[0]!;
221388
const user = params.get("user") ?? defaultOsUser();
222389
const database =
223390
params.get("dbname") ?? libpqEnv("PGDATABASE") ?? (user.length > 0 ? user : "postgres");
@@ -227,17 +394,22 @@ function parseKeywordValueDsn(value: string): LegacyPgConnInput | undefined {
227394
if (isInvalidSslmode(sslmode)) return undefined;
228395
const sslrootcert = params.get("sslrootcert") ?? libpqEnv("PGSSLROOTCERT");
229396
const options = params.get("options");
230-
// Password precedence (pgconn): connection string → PGPASSWORD → `.pgpass`.
231-
const password =
232-
params.get("password") ??
233-
libpqEnv("PGPASSWORD") ??
234-
legacyPgpassPassword(host, port, database, user);
397+
// Password precedence (pgconn): a `password=` entry — even empty — overrides
398+
// PGPASSWORD; an empty resolved value then falls through to `.pgpass`.
399+
const password = resolveLibpqPassword(
400+
params.has("password") ? params.get("password")! : undefined,
401+
primary.host,
402+
primary.port,
403+
database,
404+
user,
405+
);
235406
return {
236-
host,
237-
port,
407+
host: primary.host,
408+
port: primary.port,
238409
user,
239410
password,
240411
database,
412+
...(hostList.length > 1 ? { fallbacks: hostList.slice(1) } : {}),
241413
...(options !== undefined && options.length > 0 ? { options } : {}),
242414
...(sslmode !== undefined && sslmode.length > 0 ? { sslmode } : {}),
243415
...(sslrootcert !== undefined && sslrootcert.length > 0 ? { sslrootcert } : {}),

0 commit comments

Comments
 (0)