Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/js/internal/sql/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,14 @@ function parseOptions(
username ||= options.user || options.username || decodeIfValid(url.username);
password ||= options.pass || options.password || decodeIfValid(url.password);

path ||= options.path || url.pathname;
// Only use url.pathname as the Unix socket path for unix:// URLs.
// For postgres://, mysql://, etc., the pathname is the database name (e.g. "/mydb"),
// not a socket path. Using it as the socket path causes FailedToOpenSocket (#27713).
if (url.protocol === "unix:") {
path ||= options.path || url.pathname;
} else {
path ||= options.path;
}
Comment on lines +620 to +627
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 Pre-existing issue: For unix:// URLs, the database resolution fallback at line 732 still uses decodeIfValid((url?.pathname ?? "").slice(1)) without a unix: protocol guard. This means new SQL("unix:///var/run/postgresql", { adapter: "postgres" }) without an explicit database option would incorrectly set the database name to "var/run/postgresql". Consider adding the same url.protocol === "unix:" guard used for path (line 623) to the database fallback.

Extended reasoning...

What the bug is

The PR correctly fixes the path variable so that url.pathname is only used as a Unix socket path for unix:// protocol URLs (line 623). However, further down in the function, the database name resolution for postgres (line 732), mysql (line 743), and mariadb still uses decodeIfValid((url?.pathname ?? "").slice(1)) as a fallback for all protocols, including unix:.

Concrete example

Consider: new SQL("unix:///var/run/postgresql", { adapter: "postgres" })

  1. The URL is parsed: url.protocol = "unix:", url.pathname = "/var/run/postgresql"
  2. The PR correctly sets path = "/var/run/postgresql" (the socket path) because url.protocol === "unix:"
  3. When resolving the database name at line 727-733, the fallback chain is evaluated:
    • options.database is undefined
    • options.db is undefined
    • env.PG_DATABASE is undefined
    • env.PGDATABASE is undefined
    • decodeIfValid("/var/run/postgresql".slice(1)) evaluates to "var/run/postgresql"this is the bug
  4. The database name is set to "var/run/postgresql", which is the socket path minus the leading slash, not a valid database name.

Why existing code does not prevent it

The url.protocol === "unix:" guard was only added to the path resolution (line 623), not to the database resolution (lines 725-745). The database fallback chain unconditionally reads url.pathname regardless of protocol.

Impact

In practice, most users of unix:// URLs will explicitly specify a database option, which short-circuits the fallback chain. However, if they omit it, they will get a confusing connection error about a non-existent database named after their socket path. The practical impact is limited but the inconsistency with the path fix is worth addressing.

How to fix

Add a similar url.protocol === "unix:" guard to the database resolution. For example, replace decodeIfValid((url?.pathname ?? "").slice(1)) with (url?.protocol !== "unix:" ? decodeIfValid((url?.pathname ?? "").slice(1)) : null) in all three adapter cases (postgres at line 732, mysql at line 743, mariadb).

Pre-existing status

This is a pre-existing issue. The database resolution code at lines 725-745 was not modified by this PR. The url.pathname fallback for database names has always applied to all protocols. However, since this PR makes unix:// URLs more functional by fixing the socket path issue, it creates a natural place to also address this related problem. As one verifier noted, the typical unix:// usage pattern involves explicitly specifying the database option, so the practical impact is limited.


const queryObject = url.searchParams.toJSON();
for (const key in queryObject) {
Expand Down
128 changes: 128 additions & 0 deletions test/regression/issue/27713.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
import { SQL } from "bun";
import { afterAll, beforeEach, describe, expect, test } from "bun:test";
import { isWindows } from "harness";

// Regression test for https://github.com/oven-sh/bun/issues/27713
// Bun SQL was treating the Postgres URL path component (the database name)
// as a Unix domain socket path, causing FailedToOpenSocket on any URL with
// a database name.

describe("SQL should not treat URL pathname as Unix socket path (#27713)", () => {
const originalEnv = { ...process.env };

// prettier-ignore
const SQL_ENV_VARS = [
"DATABASE_URL", "DATABASEURL",
"TLS_DATABASE_URL",
"POSTGRES_URL", "PGURL", "PG_URL",
"TLS_POSTGRES_DATABASE_URL",
"MYSQL_URL", "MYSQLURL",
"TLS_MYSQL_DATABASE_URL",
"PGHOST", "PGUSER", "PGPASSWORD", "PGDATABASE", "PGPORT",
"PG_HOST", "PG_USER", "PG_PASSWORD", "PG_DATABASE", "PG_PORT",
"MYSQL_HOST", "MYSQL_USER", "MYSQL_PASSWORD", "MYSQL_DATABASE", "MYSQL_PORT",
];

beforeEach(() => {
for (const key of SQL_ENV_VARS) {
delete process.env[key];
delete Bun.env[key];
delete import.meta.env[key];
}
});

afterAll(() => {
for (const key of SQL_ENV_VARS) {
if (key in originalEnv) {
process.env[key] = originalEnv[key]!;
Bun.env[key] = originalEnv[key]!;
import.meta.env[key] = originalEnv[key]!;
} else {
delete process.env[key];
delete Bun.env[key];
delete import.meta.env[key];
}
}
});

test("postgres URL with database name should not set path", () => {
const sql = new SQL("postgres://user:pass@myhost:5432/mydb");
expect(sql.options.hostname).toBe("myhost");
expect(sql.options.port).toBe(5432);
expect(sql.options.database).toBe("mydb");
// path must not be the database name "/mydb"
expect(sql.options.path).toBeUndefined();
});

test("postgres URL passed via url option should not set path", () => {
const sql = new SQL({
url: "postgres://user:pass@myhost:5432/mydb",
});
expect(sql.options.hostname).toBe("myhost");
expect(sql.options.port).toBe(5432);
expect(sql.options.database).toBe("mydb");
expect(sql.options.path).toBeUndefined();
});

test("DATABASE_URL with database name should not set path when using explicit options", () => {
process.env.DATABASE_URL = "postgres://user:pass@envhost:5432/envdb";

const sql = new SQL({
hostname: "myhost",
port: 5432,
username: "user",
password: "pass",
database: "mydb",
});

expect(sql.options.hostname).toBe("myhost");
expect(sql.options.database).toBe("mydb");
// path must not be "/envdb" from DATABASE_URL
expect(sql.options.path).toBeUndefined();
});

test("DATABASE_URL with database name should not set path when used implicitly", () => {
process.env.DATABASE_URL = "postgres://user:pass@envhost:5432/envdb";

const sql = new SQL();
expect(sql.options.hostname).toBe("envhost");
expect(sql.options.port).toBe(5432);
expect(sql.options.database).toBe("envdb");
// path must not be "/envdb"
expect(sql.options.path).toBeUndefined();
});

test("postgres URL with database name matching existing directory should not set path", () => {
// This is the actual bug: when the URL pathname matches an existing filesystem
// path (like /tmp), the old code would pass it as a Unix socket path.
// The database name in postgres://.../<dbname> is "/tmp" here, which exists.
const sql = new SQL("postgres://user:pass@myhost:5432/tmp");
expect(sql.options.hostname).toBe("myhost");
expect(sql.options.port).toBe(5432);
expect(sql.options.database).toBe("tmp");
// Before the fix, this would be "/tmp" (or "/tmp/.s.PGSQL.5432" if that exists),
// causing the connection to use Unix domain socket instead of TCP.
expect(sql.options.path).toBeUndefined();
});

test("mysql URL with database name should not set path", () => {
const sql = new SQL("mysql://user:pass@myhost:3306/mydb");
expect(sql.options.hostname).toBe("myhost");
expect(sql.options.port).toBe(3306);
expect(sql.options.database).toBe("mydb");
expect(sql.options.path).toBeUndefined();
});

test.skipIf(isWindows)("unix:// protocol should still use pathname as socket path", () => {
const socketPath = `/tmp/bun-test-27713-${process.pid}.sock`;
using sock = Bun.listen({
unix: socketPath,
socket: {
data: () => {},
},
});

const sql = new SQL(`unix://${sock.unix}`, { adapter: "postgres" });
expect(sql.options.path).toBe(socketPath);
});
});