[
https://issues.apache.org/jira/browse/THRIFT-6211?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-6211.
--------------------------------
Fix Version/s: 0.25.0
Assignee: Jens Geyer
Resolution: Fixed
> Go: add native go test -fuzz targets and a committed seed corpus that runs in
> CI
> --------------------------------------------------------------------------------
>
> Key: THRIFT-6211
> URL: https://issues.apache.org/jira/browse/THRIFT-6211
> Project: Thrift
> Issue Type: Improvement
> Components: Go - Library
> Reporter: Jens Geyer
> Assignee: Jens Geyer
> Priority: Minor
> Fix For: 0.25.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> h2. Where Go fuzzing stands today
> {{lib/go/test/fuzz}} holds go-fuzz (dvyukov) style targets — {{func
> FuzzX(data []byte) int}} behind the {{gofuzz}} build tag — which is what the
> OSS-Fuzz build consumes. Nothing in this repository actually fuzzes them.
> {{make -C lib/go check}} recurses into {{lib/go/test/fuzz}} and runs {{go
> test -tags gofuzz}}, and the only test in that package is:
> {code}
> func TestFuzz(t *testing.T) {
> FuzzTutorial([]byte{1, 2, 3})
> }
> {code}
> One call, one three-byte input. That is a compile check, and a useful one —
> it catches build drift against the generated code — but it finds nothing.
> There is also no {{testdata/fuzz}} corpus anywhere in the tree. So when
> OSS-Fuzz finds an input and it gets fixed, the input has nowhere to land in
> this repository, and nothing replays it afterwards.
> h2. Proposal: add native Go fuzz targets alongside the existing ones
> Native fuzzing ({{func FuzzX(f *testing.F)}}) has been in the Go toolchain
> since 1.18, and it buys one thing the current arrangement cannot:
> *{{go test}} without {{-fuzz}} replays every {{f.Add}} seed and every file
> under {{testdata/fuzz/<Target>/}} as an ordinary unit test.*
> So a committed corpus becomes a regression suite that costs milliseconds,
> needs no fuzzing infrastructure in CI, and runs on both Go versions in the
> existing {{lib-go}} matrix. {{go test -fuzz=FuzzX -fuzztime=Nm}} then does
> real fuzzing on demand or on a schedule.
> This is an addition, not a replacement — the go-fuzz targets stay as they are
> and OSS-Fuzz keeps consuming them.
> Concretely:
> # *Library-only targets, no generated code.* The protocol and transport read
> paths can be driven through {{ReadMessageBegin}} plus {{thrift.Skip}}, which
> is what generated struct-read code amounts to: dispatch on the wire-supplied
> type, read containers at wire-supplied sizes. Targets written that way need
> nothing but {{lib/go/thrift}}, so they can live there as ordinary
> {{*_test.go}} files and run in the existing CI job with no build-system
> change and no compiler dependency.
> # *Generated-code targets.* Native equivalents of the existing struct and
> round-trip targets in {{lib/go/test/fuzz}}, which do need {{gen-go}} and
> therefore stay behind the current make wiring.
> # *A committed {{testdata/fuzz}} seed corpus*, so fixed inputs stay fixed.
> This is the "add corpora" item from THRIFT-5855.
> # *Optionally a scheduled workflow* that runs {{-fuzz}} for a few minutes per
> target, separate from the PR build. Or simply document the command in
> FUZZING.md and leave scheduled fuzzing to OSS-Fuzz.
> h2. Coverage the current targets do not reach
> A sweep run while investigating THRIFT-6210 used twelve native targets of
> exactly this shape, about 28 million executions against master @
> {{5c5e93e1e}}. It found no panic — the result is not the point here; what
> matters is that the harness needed nothing except the library, and that it
> reached code no existing target does:
> * TBinaryProtocol (strict and non-strict), TCompactProtocol, TJSONProtocol,
> TSimpleJSONProtocol — through both the enveloped entry point and the
> bare-struct one
> * THeaderProtocol, including client-type auto-detection, the transform-ID
> list and info-header parsing — none of which any current target reaches
> * TFramedTransport
> * A full server-side dispatch hop through TMultiplexedProcessor, using a
> hand-written processor rather than generated code
> * {{ParseTuuid}}
> The existing targets cover binary/compact/JSON deserialization and round-trip
> of one generated struct. The header and framed transports,
> TSimpleJSONProtocol, and the dispatch path are not covered at all today.
> h2. Notes
> FUZZING.md already states the intent: "To ensure fuzzing can find issues as
> soon as possible, we will enable fuzzing support in CI once the fuzzers are
> stable." This is a cheap way to get most of that for Go without running a
> fuzzer in the PR path — the corpus replay is just {{go test}}.
> Related: THRIFT-5855 (fuzzing epic; "adding corpora" and testing the
> networking code are both listed there), THRIFT-6210 (the investigation this
> came out of).
> Investigation and this description were AI-assisted (Claude Opus 5); the
> coverage claims were verified by execution against master.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
