Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 4 additions & 1 deletion src/js/internal/sql/mysql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,11 @@
return getHelperCommandFromDetect(query, true);
}

isUpsertUpdate(query: string): boolean {
return query.trimEnd().endsWith("ON DUPLICATE KEY UPDATE");
// SQL keywords are case-insensitive, so match any spelling of the
// suffix; only uppercase the tail, the query can be large
const UPSERT_SUFFIX = "ON DUPLICATE KEY UPDATE";
return query.trimEnd().slice(-UPSERT_SUFFIX.length).toUpperCase() === UPSERT_SUFFIX;

Check notice on line 286 in src/js/internal/sql/mysql.ts

View check run for this annotation

Claude / Claude Code Review

isUpsertUpdate still requires exact single-space whitespace between keywords

nit (pre-existing, non-blocking): this still requires exactly one ASCII space between each keyword. `detectCommand()` tokenizes on any whitespace, so `... ON DUPLICATE KEY\n UPDATE ${sql(data)}` (or double spaces / tabs) is detected as `SQLCommand.update`, fails this 23-char tail match, and gets the same spurious ` SET ` this PR fixes for lowercase. Since this line is being rewritten anyway, testing the trimmed tail against `/on\s+duplicate\s+key\s+update$/i` would close the whole class — but t
Comment thread
robobun marked this conversation as resolved.
Outdated
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

Expand Down
155 changes: 155 additions & 0 deletions test/js/sql/sql-mysql-upsert-keyword-case.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
// The MySQL helper normalizer appends " SET " after an update helper unless the
// query already ends with ON DUPLICATE KEY UPDATE. SQL keywords are
// case-insensitive, so the suffix check must be too; a lowercase spelling used
// to produce "... on duplicate key update SET `age` = ?", which is invalid.
// https://github.com/oven-sh/bun/issues/32035
//
// Uses a minimal mock MySQL server that captures the text of every
// COM_STMT_PREPARE and replies with an error packet, so the test can assert
// the exact generated SQL without Docker.

import { SQL } from "bun";
import { expect, test } from "bun:test";
import { once } from "events";
import net from "net";

function u16le(n: number) {
return Buffer.from([n & 0xff, (n >> 8) & 0xff]);
}
function u24le(n: number) {
return Buffer.from([n & 0xff, (n >> 8) & 0xff, (n >> 16) & 0xff]);
}
function u32le(n: number) {
return Buffer.from([n & 0xff, (n >> 8) & 0xff, (n >> 16) & 0xff, (n >>> 24) & 0xff]);
}
function packet(seq: number, payload: Buffer) {
return Buffer.concat([u24le(payload.length), Buffer.from([seq]), payload]);
}

// Server capability flags (subset sufficient for the prepared-statement path).
const CLIENT_PROTOCOL_41 = 1 << 9;
const CLIENT_SECURE_CONNECTION = 1 << 15;
const CLIENT_PLUGIN_AUTH = 1 << 19;
const CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA = 1 << 21;
const CLIENT_DEPRECATE_EOF = 1 << 24;
const SERVER_CAPS =
CLIENT_PROTOCOL_41 |
CLIENT_SECURE_CONNECTION |
CLIENT_PLUGIN_AUTH |
CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA |
CLIENT_DEPRECATE_EOF;

function handshakeV10() {
const authData1 = Buffer.alloc(8, 0x61);
const authData2 = Buffer.alloc(13, 0x62); // includes trailing NUL as part of 13 bytes
authData2[12] = 0;
const payload = Buffer.concat([
Buffer.from([10]), // protocol version
Buffer.from("mock-5.7.0\0"), // server version NUL-terminated
u32le(1), // connection id
authData1, // auth-plugin-data-part-1 (8)
Buffer.from([0]), // filler
u16le(SERVER_CAPS & 0xffff), // capability flags lower
Buffer.from([0x2d]), // character set (utf8mb4_general_ci)
u16le(0x0002), // status flags (SERVER_STATUS_AUTOCOMMIT)
u16le((SERVER_CAPS >>> 16) & 0xffff), // capability flags upper
Buffer.from([21]), // length of auth-plugin-data
Buffer.alloc(10, 0), // reserved
authData2, // auth-plugin-data-part-2 (13 bytes)
Buffer.from("mysql_native_password\0"),
]);
return packet(0, payload);
}

function okPacket(seq: number) {
// header, affected_rows (lenenc 0), last_insert_id (lenenc 0), status flags, warnings
return packet(seq, Buffer.from([0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00]));
}

function errorPacket(seq: number, errno: number, message: string) {
const payload = Buffer.concat([Buffer.from([0xff]), u16le(errno), Buffer.from("#42000"), Buffer.from(message)]);
return packet(seq, payload);
}

const COM_QUERY = 0x03;
const COM_STMT_PREPARE = 0x16;

test("upsert helper omits SET for every spelling of ON DUPLICATE KEY UPDATE", async () => {
const prepared: string[] = [];

const server = net.createServer(socket => {
let buffered = Buffer.alloc(0);
let authed = false;

socket.write(handshakeV10());

socket.on("data", chunk => {
buffered = Buffer.concat([buffered, chunk]);
while (buffered.length >= 4) {
const len = buffered[0] | (buffered[1] << 8) | (buffered[2] << 16);
if (buffered.length < 4 + len) break;
const seq = buffered[3];
const payload = buffered.subarray(4, 4 + len);
buffered = buffered.subarray(4 + len);

if (!authed) {
// HandshakeResponse41 from client → accept unconditionally.
authed = true;
socket.write(okPacket(seq + 1));
continue;
}

const cmd = payload[0];
if (cmd === COM_STMT_PREPARE) {
// Capture the normalized query text, then fail the statement; the
// assertion below is on the generated SQL, not on execution.
prepared.push(payload.subarray(1).toString("utf-8"));
socket.write(errorPacket(seq + 1, 1064, "mock: rejecting every prepare"));
} else if (cmd === COM_QUERY) {
socket.write(okPacket(seq + 1));
} else {
// COM_QUIT or anything else → close.
socket.end();
}
}
});
});

server.listen(0, "127.0.0.1");
await once(server, "listening");
const { port } = server.address() as net.AddressInfo;

try {
await using sql = new SQL({ url: `mysql://root@127.0.0.1:${port}/db`, max: 1 });

const row = { id: 1, age: 30 };
const update = { age: 31 };

// Same upsert with three spellings of the keyword; each must reach the
// server (errno 1064 proves the round-trip happened).
const errUpper = await sql`INSERT INTO users ${sql(row)} ON DUPLICATE KEY UPDATE ${sql(update)}`.catch(
(e: any) => e,
);
expect(errUpper?.errno).toBe(1064);

const errLower = await sql`INSERT INTO users ${sql(row)} on duplicate key update ${sql(update)}`.catch(
(e: any) => e,
);
expect(errLower?.errno).toBe(1064);

const errMixed = await sql`INSERT INTO users ${sql(row)} On Duplicate Key Update ${sql(update)}`.catch(
(e: any) => e,
);
expect(errMixed?.errno).toBe(1064);

// No "SET" after the keyword in any spelling; the update helper's columns
// follow it directly.
expect(prepared).toEqual([
"INSERT INTO users (`id`, `age`) VALUES(?, ?) ON DUPLICATE KEY UPDATE `age` = ? ",
"INSERT INTO users (`id`, `age`) VALUES(?, ?) on duplicate key update `age` = ? ",
"INSERT INTO users (`id`, `age`) VALUES(?, ?) On Duplicate Key Update `age` = ? ",
]);
} finally {
await new Promise<void>(r => server.close(() => r()));
}
});
36 changes: 36 additions & 0 deletions test/js/sql/sql-mysql.helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,42 @@ describeWithContainer(
expect(result[0].email).toBe("bunny2@bun.com");
}
});

// SQL keywords are case-insensitive; lowercase spellings must not get an
// extra SET appended. https://github.com/oven-sh/bun/issues/32035
test("upsert helper with lowercase on duplicate key update", async () => {
await using sql = new SQL({ ...getOptions(), max: 1 });
const random_name = "test_" + randomUUIDv7("hex").replaceAll("-", "");
await sql`
CREATE TABLE IF NOT EXISTS ${sql(random_name)} (
id int PRIMARY KEY,
foo text NOT NULL,
email VARCHAR(255) NOT NULL UNIQUE
)
`;

await sql`INSERT INTO ${sql(random_name)} ${sql({ id: 1, foo: "hello", email: "bunny@bun.com" })}`;

{
const data = { foo: "hello2", email: "bunny2@bun.com" };
await sql`
INSERT INTO ${sql(random_name)} ${sql({ id: 1, ...data })}
on duplicate key update ${sql(data)}
`;
const result = await sql`SELECT * FROM ${sql(random_name)}`;
expect(result).toEqual([{ id: 1, foo: "hello2", email: "bunny2@bun.com" }]);
}

{
const data = { foo: "hello3", email: "bunny3@bun.com" };
await sql`
INSERT INTO ${sql(random_name)} ${sql({ id: 1, ...data })}
On Duplicate Key Update ${sql(data)}
`;
const result = await sql`SELECT * FROM ${sql(random_name)}`;
expect(result).toEqual([{ id: 1, foo: "hello3", email: "bunny3@bun.com" }]);
}
});
test("update helper with IN and column name", async () => {
await using sql = new SQL({ ...getOptions(), max: 1 });
const random_name = "test_" + randomUUIDv7("hex").replaceAll("-", "");
Expand Down
Loading