Fixed grpc health check bug

[zasweq]

Signed-off-by: Zach Reyes zasweq@google.com

Commit Message: Fixed gRPC health check bug
Additional Description: Fixes issue https://oss-fuzz.com/testcase-detail/6242204122873856. My health check fuzzer ran into this bug. What happened with this bug was that the decoder for gRPC frames sometimes encoded frames into a frame vector even though decode() returns false in the case of valid frames and then invalid frames in the buffer. Thus, this section of decodeData() called decoder_->decode() which returned false, which led to onRpcComplete being called: https://github.com/envoyproxy/envoy/blob/master/source/common/upstream/health_checker_impl.cc#L623. Thus, I added a return (which used to be implicit) after this, as frames() may still put frames in the grpc frame vector, thus calling onRpcComplete() again in the same decodeData() and causing a null dereference. I also added unit tests to show that the decoder can still put frames in the output vector even though it fails eventually and returns false.

Same as #13855, but different fix.
Risk Level: Low
Testing: Added regression test.

[zasweq]

/assign @asraa @adisuissa @htuch @mattklein123

[adisuissa]

Thanks for fixing this, I'll close my PR for now.
What happens if the gRPC decoder clears the output (frames) if the decoding fails?

[asraa]

Out of curiosity, is it the presence of null bytes in the frame data that makes the grpc decoder fail or is it something in conjunction with that?

(if so, is it easy to simplify the test case?)

It looks like

envoy/source/common/grpc/codec.cc

Line 48 in bd73f3c

decoding_error_ = true;

is the only case to cause a decoding_error_

[asraa]

\cc @fengli79
cc'ing in case you have some context on the grpc decoder contract

[asraa]

Thanks, please check format!

[asraa]

Great tests! LGTM
/wait

[asraa]

Thanks, LGTM!
@envoyproxy/senior-maintainers could you please take a very quick look to approve this bug fix handling a faulty response?

[mattklein123]

LGTM can you merge main?

/wait