Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
72 changes: 69 additions & 3 deletions src/sql/mysql/MySQLTypes.zig
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,68 @@
return fromBinary(data.slice());
}

/// Parse MySQL's text-protocol DATE/DATETIME/TIMESTAMP representation:
/// "YYYY-MM-DD"
/// "YYYY-MM-DD HH:MM:SS"
/// "YYYY-MM-DD HH:MM:SS.ffffff" (1-6 fractional digits)
///
/// MySQL TIMESTAMP values are returned in the server's session TZ; DATETIME
/// is a naive wall-clock. Both are returned without a TZ designator, and
/// Bun interprets them as UTC components so the resulting JS `Date`
/// round-trips the bytes the server sent, regardless of the client TZ.
pub fn fromText(text: []const u8) !DateTime {
if (text.len < 10) return error.InvalidDateTimeText;

const year = std.fmt.parseInt(u16, text[0..4], 10) catch return error.InvalidDateTimeText;
if (text[4] != '-') return error.InvalidDateTimeText;
const month = std.fmt.parseInt(u8, text[5..7], 10) catch return error.InvalidDateTimeText;
if (text[7] != '-') return error.InvalidDateTimeText;
const day = std.fmt.parseInt(u8, text[8..10], 10) catch return error.InvalidDateTimeText;

// Reject MySQL zero-date sentinels like "0000-00-00" and impossible
// calendar values (e.g. 2024-02-31) so the caller produces NaN,
// matching the behaviour of the pre-existing JS-parser path.
// Otherwise JSC's GregorianDateTime would silently normalize them
// into bogus timestamps instead of an Invalid Date.
if (month < 1 or month > 12) return error.InvalidDateTimeText;
if (day < 1 or day > daysInMonth(year, month)) return error.InvalidDateTimeText;

var result: DateTime = .{ .year = year, .month = month, .day = day };
if (text.len == 10) return result;

// Either "YYYY-MM-DD HH:MM:SS" or "YYYY-MM-DDTHH:MM:SS" (ISO-style).
if (text.len < 19 or (text[10] != ' ' and text[10] != 'T')) return error.InvalidDateTimeText;

result.hour = std.fmt.parseInt(u8, text[11..13], 10) catch return error.InvalidDateTimeText;
if (text[13] != ':') return error.InvalidDateTimeText;
result.minute = std.fmt.parseInt(u8, text[14..16], 10) catch return error.InvalidDateTimeText;
if (text[16] != ':') return error.InvalidDateTimeText;
result.second = std.fmt.parseInt(u8, text[17..19], 10) catch return error.InvalidDateTimeText;
// Same rationale as the date checks above. MySQL's strict modes
// reject these values, but permissive modes will happily store
// them.
if (result.hour > 23 or result.minute > 59 or result.second > 59) {
return error.InvalidDateTimeText;
}

if (text.len == 19) return result;
if (text[19] != '.') return error.InvalidDateTimeText;
Comment thread
robobun marked this conversation as resolved.

// Fractional seconds: up to 6 digits, right-padded to microseconds.
const frac = text[20..];
if (frac.len == 0 or frac.len > 6) return error.InvalidDateTimeText;
var micro: u32 = 0;
for (frac) |c| {
if (c < '0' or c > '9') return error.InvalidDateTimeText;
micro = micro * 10 + (c - '0');
}
var pad: usize = 6 - frac.len;
while (pad > 0) : (pad -= 1) micro *= 10;
result.microsecond = micro;
Comment thread
coderabbitai[bot] marked this conversation as resolved.

return result;
}

pub fn fromBinary(val: []const u8) DateTime {
switch (val.len) {
4 => {
Expand Down Expand Up @@ -598,13 +660,17 @@
},
else => return 0,
}
}

