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: fixes oss-fuzz: 9204 #3935

Merged
merged 3 commits into from Jul 27, 2018

Conversation

Projects
None yet
3 participants
@anirudhmurali
Copy link
Member

anirudhmurali commented Jul 23, 2018

Title: Fixes oss-fuzz: 9204

Description: oss-fuzz issue (9204): https://oss-fuzz.com/v2/testcase-detail/5366294281977856
I suppose Envoy doesn't support multiple health checks still. Instead of assertion, have replaced it with throw. This doesn't crash the build. Let me know if any changes.

Risk Level: Low

Testing: Tested unit tests (bazel test //server:server_fuzz_test), built and ran fuzzers with oss-fuzz.

Signed-off-by: Anirudh M m.anirudh18@gmail.com

@zuercher
Copy link
Member

zuercher left a comment

Looks good. Just one style nit.

ASSERT(cluster.health_checks().size() == 1);
new_cluster->setHealthChecker(HealthCheckerFactory::create(
cluster.health_checks()[0], *new_cluster, runtime, random, dispatcher));
if (cluster.health_checks().size() != 1)

This comment has been minimized.

@zuercher

zuercher Jul 23, 2018

Member

I believe our style is to use curly braces around both the if and else statements in this case.

cluster.health_checks().size()));
} else {
new_cluster->setHealthChecker(HealthCheckerFactory::create(
cluster.health_checks()[0], *new_cluster, runtime, random, dispatcher, log_manager));

This comment has been minimized.

@zuercher

zuercher Jul 24, 2018

Member

I think you need to delete log_manager.

@htuch htuch self-assigned this Jul 24, 2018

cluster.health_checks()[0], *new_cluster, runtime, random, dispatcher));
if (cluster.health_checks().size() != 1) {
throw EnvoyException(
fmt::format("Multiple health checks not supported. Health checks count: {}",

This comment has been minimized.

@htuch

htuch Jul 24, 2018

Member

Ideally add a unit test in addition to the corpus, thanks.

@htuch
Copy link
Member

htuch left a comment

Thanks!

@htuch

This comment has been minimized.

Copy link
Member

htuch commented Jul 25, 2018

@anirudhmurali can you merge master to workaround the merge conflict here and push? Ta.

anirudhmurali added some commits Jul 23, 2018

fuzz: fixes oss-fuzz: 9204
Signed-off-by: Anirudh M <m.anirudh18@gmail.com>
added unit tests
Signed-off-by: Anirudh M <m.anirudh18@gmail.com>
resolved merge conflicts
Signed-off-by: Anirudh M <m.anirudh18@gmail.com>

@anirudhmurali anirudhmurali force-pushed the anirudhmurali:oss-fuzz-9204 branch from 3e1d866 to 4febc3d Jul 26, 2018

@htuch

htuch approved these changes Jul 27, 2018

@htuch htuch merged commit 0e2c795 into envoyproxy:master Jul 27, 2018

12 checks passed

DCO All commits have a DCO sign-off from the author
Details
ci/circleci: api Your tests passed on CircleCI!
Details
ci/circleci: asan Your tests passed on CircleCI!
Details
ci/circleci: build_image Your tests passed on CircleCI!
Details
ci/circleci: coverage Your tests passed on CircleCI!
Details
ci/circleci: docs Your tests passed on CircleCI!
Details
ci/circleci: filter_example_mirror Your tests passed on CircleCI!
Details
ci/circleci: format Your tests passed on CircleCI!
Details
ci/circleci: ipv6_tests Your tests passed on CircleCI!
Details
ci/circleci: mac Your tests passed on CircleCI!
Details
ci/circleci: release Your tests passed on CircleCI!
Details
ci/circleci: tsan Your tests passed on CircleCI!
Details

nickrmc83 added a commit to thales-e-security/envoy that referenced this pull request Aug 23, 2018

fuzz: fixes oss-fuzz: 9204 (envoyproxy#3935)
oss-fuzz issue (9204): https://oss-fuzz.com/v2/testcase-detail/5366294281977856
I suppose Envoy doesn't support multiple health checks still. Instead of assertion, have replaced it with throw. This doesn't crash the build. Let me know if any changes.

Risk Level: Low

Testing: Tested unit tests (bazel test //server:server_fuzz_test), built and ran fuzzers with oss-fuzz.

Signed-off-by: Anirudh M <m.anirudh18@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment