postgres: validate binary datum length and range before decoding - #33576
Open
robobun wants to merge 7 commits into
Open
postgres: validate binary datum length and range before decoding#33576robobun wants to merge 7 commits into
robobun wants to merge 7 commits into
Claude / Claude Code Review
completed
Jul 7, 2026 in 14m 58s
Code review found 2 potential issues
Found 2 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/sql_jsc/postgres/DataCell.rs:847-856 |
0-byte binary timestamp/time datum returns null instead of InvalidBinaryData |
| 🟡 Nit | test/js/sql/postgres-binary-datum-validation.test.ts:97-103 |
float4 binary length validation has no test coverage |
Annotations
Check warning on line 856 in src/sql_jsc/postgres/DataCell.rs
claude / Claude Code Review
0-byte binary timestamp/time datum returns null instead of InvalidBinaryData
Minor consistency gap: the pre-existing `if bytes.is_empty() { return Ok(SQLDataCell::null()) }` runs before the new `bytes.len() != 8` guard here (and in the `T::time | T::timetz` arm), so a 0-byte binary timestamp still surfaces as `null` instead of `InvalidBinaryData` — unlike the bool arm, which this PR now rejects for len=0. Consider moving the empty check inside the text branch, or checking `binary` first.
Check warning on line 103 in test/js/sql/postgres-binary-datum-validation.test.ts
claude / Claude Code Review
float4 binary length validation has no test coverage
The `malformed` table has no `float4` case, even though the PR adds an explicit `bytes.len() != 4` guard for `T::float4` and lists it in the fix table. Consider adding a row like `{ name: "float4 with 2-byte datum", oid: 700, col: i16(0), code: /ERR_POSTGRES_INVALID_BINARY_DATA/ }` alongside the `float8` case so both siblings are covered.
Loading