[fuzz] H1 codec fuzzer - disable dispatching data on stream that was …

…reset (#13014)

* [h1 codec] Avoid sending protocol error on reset stream
An H1 codec fuzzer detected an assertion failure in ServerConnectionImpl::sendProtocolError().
This is a valid assertion, and the crash happens because the call to onMessageBeginBase() doesn't initialize active_request_ after the stream was reset, during the fuzz test buffer draining step. Further investigation showed that this shouldn't happen because the connection is closed when the stream is reset, and so sendProtocolError() will not be called.

Signed-off-by: Adi Suissa-Peleg <adip@google.com>

test/common/http/codec_impl_fuzz_test.cc

@@ -385,7 +385,8 @@ class HttpStream : public LinkedObject<HttpStream> {
 // the buffer via swap() or modified with mutate().
 class ReorderBuffer {
 public:
-  ReorderBuffer(Connection& connection) : connection_(connection) {}
+  ReorderBuffer(Connection& connection, const bool& should_close_connection)
+      : connection_(connection), should_close_connection_(should_close_connection) {}
 
   void add(Buffer::Instance& data) {
     bufs_.emplace_back();
@@ -397,6 +398,10 @@ class ReorderBuffer {
     while (!bufs_.empty()) {
       Buffer::OwnedImpl& buf = bufs_.front();
       while (buf.length() > 0) {
+        if (should_close_connection_) {
+          ENVOY_LOG_MISC(trace, "Buffer dispatch disabled, stopping drain");
+          return codecClientError("preventing buffer drain due to connection closure");
+        }
         status = connection_.dispatch(buf);
         if (!status.ok()) {
           ENVOY_LOG_MISC(trace, "Error status: {}", status.message());
@@ -439,6 +444,9 @@ class ReorderBuffer {
 
   Connection& connection_;
   std::deque<Buffer::OwnedImpl> bufs_;
+  // A reference to a flag indicating whether the reorder buffer is allowed to dispatch data to
+  // the connection (reference to should_close_connection).
+  const bool& should_close_connection_;
 };
 
 using HttpStreamPtr = std::unique_ptr<HttpStream>;
@@ -494,8 +502,14 @@ void codecFuzz(const test::common::http::CodecImplFuzzTestCase& input, HttpVersi
         headers_with_underscores_action);
   }
 
-  ReorderBuffer client_write_buf{*server};
-  ReorderBuffer server_write_buf{*client};
+  // We track whether the connection should be closed for HTTP/1, since stream resets imply
+  // connection closes.
+  bool should_close_connection = false;
+
+  // The buffers will be blocked from dispatching data if should_close_connection is set to true.
+  // This prevents sending data if a stream reset occurs during the test cleanup when using HTTP/1.
+  ReorderBuffer client_write_buf{*server, should_close_connection};
+  ReorderBuffer server_write_buf{*client, should_close_connection};
 
   ON_CALL(client_connection, write(_, _))
       .WillByDefault(Invoke([&](Buffer::Instance& data, bool) -> void {
@@ -545,10 +559,6 @@ void codecFuzz(const test::common::http::CodecImplFuzzTestCase& input, HttpVersi
     return status;
   };
 
-  // We track whether the connection should be closed for HTTP/1, since stream resets imply
-  // connection closes.
-  bool should_close_connection = false;
-
   constexpr auto max_actions = 1024;
   bool codec_error = false;
   for (int i = 0; i < std::min(max_actions, input.actions().size()) && !should_close_connection &&