Skip to content

Remove empty config from fuzz corpus#43627

Closed
ravenblackx wants to merge 1 commit into
envoyproxy:mainfrom
ravenblackx:bad_fuzz
Closed

Remove empty config from fuzz corpus#43627
ravenblackx wants to merge 1 commit into
envoyproxy:mainfrom
ravenblackx:bad_fuzz

Conversation

@ravenblackx
Copy link
Copy Markdown
Contributor

Commit Message: Remove empty config from fuzz corpus
Additional Description: The corpus of fuzz tests is supposed to be like a regression test for reproducible errors that were previously caught by the fuzz tester. The empty config does not create a consistent test - it picks a filter "at random" - which is consistent, because seeded, until unrelated changes, such as adding another filter, change the outcome of that randomness and result in test failure that isn't actually related to the PR making the change.

There are two reasonable ways to do a test that's actually reproducible for empty configs - either make it not empty, by including the name of the filter to be tested (envoy.filters.http.mcp_json_rest_bridge is one example that fails this test, which is what's provoking this PR), or, better, have a separate test case that performs an equivalent test with the empty config against every filter. I can't add either of those in this PR because in either model the test would already be failing on mcp_json_rest_bridge.
Risk Level: None, test-only
Testing: Small reduction
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

Signed-off-by: Raven Black <ravenblack@dropbox.com>
Copy link
Copy Markdown
Member

@botengyao botengyao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this corpus cannot prevent the actual fuzzer running with empty input, but could unblock the local test. I am leaning towards to @tyxia's PR #43646, wdyt?

@tyxia
Copy link
Copy Markdown
Member

tyxia commented Feb 25, 2026

Removing this corpus cannot prevent the actual fuzzer running with empty input, but could unblock the local test. I am leaning towards to @tyxia's PR #43646, wdyt?
Thanks for pointing me to this PR!

SGTM. It is now a CI breakage that we should get it fixed sooner than later. Excluding mcp_json_rest is fine as it is work-in-progress

@yanavlasov
Copy link
Copy Markdown
Contributor

Is this PR still needed? Did #43646 address the problem?

/wait-any

@ravenblackx
Copy link
Copy Markdown
Contributor Author

That's fine for unblocking - I would still be in favor of removing blank from the corpus because it still has the property of being inconsistent when unrelated things change, but I'll close this and do a more complete version of that (creating a separate blank config test that validates against every filter, and having a corpus-file test that requires corpus files to have filter names, to avoid the problem of unrelated changes angering this test. Also should probably do something about the other randomization in the input to this test.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants