-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
initial fuzzing harness #153
Conversation
willscott
commented
Mar 27, 2020
•
edited
Loading
edited
- allow dynamic specification of harnessed datastore implementation
- include closing / re-opening of datastore in the script
@ribasushi if you have cycles, maybe you can take a first pass at review and see if this structure looks reasonable for a datastore fuzzing harness |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty excellently written! The instructions worked out of the box ( I made one suggestion to add, no edits )
I do not know the tooling too well, but from what I was able to read + running this locally: LGTM
```golang | ||
go-fuzz-build | ||
go-fuzz | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a section at the very end:
If you have not installed the fuzzing tools yet, execute:
go get -u github.com/dvyukov/go-fuzz/go-fuzz github.com/dvyukov/go-fuzz/go-fuzz-build
fuzz/fuzzer.go
Outdated
|
||
// Fuzz is a go-fuzzer compatible input point for replaying | ||
// data (interpreted as a script of commands) | ||
// to kown ipfs datastore implementations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// to kown ipfs datastore implementations | |
// to known ipfs datastore implementations |
go.mod
Outdated
github.com/google/uuid v1.1.1 | ||
github.com/ipfs/go-ds-badger v0.2.1 | ||
github.com/ipfs/go-ds-leveldb v0.4.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to add these as hard deps? Perhaps the fuzz portion needs to become a sub-project? @Stebalien thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't. We should create a sub-module.
go 1.14 | ||
|
||
require ( | ||
github.com/ipfs/go-datastore v0.4.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can add a replace
directive pointing to ../
.
fuzz/README.md
Outdated
```golang | ||
go get github.com/ipfs/go-datastore | ||
cd go-datastore/fuzz | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't going to work reliably (gopath, etc). I'd just remove it.
fuzzer "github.com/ipfs/go-datastore/fuzz" | ||
dsq "github.com/ipfs/go-datastore/query" | ||
|
||
"github.com/spf13/pflag" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd slightly prefer to use github.com/urfave/cli to be consistent with the other projects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that i'm not pulling in a full app/cli framework for these entry points, i'm going to defer on that for now.