Fix `fuzz_repo` False Positive With a Refactor to Improve Effectiveness & Efficiency (#1317)
Sorry for the churn on this test; filesystem I/O makes for slow feedback
when trying to test locally.
---
Anyway, the issues listed below are addressed in this PR by simplifying
the test harness implementation. The result being significantly improved
fuzzing coverage and slightly improved execution speed.
Prior to the changes introduced here, the implementation of `fuzz_repo`
had several issues:
1. `repo.stage()` was not called before the first `repo.do_commit()`
call
2. When `repo.stage()` was eventually called, the argument it was passed
was incorrect (a list of absolute paths instead of relative to the repo
dir) causing a `ValueError` that broke the fuzzer runs. This was hidden
during the initial local testing because of point 3 below.
3. Inefficient consumption of the fuzzer provided data resulted in the
input bytes being exhausted early in the `TestOneInput` execution until
the fuzzer was able to generate a corpus large enough to satisfy all of
the `Consume*` calls.
Other changes:
- `EnhancedFuzzedDataProvider.ConsumeRandomString` now accepts an
argument to optionally exclude unicode surrogates from the returned
string (useful for places where they will always raise an exception like
most filesystem operations.)
- Adds new fuzzing engine recommended dictionary entries (now that the
test is effective enough to generate recommendations.)
Refactor `fuzz_repo` to Improve Effectiveness & Efficiency
Prior to the changes introduced here, the implementation of `fuzz_repo`
had several issues:
1. `repo.stage()` was not called before the first `repo.do_commit()`
call
2. When `repo.stage()` was eventually called, the argument it was passed
was incorrect (a list of absolute paths instead of relative to the
repo dir) causing a `ValueError` that broke the fuzzer runs. This was
hidden during the initial local testing because of point 3 below.
3. Inefficient consumption of the fuzzer provided data resulted in the
input bytes being exhausted early in the `TestOneInput` execution
until the fuzzer was able to generate a corpus large enough to
satisfy all of the `Consume*` calls.
The issues listed above are addressed here by simplifying the test
harness implementation. The result being significanly improved fuzzing
coverage and slightly improved execution speed.
Other changes:
- `EnhancedFuzzedDataProvider.ConsumeRandomString` now accepts an
argument to optionally exclude unicode surrogates from the returned
string (useful for places where they will always raise an exception
like most filesystem operations.)
- Adds new fuzzing engine recommended dictionary entries (now that the
test is effective enough to generate recommendations.)
Update Known Exception Handling in `fuzz_repo` to Prevent False Positives (#1316)
The `fuzz_repo` fuzz target is crashing the fuzzer because of a known
exception case where the time ofsets in commit messages can raise a
`ValueError`, which is not an interesting in the context of fuzzing.
Update Known Exception Handling in `fuzz_repo` to Prevent False Positives
The `fuzz_repo` fuzz target is crashing the fuzzer because of a known
exception case where the time ofsets in commit messages can raise a
`ValueError`, which is not an interesting in the context of fuzzing.
As a follow-up to #1304, this PR introduces additional fuzz targets,
fuzz test dictionaries, and `fuzzing/fuzz-targets/test_utils.py` which
includes test utilities to help DRY fuzzing test code.
The changes here should increase fuzzing coverage from ~2% to ~17% based
on the results of my local testing.
The commit messages in this PR should describe the specific changes, but
the most significant information detailed below:
**fuzzing/fuzz-targets/fuzz_object_store.py**
- Tests the `Blob`, `Tree`, and `Commit` classes using fuzzer provided
data.
- This test is based on the example code in the [Object Store
tutorial](https://www.dulwich.io/docs/tutorial/object-store.html),
`fuzz_object_store.py` uses a `MemoryRepo` to avoid disk IO where
possible, in the interest of test execution efficiency.
**fuzzing/fuzz-targets/fuzz_repo.py**
- Tests basic functionality of the `Repo` class.
- This test must perform actual disk IO to effectively test all
functionality, so it is somewhat slow compared to other fuzz targets in
this repo. There might be ways to improve this, but as of this PR it
works well enough.
## `fuzzing/fuzz-targets/test_utils.py`
- Adds a `EnhancedFuzzedDataProvider` class that extends
`atheris.FuzzedDataProvider` to abstract some common use-cases into DRY
method calls.
- The `is_expected_error` helper function was extracted from
`fuzz_configfile.py` into this dedicated test utility file so it can be
reused by other fuzz harnesses in `fuzz-targets/`.
- Also renamed and better documented the `is_expected_error` function
now that it is shared.
## Other Notes
I've tested all of the changes proposed here extensively in my local
environment. They are working well enough that I feel they are a net
value add to the fuzz test suite, but **these tests can likely be
further optimized to improve coverage and efficiency**. I plan to keep
an eye on their performance and further optimize the tests & supporting
code as needed.
