Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fuzz: move fuzz_target from oss-fuzz #861

Merged
merged 4 commits into from Apr 29, 2023

Conversation

manunio
Copy link
Contributor

@manunio manunio commented Apr 22, 2023

  • This pr attempts to move fuzz_toml fuzz_target from oss-fuzz to go-toml.
  • Its recommended by oss-fuzz to move fuzz-target upstream as this eases maintenance of fuzz_target.
  • It also include check against too large input.
  • Skips fuzz from coverage report.
  • Adds a new ossfuzz package

- This pr attempts to move `fuzz_toml` fuzz_target from oss-fuzz to go-toml.
- Its recommended by oss-fuzz to move fuzz-target upstream as this eases maintenance of fuzz_target.
- It also include check against too large input.
@manunio
Copy link
Contributor Author

manunio commented Apr 22, 2023

  • fuzzing check: It will be fixed at oss-fuzz as soon as this pr lands.

@manunio manunio marked this pull request as draft April 24, 2023 19:54
@manunio manunio marked this pull request as ready for review April 25, 2023 09:26
ci.sh Show resolved Hide resolved
oss_fuzz.go Outdated Show resolved Hide resolved
@manunio
Copy link
Contributor Author

manunio commented Apr 28, 2023

Friendly Ping :)

@pelletier
Copy link
Owner

Sorry for the delay!

I think I know what's going on. When the code is in oss_fuzz.go, the coverage assumes it's part of the codebase. When it's part of the _test.go file, Go expects all Fuzz* functions to have a specific signature for the built-in fuzzing mechanism.

I just realized that the current proposal would add FuzzToml to the public API of go-toml, which I'd like to avoid. Is there a way to either put it in a new /ossfuzz directory (if you need to call the function directly) or /cmd/ossfuzz if you'd rather use a binary.

@manunio
Copy link
Contributor Author

manunio commented Apr 28, 2023

Sorry for the delay!

I think I know what's going on. When the code is in oss_fuzz.go, the coverage assumes it's part of the codebase. When it's part of the _test.go file, Go expects all Fuzz* functions to have a specific signature for the built-in fuzzing mechanism.

I just realized that the current proposal would add FuzzToml to the public API of go-toml, which I'd like to avoid. Is there a way to either put it in a new /ossfuzz directory (if you need to call the function directly) or /cmd/ossfuzz if you'd rather use a binary.

Hi, Thanks for the feedback, I have made changes as per your request :)

@pelletier pelletier added the build Issues regarding go-toml's CI system. label Apr 28, 2023
@pelletier pelletier merged commit 2aa0836 into pelletier:v2 Apr 29, 2023
9 checks passed
@pelletier
Copy link
Owner

Thank you!

@manunio manunio deleted the move-from-oss-fuzz branch April 29, 2023 06:05
jonathanmetzman pushed a commit to google/oss-fuzz that referenced this pull request May 1, 2023
This pr removes the fuzz_target, which were moved upstream in
pelletier/go-toml#861
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues regarding go-toml's CI system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants