Skip to content

fuzz: move fuzz_target from oss-fuzz#861

Merged
pelletier merged 4 commits intopelletier:v2from
manunio:move-from-oss-fuzz
Apr 29, 2023
Merged

fuzz: move fuzz_target from oss-fuzz#861
pelletier merged 4 commits intopelletier:v2from
manunio:move-from-oss-fuzz

Conversation

@manunio
Copy link
Copy Markdown
Contributor

@manunio manunio commented Apr 22, 2023 •
edited
Loading

  • 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

@manunio
fuzz: move fuzz_target from oss-fuzz
4be788c 
- 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
Copy Markdown
Contributor Author

manunio commented Apr 22, 2023 •
edited
Loading

  • 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
pelletier
pelletier reviewed Apr 25, 2023
View reviewed changes
Comment thread ci.sh
pelletier
pelletier reviewed Apr 25, 2023
View reviewed changes
Comment thread oss_fuzz.go Outdated
@manunio
Copy link
Copy Markdown
Contributor Author

manunio commented Apr 28, 2023

Friendly Ping :)

@pelletier
Copy link
Copy Markdown
Owner

pelletier 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.

@manunio
Copy link
Copy Markdown
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
@pelletier
Copy link
Copy Markdown
Owner

pelletier commented Apr 29, 2023

Thank you!

@manunio manunio deleted the move-from-oss-fuzz branch April 29, 2023 06:05
@manunio manunio mentioned this pull request Apr 29, 2023
Merged
jonathanmetzman pushed a commit to google/oss-fuzz that referenced this pull request May 1, 2023
@manunio
go-toml: move fuzz_target upstream (#10190)
642736e 
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

Reviewers

@pelletier pelletier pelletier approved these changes

Assignees

No one assigned

Labels

build Issues regarding go-toml's CI system.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@manunio @pelletier