feat(fuzz): fuzz the driver's own parsers, and fix a silent time-zone overflow - #7
Merged
Conversation
stackable-odbc-core fuzzes `write_column_value`, which turns a `ColumnValue`
into the caller's buffer. Nothing covered the step before it, where a
coordinator's JSON becomes that `ColumnValue`, nor the type-signature parsing,
the Trino escape dialect or the connection-string value parsing. Those are this
crate's, and three of them read text chosen by the server.
Four targets in a `fuzz/` workspace, mirroring core's: `json_value`,
`type_name`, `escape` and `connect_params`. Each builds its input from a
grammar rather than from raw bytes, because a random byte string is not a
balanced `{fn CONVERT(x, SQL_INTEGER)}` and never reaches the parsers at all.
They reach the code through a new `fuzz_api` module behind a default-off
`fuzzing` feature, following how core gates `test-support`: the modules are
private on purpose, and one gated re-export keeps the fuzzed surface readable
in one screen instead of widening three modules item by item.
`json_value` found an unchecked `i32` multiply on the time-zone offset in
`parse_trino_time_with_tz`, and `shift_time` carried the same defect one frame
down on the time-of-day hour. Both fields are free-form text, so a value no
zone or clock could hold still parses as an `i32`. This crate declares no
`[profile.release]`, so overflow checks are off in the shipped driver: rather
than panicking it wrapped and reported a different time. Both now use checked
arithmetic and fall back to text, which is what every other unparseable
temporal value here already does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…parsers
`parse_precision_param` and `parse_scale_param` locate the two ends of the
argument list independently, with `find('(')` and `rfind(')')`. Nothing about
the string guarantees the opening parenthesis comes first, and indexing a
reversed range panics.
Both callers gate on `TrinoTypeName::parse`, which no such name survives: the
stray `)` lands in the base name the gate matches on. So this is not a
reachable defect, and the `type_name` fuzz target confirms it across eighteen
million executions with a generator arm built to emit exactly that shape. It
does mean the gate is load-bearing for a property the parsers should hold on
their own.
`name.get(..)` rather than `name[..]`, which is what `parse_fraction_nanos`
already does a few lines down and for the same stated reason. Both functions
are now total: no input reaches a panic. Behaviour on every well-formed
signature is unchanged, and the tests assert both halves of that.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Core fuzzes
write_column_value(ColumnValue→ caller's buffer). Nothingcovered the step before it, where a coordinator's JSON becomes that
ColumnValue, nor the type-signature parsers, the Trino escape dialect, or theconnection-string value parsing — those are this crate's, and three of them read
text chosen by the server.
Four targets in a
fuzz/workspace mirroring core's:json_value,type_name,escape,connect_params. Each builds input from a grammar, since random bytesnever form a balanced
{fn CONVERT(x, SQL_INTEGER)}. They reach the privatemodules through a
fuzz_apimodule behind a default-offfuzzingfeature,following how core gates
test-support.json_valuefound an uncheckedi32multiply on the time-zone offset inparse_trino_time_with_tz, with the same defect inshift_timeon thetime-of-day hour. Both fields are free-form text. This crate declares no
[profile.release], so overflow checks are off in the shipped driver: it did notpanic, it wrapped and returned a different time. Both now use checked
arithmetic and fall back to text. A second commit hardens
parse_precision_param/parse_scale_param, which indexed a range built fromindependent
find/rfindcalls — unreachable behind theTrinoTypeName::parsegate, but the parsers should hold that property themselves.
Checklist
pre-commit run --all-filespasses. It is the single source of truth for what must pass.CHANGELOG.mdhas an entry under## [Unreleased], if an ODBC application can observe the difference. A changed SQLSTATE, a changedSQLGetInfovalue, a new connection-string key or a different type mapping all count.map_trino_error, which is the single place that decides the SQLSTATE and carries Trino's own error code through toSQLGetDiagRec.If it applies
src/backend/types/connect_params.rsand the table inREADME.md. The Windows dialog is generated from the parser, and a test insrc/lib.rsfails if the two disagree../integration-tests/setup.shthen./integration-tests/run-tests.sh. CI runs the Linux suites, so this is about anything you added to them.integration-tests/windows/WINDOWS.md.stackable-odbc-coreor in the Trino client was fixed where it lives, not worked around here, and this pull request says which version it needs.connector/.Notes for the reviewer