tlv: fuzz test encoding/decoding #7889
Conversation
| // bytes unparsed from data, causing the encoded data to be shorter than the | ||
| // original. | ||
| func bigSizeHarness(t *testing.T, data []byte, val1, val2 interface{}) { | ||
| if len(data) > 9 { |
There was a problem hiding this comment.
I'm starting to wonder how often this is hit and whether it deteriorates the efficacy of our test?
There was a problem hiding this comment.
It should increase efficacy, actually. Shorter inputs are faster for the fuzzer to generate, mutate, and execute. So by short-circuiting for inputs longer than necessary, we encourage the fuzzing engine to spend more time on smaller inputs.
Other fuzzing engines (e.g., libFuzzer, AFL) have a flag for max input size, which prevents the fuzzer from generating inputs larger than that. Go's fuzzing engine doesn't have such a flag, unfortunately.
|
|
||
| var buf [8]byte | ||
| if err := DBigSize(r, val1, &buf, 9); err != nil { | ||
| return |
There was a problem hiding this comment.
why can we return here instead of assertion?
There was a problem hiding this comment.
Not every byte sequence is a valid BigSize, so we shouldn't fail the test when we encounter an invalid sequence.
tlv/fuzz_test.go
Outdated
|
|
||
| func FuzzTUint64(f *testing.F) { | ||
| f.Fuzz(func(t *testing.T, data []byte, decodeLen uint8) { | ||
| if decodeLen > 8 { |
There was a problem hiding this comment.
what do you say if we just iterate from 0 to 7 here?
There was a problem hiding this comment.
Done in 93cd750.
| b64 [64]byte | ||
| pk *btcec.PublicKey | ||
| b []byte | ||
| bs32 uint32 |
There was a problem hiding this comment.
curious about what bs and tu stand for?
There was a problem hiding this comment.
BigSize and Truncated Uint.
|
|
||
| var encodeRecords []Record | ||
| for typ, val := range parsedTypes { | ||
| if typ < Type(len(decodedRecords)) { |
There was a problem hiding this comment.
would be helpful if we add some docs explaining why this check.
There was a problem hiding this comment.
Done in 5bb4df4.
We use a new harness to compare decoded values instead of encoded values, since there may be some unparsed bytes in the original data.
These fuzz tests are identical to non-truncated integers, except that we allow the fuzzer to choose decode lengths shorter than the length of normal integers.
loop over decode length
better documentation
| // bytes unparsed from data, causing the encoded data to be shorter than the | ||
| // original. | ||
| func bigSizeHarness(t *testing.T, data []byte, val1, val2 interface{}) { | ||
| if len(data) > 9 { |
There was a problem hiding this comment.
It should increase efficacy, actually. Shorter inputs are faster for the fuzzer to generate, mutate, and execute. So by short-circuiting for inputs longer than necessary, we encourage the fuzzing engine to spend more time on smaller inputs.
Other fuzzing engines (e.g., libFuzzer, AFL) have a flag for max input size, which prevents the fuzzer from generating inputs larger than that. Go's fuzzing engine doesn't have such a flag, unfortunately.
|
|
||
| var buf [8]byte | ||
| if err := DBigSize(r, val1, &buf, 9); err != nil { | ||
| return |
There was a problem hiding this comment.
Not every byte sequence is a valid BigSize, so we shouldn't fail the test when we encounter an invalid sequence.
tlv/fuzz_test.go
Outdated
|
|
||
| func FuzzTUint64(f *testing.F) { | ||
| f.Fuzz(func(t *testing.T, data []byte, decodeLen uint8) { | ||
| if decodeLen > 8 { |
There was a problem hiding this comment.
Done in 93cd750.
| b64 [64]byte | ||
| pk *btcec.PublicKey | ||
| b []byte | ||
| bs32 uint32 |
There was a problem hiding this comment.
BigSize and Truncated Uint.
|
|
||
| var encodeRecords []Record | ||
| for typ, val := range parsedTypes { | ||
| if typ < Type(len(decodedRecords)) { |
There was a problem hiding this comment.
Done in 5bb4df4.
|
@yyforyongyu: review reminder |
Based on #4641.