sql(mysql): fix truncated Time::to_binary when microseconds != 0

[robobun]

What

Time::to_binary (the MySQL TIME binary-parameter encoder) writes 13 bytes when microseconds != 0 but returned 12, so Value::to_data copied buffer[0..12] and dropped buffer[12] (the high byte of the little-endian u32 microseconds). The length prefix in buffer[0] is 12, so the packet would advertise 12 payload bytes but carry only 11, desynchronizing the COM_STMT_EXECUTE stream the moment this encoder runs.

} else {
    buffer[0] = 12; // length
    buffer[9..13].copy_from_slice(&self.microseconds.to_le_bytes());
    12   // ← writes buffer[0..=12] but returns 12; should be 13
}

The microseconds == 0 branch (writes 9, returns 9) and DateTime::to_binary's DATETIME-with-microseconds branch (writes 12, returns 12) are already consistent; only this branch is off by one. The same bug was present in the pre-Rust Zig implementation.

Why this has no JS test

This encode path is currently unreachable from JS. Value::Time is only constructed when Value::from_js is called with FieldType::MYSQL_TYPE_TIME, and the only source of that field type is Signature::generate → field_type_from_js, which maps a JS Date to MYSQL_TYPE_DATETIME (never TIME). The server-reported param types (statement.params, filled from the StmtPrepareOK column definitions) are decoded but only used for count checks; they never feed back into encoding. So today a Date bound to a TIME(6) column is sent as a DATETIME and coerced server-side, and Time::to_binary never executes.

A fail-before JS test is therefore not constructible, and cargo test -p bun_sql_jsc cannot link standalone (the crate transitively pulls in Windows link stubs on Linux), so a Rust unit test would not run either. The fix is applied so whoever later wires up TIME parameter binding (for example by honouring the server-reported param type) does not inherit a latent protocol desync.

Spotted during code review on #34452.

[robobun]

^{Updated 10:47 PM PT - Jul 16th, 2026}

@robobun, your commit 51a17bd is building: #74444

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 78ea9834-df85-4a78-9894-86fc2372b0c0

📥 Commits

Reviewing files that changed from the base of the PR and between 6352b79 and 51a17bd.

📒 Files selected for processing (1)

• src/sql_jsc/mysql/MySQLValue.rs

---

Walkthrough

Changes

MySQL TIME encoding

Layer / File(s)	Summary
Fractional TIME byte count src/sql_jsc/mysql/MySQLValue.rs	Time::to_binary now returns 13 when fractional microseconds occupy buffer[9..13], matching the encoded byte count.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is detailed, but it doesn't follow the template headings and lacks a clear 'How did you verify your code works?' section.	Rename the sections to match the template and add a concise verification/testing summary, even if it only explains why no runtime test was possible.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately summarizes the main fix to MySQL TIME binary encoding.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status: one-character correctness fix in an encoder that is currently unreachable from JS (see the PR body for the call-chain analysis). No fail-before JS test is constructible, and cargo test -p bun_sql_jsc cannot link standalone, so this needs a maintainer to merge on the strength of the analysis rather than a gate proof. cargo check -p bun_sql_jsc passes.

[claude]

LGTM — one-byte off-by-one fix, verifiably correct by inspection.

What was reviewed:

• Byte accounting: buffer[0..=12] is written (length prefix + 12 payload bytes), so returning 13 matches; the sibling microseconds == 0 branch (len=8, return 9) and DateTime::to_binary (len=11, return 12) follow the same length+1 pattern.
• to_data's stack buffer is [0u8; 15], so the new 13-byte slice is in bounds.
• Confirmed the no-test justification: field_type_from_js never yields MYSQL_TYPE_TIME, so this encoder is currently unreachable from JS.

Extended reasoning...

Overview

Single-character change in src/sql_jsc/mysql/MySQLValue.rs: the return value of Time::to_binary's microseconds-present branch changes from 12 to 13. The function writes a 1-byte length prefix (buffer[0] = 12) followed by 12 payload bytes (negative flag, 4-byte days, hours, minutes, seconds, 4-byte microseconds) into buffer[0..=12], so the caller (Value::to_data, which does Data::create(&buffer[0..pos])) needs pos = 13 to copy the full record. Returning 12 would drop the high byte of the little-endian microseconds and leave the on-wire record one byte short of its own length prefix.

Security risks

None. This is an internal binary-protocol encoder for MySQL TIME parameters; no untrusted input parsing, auth, or crypto is touched. The only bounds concern — whether index 12 fits in the caller's buffer — is satisfied ([0u8; 15]).

Level of scrutiny

Low. The fix is mechanical and self-evidently consistent with the two adjacent branches: microseconds == 0 writes 9 bytes and returns 9; DateTime::to_binary's 11-byte DATETIME branch writes 12 and returns 12. The MySQL binary protocol spec for TIME (length-prefixed 8 or 12 payload bytes) matches. There are no other call sites or sibling encoders sharing this bug.

Other factors

The PR ships without a test, which normally blocks approval per the repo's review rules. However, the description gives a concrete, checkable reason: Value::Time is only constructed via Value::from_js with FieldType::MYSQL_TYPE_TIME, and field_type_from_js (the only source of param field types via Signature::generate) maps JS Date to MYSQL_TYPE_DATETIME, never TIME. I verified this in the preloaded source. The code path is therefore dead today, a fail-before test is not constructible, and the crate can't host a standalone Rust unit test. Given the fix is a one-byte constant that makes an unreachable encoder internally consistent, deferring on the test requirement is reasonable here.

[robobun]

Closing: this cannot carry a regression test.

The Time::to_binary encode path is provably unreachable from JS today. Value::Time is only constructed when Value::from_js is called with FieldType::MYSQL_TYPE_TIME, and the sole source of that field type (Signature::generate → field_type_from_js, src/sql_jsc/mysql/MySQLValue.rs:18) maps a JS Date to MYSQL_TYPE_DATETIME, never TIME. The server-reported param types filled into statement.params at MySQLConnection.rs:1176 are read only for .len() count checks and never feed into encoding. cargo test -p bun_sql_jsc cannot link standalone either (transitive Windows link stubs on Linux), so a Rust unit test would not run.

The off-by-one itself is real: the branch writes buffer[0..=12] (length prefix 12 + 12 payload bytes) but returned 12, so Value::to_data would slice buffer[0..12] and drop the high byte of the microseconds. Whoever wires up TIME parameter binding (for example by honouring the server-reported param type instead of the JS-inferred one) will need this one-character change, and at that point it becomes JS-observable and testable.

The diff stays on the branch farm/1c0c56f5/mysql-time-encode-truncation for reference.