pub fn toJSTimestamp(this: *const DateTime, globalObject: *JSC.JSGlobalObject) bun.JSError!f64 {
return globalObject.gregorianDateTimeToMS(
// MySQL's binary DATETIME/TIMESTAMP protocol encodes raw
// year/month/day/hour/minute/second/microsecond components with no
// timezone information. Interpret them as UTC so the resulting JS
// `Date` has the correct UTC epoch regardless of the process TZ.
return globalObject.gregorianDateTimeToMSUTC(
this.year,
this.month,
this.day,

Check notice on line 673 in src/sql/mysql/MySQLTypes.zig

View check run for this annotation

Claude / Claude Code Review

gregorianDate() panics on pre-1970 dates (write path)

Pre-existing bug: `gregorianDate()` only iterates forward from 1970, so any negative `days` value (representing a pre-1970 date) skips both while-loops entirely and attempts `@intCast(d + 1)` to `u8` where `d` is still the original large negative number — a panic in Zig Debug/ReleaseSafe mode and silent garbage in ReleaseFast. Any INSERT of a pre-1970 JS Date (e.g. `new Date('1900-01-01')`) via Bun MySQL will trigger this through the `fromJS() → fromUnixTimestamp() → gregorianDate()` call chain.
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
this.hour,
this.minute,
this.second,
Expand Down Expand Up @@ -635,8 +701,8 @@
};
}

pub fn toJS(this: DateTime, globalObject: *JSC.JSGlobalObject) JSValue {
return JSValue.fromDateNumber(globalObject, this.toJSTimestamp());
pub fn toJS(this: DateTime, globalObject: *JSC.JSGlobalObject) bun.JSError!JSValue {
return JSValue.fromDateNumber(globalObject, try this.toJSTimestamp(globalObject));
}

pub fn fromJS(value: JSValue, globalObject: *JSC.JSGlobalObject) !DateTime {
Expand Down
6 changes: 5 additions & 1 deletion src/sql/mysql/protocol/DecodeBinaryValue.zig
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,14 @@
},
.MYSQL_TYPE_DATE, .MYSQL_TYPE_TIMESTAMP, .MYSQL_TYPE_DATETIME => switch (try reader.byte()) {
0 => {
return SQLDataCell{ .tag = .date, .value = .{ .date = 0 } };
// MySQL's binary protocol sends a zero-length payload for
// zero-date sentinels like '0000-00-00'. Return NaN so the
// JS side sees an Invalid Date, matching the text-protocol
// path instead of surfacing the epoch (1970-01-01T00:00:00Z).
return SQLDataCell{ .tag = .date, .value = .{ .date = std.math.nan(f64) } };
},
11, 7, 4 => |l| {
var data = try reader.read(l);

Check failure on line 135 in src/sql/mysql/protocol/DecodeBinaryValue.zig

View check run for this annotation

Claude / Claude Code Review

Partial zero-dates in binary protocol normalize instead of NaN

The binary protocol path for partial zero-dates like '2024-00-15' (month=0) or '2024-01-00' (day=0) silently normalizes them to wrong timestamps instead of NaN. MySQL sends these as non-zero-length 4/7/11-byte payloads, bypassing the zero-length NaN guard added by this PR; fromBinary() reads month/day with no range validation, so toJSTimestamp() calls gregorianDateTimeToMSUTC(year, 0, day) which triggers setMonth(-1) in WTF::GregorianDateTime and normalizes to December of year-1. Meanwhile fromT
Comment thread
robobun marked this conversation as resolved.
defer data.deinit();
const time = try DateTime.fromData(&data);
return SQLDataCell{ .tag = .date, .value = .{ .date = try time.toJSTimestamp(globalObject) } };
Expand Down
12 changes: 9 additions & 3 deletions src/sql/mysql/protocol/ResultSet.zig
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,15 @@ pub const Row = struct {
cell.* = SQLDataCell{ .tag = .string, .value = .{ .string = if (slice.len > 0) bun.String.cloneUTF8(slice).value.WTFStringImpl else null }, .free_value = 1 };
},
.MYSQL_TYPE_DATE, .MYSQL_TYPE_DATETIME, .MYSQL_TYPE_TIMESTAMP => {
var str = bun.String.init(value.slice());
defer str.deref();
// MySQL text protocol returns naive "YYYY-MM-DD[ HH:MM:SS[.ffffff]]"
// values with no timezone designator. Parse the components ourselves
// and interpret them as UTC, matching the binary-protocol path. Using
// the generic JS date parser would treat them as local time and shift
// the result by the client's UTC offset (issue #29208).
const slice = value.slice();
const date = brk: {
break :brk str.parseDate(this.globalObject) catch |err| {
const dt = DateTime.fromText(slice) catch break :brk std.math.nan(f64);
break :brk dt.toJSTimestamp(this.globalObject) catch |err| {
_ = this.globalObject.takeException(err);
break :brk std.math.nan(f64);
};
Comment thread
robobun marked this conversation as resolved.
Expand Down Expand Up @@ -265,6 +270,7 @@ const Data = @import("../../shared/Data.zig").Data;
const SQLDataCell = @import("../../shared/SQLDataCell.zig").SQLDataCell;
const SQLQueryResultMode = @import("../../shared/SQLQueryResultMode.zig").SQLQueryResultMode;
const decodeLengthInt = @import("./EncodeInt.zig").decodeLengthInt;
const DateTime = @import("../MySQLTypes.zig").Value.DateTime;

const DecodeBinaryValue = @import("./DecodeBinaryValue.zig");
const decodeBinaryValue = DecodeBinaryValue.decodeBinaryValue;
Expand Down
127 changes: 127 additions & 0 deletions test/regression/issue/29208.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
// https://github.com/oven-sh/bun/issues/29208
//
// MySQL DATETIME/TIMESTAMP values were deserialized through JSC's local-time
// constructor, so on any machine whose process TZ was not UTC the returned
// JS `Date` was off by the client's UTC offset. `bun test` forces
// TZ=Etc/UTC on the test runner, which masks the bug, so we set
// process.env.TZ before decoding and round-trip both DATETIME and TIMESTAMP
// via the binary (prepared) and text (simple) protocols.

import { SQL, randomUUIDv7 } from "bun";
import { beforeAll, describe, expect, test } from "bun:test";
Comment thread
claude[bot] marked this conversation as resolved.
import { describeWithContainer, isDockerEnabled } from "harness";

const EXPECTED_ISO = "2024-01-15T05:30:45.678Z" as const;

async function runRoundTrip(url: string) {
// Apply the non-UTC TZ *before* any Date is constructed or SQL query is
// decoded — JSC's date cache reads $TZ lazily on its first use.
const savedTz = process.env.TZ;
process.env.TZ = "Asia/Bangkok";

try {
// With TZ=Asia/Bangkok (UTC+7, no DST) the local-time constructor
// interprets (2024, 0, 15, 12, 30, 45, 678) as
// 2024-01-15T12:30:45.678+07:00 = 2024-01-15T05:30:45.678Z.
const sent = new Date(2024, 0, 15, 12, 30, 45, 678);
expect(sent.toISOString()).toBe(EXPECTED_ISO);
expect(Intl.DateTimeFormat().resolvedOptions().timeZone).toBe("Asia/Bangkok");

await using sql = new SQL({ url, max: 1 });
const tableName = "ts_29208_" + randomUUIDv7("hex").replaceAll("-", "");

await sql`DROP TABLE IF EXISTS ${sql(tableName)}`;
await sql`CREATE TABLE ${sql(tableName)} (id INT PRIMARY KEY, ts DATETIME(3), tstz TIMESTAMP(3))`;

try {
await sql`INSERT INTO ${sql(tableName)} (id, ts, tstz) VALUES (${1}, ${sent}, ${sent})`;

// Binary (prepared statement) protocol.
const [bin] = (await sql`SELECT ts, tstz FROM ${sql(tableName)} WHERE id = 1`) as any[];
// Text (simple query) protocol.
const [txt] = (await sql`SELECT ts, tstz FROM ${sql(tableName)} WHERE id = 1`.simple()) as any[];

// Every column — binary and text, DATETIME and TIMESTAMP — must decode
// to the same UTC instant the client sent.
expect({
binaryDatetime: (bin.ts as Date).toISOString(),
binaryTimestamp: (bin.tstz as Date).toISOString(),
textDatetime: (txt.ts as Date).toISOString(),
textTimestamp: (txt.tstz as Date).toISOString(),
}).toEqual({
binaryDatetime: EXPECTED_ISO,
binaryTimestamp: EXPECTED_ISO,
textDatetime: EXPECTED_ISO,
textTimestamp: EXPECTED_ISO,
});
} finally {
await sql`DROP TABLE IF EXISTS ${sql(tableName)}`;
}
} finally {
if (savedTz === undefined) delete process.env.TZ;
else process.env.TZ = savedTz;
}
}

// ─── Docker path (used in CI) ───────────────────────────────────────────────
// Not `concurrent: true` — this test mutates process.env.TZ, which is global.
// Running in the default serial mode keeps the TZ flip isolated from any
// other concurrent tests.
if (isDockerEnabled()) {
describeWithContainer("issue #29208 (containerized MySQL)", { image: "mysql_plain" }, container => {
beforeAll(() => container.ready);
test("DATETIME/TIMESTAMP decode as UTC under non-UTC TZ", async () => {
await runRoundTrip(`mysql://root@${container.host}:${container.port}/bun_sql_test`);
});
});
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// ─── Local-server path (used in dev/reproduction shells without Docker) ────
//
// Detection order:
// 1. BUN_TEST_LOCAL_MYSQL_URL — explicit override.
// 2. mysql://bun_test:bun_test_pw@127.0.0.1:3306/bun_sql_test — the farm
// convention; auto-provisioned via `mysql -u root` if reachable.
//
// Skipped cleanly if neither is available.
describe("issue #29208 (local MySQL)", () => {
let resolvedUrl: string | undefined;

beforeAll(async () => {
const explicitUrl = process.env.BUN_TEST_LOCAL_MYSQL_URL;
if (explicitUrl) {
resolvedUrl = explicitUrl;
return;
}

// Idempotently auto-provision the farm-convention user. If the mysql
// CLI is missing or root isn't trusted, provisioning fails silently and
// the test becomes a no-op.
try {
await using proc = Bun.spawn({
cmd: ["mysql", "-u", "root"],
stdin: new TextEncoder().encode(
`CREATE DATABASE IF NOT EXISTS bun_sql_test;
CREATE USER IF NOT EXISTS 'bun_test'@'%' IDENTIFIED BY 'bun_test_pw';
GRANT ALL ON bun_sql_test.* TO 'bun_test'@'%';
FLUSH PRIVILEGES;`,
),
stdout: "ignore",
stderr: "ignore",
});
if ((await proc.exited) === 0) {
resolvedUrl = "mysql://bun_test:bun_test_pw@127.0.0.1:3306/bun_sql_test";
}
} catch {
// mysql CLI unavailable — no local server path, rely on Docker above.
}
});

test("DATETIME/TIMESTAMP decode as UTC under non-UTC TZ", async () => {
if (!resolvedUrl) {
// No local MySQL — skip cleanly. CI relies on the Docker path above.
return;
}
await runRoundTrip(resolvedUrl);
});
});
Loading