Revert WinZipAesStreamFuzzer, ZipCryptoStreamFuzzer - #131075
Conversation
|
Azure Pipelines: Successfully started running 5 pipeline(s). 11 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @karelz, @dotnet/area-system-io-compression |
There was a problem hiding this comment.
Pull request overview
This PR removes two broken fuzzing harnesses from the DotnetFuzzing project and updates the OneFuzz deployment pipeline accordingly, reducing noise/failures and unblocking fuzzing runs for other targets.
Changes:
- Delete
WinZipAesStreamFuzzerandZipCryptoStreamFuzzerfromsrc/libraries/Fuzzing/DotnetFuzzing/Fuzzers/. - Remove the corresponding OneFuzz deployment steps from
eng/pipelines/libraries/fuzzing/deploy-to-onefuzz.yml.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/Fuzzing/DotnetFuzzing/Fuzzers/ZipCryptoStreamFuzzer.cs | Removes the ZipCrypto stream fuzzer harness (file deleted). |
| src/libraries/Fuzzing/DotnetFuzzing/Fuzzers/WinZipAesStreamFuzzer.cs | Removes the WinZip AES stream fuzzer harness (file deleted). |
| eng/pipelines/libraries/fuzzing/deploy-to-onefuzz.yml | Stops deploying the removed fuzzers to OneFuzz. |
alinpahontu2912
left a comment
alinpahontu2912
left a comment
There was a problem hiding this comment.
Sorry, will take a look. Approving to unblock
|
Workflow state for the Holistic Review Orchestrator. {
"version": 5,
"last_dispatched_commit": "a6562fd5005a01081bf0efa4c01edfd1899f87bf",
"last_dispatched_base_ref": "main",
"last_dispatched_base_sha": "d349aa7988e9adf0ee0f5f0473886c0582975955",
"last_reviewed_commit": "a6562fd5005a01081bf0efa4c01edfd1899f87bf",
"last_reviewed_base_ref": "main",
"last_reviewed_base_sha": "d349aa7988e9adf0ee0f5f0473886c0582975955",
"last_recorded_worker_run_id": "29744573407",
"review_attempt_commit": "",
"review_attempt_base_ref": "",
"review_attempt_count": 0,
"max_review_attempts": 5,
"review_history_format": "holistic-review-disclosure-v1",
"review_history": [
{
"commit": "a6562fd5005a01081bf0efa4c01edfd1899f87bf",
"review_id": 4735261986
}
]
} |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Holistic Review
Motivation: The PR reverts two fuzzers (WinZipAesStreamFuzzer and ZipCryptoStreamFuzzer) added in #122093. Per the description, these never built via run.bat (missing corpus file) and threw TypeInitializationException at runtime, so they were effectively non-functional and were blocking the fuzzing pipeline for other projects.
Approach: The change deletes the two fuzzer source files under src/libraries/Fuzzing/DotnetFuzzing/Fuzzers/ and removes their corresponding onefuzz-task@0 deployment steps from eng/pipelines/libraries/fuzzing/deploy-to-onefuzz.yml. This is a straightforward, deletion-only revert (0 additions, 314 deletions).
Summary: The revert is clean and complete. Fuzzers are discovered by reflection in Program.cs (types implementing IFuzzer), so deleting the source files fully deregisters them without any manual list to update. I confirmed no remaining references to WinZipAesStreamFuzzer, ZipCryptoStreamFuzzer, or their corpus names (winzipaesstream, zipcryptostream) exist at the head commit, and the two YAML deployment steps that referenced them were removed while the surrounding tasks (e.g., Utf8JsonWriterFuzzer, ZipArchiveFuzzer) remain intact and correctly bracketed by the ONEFUZZ_TASK_WORKAROUND markers. Removing non-functional fuzzers to unblock the pipeline is reasonable and low-risk. No concerns; LGTM.
Note
This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.
Generated by Holistic Review · 36.9 AIC · ⌖ 9.64 AIC · ⊞ 10K
Deletes the fuzzers added as part of #122093 (comment).
These neither build (via run.bat because of missing corpus file), neither run at all with a
TypeInitializationException, so I get the impression they were never tested/ran.Removing to unblock fuzzing for other projects.
For future reference, please don't commit new fuzzers that you haven't tested locally. It's highly unlikely you/your AI agent got them right immediately on the first try, without a single assert out of place.