Skip to content

sql: serialize Date parameters as ISO 8601 in Postgres text format - #29013

Closed
robobun wants to merge 5 commits into
mainfrom
farm/f0d905df/sql-date-iso-serialize
Closed

sql: serialize Date parameters as ISO 8601 in Postgres text format#29013
robobun wants to merge 5 commits into
mainfrom
farm/f0d905df/sql-date-iso-serialize

guard @intFromFloat(NaN) in types.date.fromJS; test invalid Date on b…

d29f00c
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed May 1, 2026 in 33m 19s

Code review found 1 important issue

Found 4 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 1
🟡 Nit 0
🟣 Pre-existing 1
Severity File:Line Issue
🔴 Important src/sql/postgres/PostgresRequest.zig:165-168 InvalidQueryBinding mid-write leaves partial Bind bytes in connection write_buffer
🟣 Pre-existing src/sql/postgres/types/date.zig:31 Pre-existing: MySQL DateTime.fromJS / Time.fromJS have the same unguarded @intFromFloat(NaN) bug

Annotations

Check failure on line 168 in src/sql/postgres/PostgresRequest.zig

See this annotation in the file changed.

@claude claude / Claude Code Review

InvalidQueryBinding mid-write leaves partial Bind bytes in connection write_buffer

The new `return error.InvalidQueryBinding` paths (here and in `types.date.fromJS`) fire *after* `writeBind` has already appended the 'B' tag, length placeholder, names, format codes, and prior parameter values to `connection.write_buffer` — and the catch blocks in `PostgresSQLConnection.advance()` never truncate `write_buffer` on failure. So a user who binds `new Date('bad')`, catches the rejection, and issues another query on the same connection will have that query's bytes appended after a par

Check notice on line 31 in src/sql/postgres/types/date.zig

See this annotation in the file changed.

@claude claude / Claude Code Review

Pre-existing: MySQL DateTime.fromJS / Time.fromJS have the same unguarded @intFromFloat(NaN) bug

Pre-existing (not introduced by this PR, follow-up only): the same `@intFromFloat(NaN)` bug this guard fixes also exists, unguarded, in the MySQL driver — `src/sql/mysql/MySQLTypes.zig` `DateTime.fromJS` (lines 643–656) and `Time.fromJS` (lines 670–684) call `getUnixTimestamp()` / `asNumber()` and pass the result straight to `@intFromFloat` with no `std.math.isFinite` check, so binding `new Date('bad')` / `NaN` / `Infinity` triggers Zig Illegal Behavior (panic in safe builds, UB in release). Wor