Call Visitor::OnError() from HttpDecoder::RaiseError().

This is to make sure connection is closed on error.

This bug was caught by ClusterFuzz at https://crbug.com/989416.

gfe-relnote: n/a, change to QUIC v99-only code.  Protected by existing disabled gfe2_reloadable_flag_quic_enable_version_99.
PiperOrigin-RevId: 262337299
Change-Id: Ib5241b0c53228dfe48356f5c03d9f4ffae499611

quic/core/http/http_decoder.cc

diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc
index 550aa6f..e105fc6 100644
--- a/quic/core/http/http_decoder.cc
+++ b/quic/core/http/http_decoder.cc
@@ -126,7 +126,6 @@
   if (current_frame_length_ > MaxFrameLength(current_frame_type_)) {
     // TODO(bnc): Signal HTTP_EXCESSIVE_LOAD or similar to peer.
     RaiseError(QUIC_INTERNAL_ERROR, "Frame is too large");
-    visitor_->OnError(this);
     return false;
   }
 
@@ -440,6 +439,7 @@
   state_ = STATE_ERROR;
   error_ = error;
   error_detail_ = std::move(error_detail);
+  visitor_->OnError(this);
 }
 
 bool HttpDecoder::ParsePriorityFrame(QuicDataReader* reader,

quic/core/http/http_decoder_test.cc

diff --git a/quic/core/http/http_decoder_test.cc b/quic/core/http/http_decoder_test.cc
index 476c2c6..7cbe949 100644
--- a/quic/core/http/http_decoder_test.cc
+++ b/quic/core/http/http_decoder_test.cc
@@ -404,6 +404,7 @@
 
     HttpDecoder decoder(&visitor_);
     EXPECT_CALL(visitor_, OnPriorityFrameStart(header_length));
+    EXPECT_CALL(visitor_, OnError(&decoder));
 
     QuicByteCount processed_bytes =
         decoder.ProcessInput(input.data(), input.size());
@@ -460,6 +461,40 @@
   EXPECT_EQ("", decoder_.error_detail());
 }
 
+TEST_F(HttpDecoderTest, CorruptSettingsFrame) {
+  const char* const kPayload =
+      "\x42\x11"                           // two-byte id
+      "\x80\x22\x33\x44"                   // four-byte value
+      "\x58\x39"                           // two-byte id
+      "\xf0\x22\x33\x44\x55\x66\x77\x88";  // eight-byte value
+  struct {
+    size_t payload_length;
+    const char* const error_message;
+  } kTestData[] = {
+      {1, "Unable to read settings frame identifier"},
+      {5, "Unable to read settings frame content"},
+      {7, "Unable to read settings frame identifier"},
+      {12, "Unable to read settings frame content"},
+  };
+
+  for (const auto& test_data : kTestData) {
+    std::string input;
+    input.push_back(4u);  // type SETTINGS
+    input.push_back(test_data.payload_length);
+    const size_t header_length = input.size();
+    input.append(kPayload, test_data.payload_length);
+
+    HttpDecoder decoder(&visitor_);
+    EXPECT_CALL(visitor_, OnSettingsFrameStart(header_length));
+    EXPECT_CALL(visitor_, OnError(&decoder));
+
+    QuicByteCount processed_bytes =
+        decoder.ProcessInput(input.data(), input.size());
+    EXPECT_EQ(input.size(), processed_bytes);
+    EXPECT_EQ(QUIC_INTERNAL_ERROR, decoder.error());
+    EXPECT_EQ(test_data.error_message, decoder.error_detail());
+  }
+}
 TEST_F(HttpDecoderTest, DataFrame) {
   InSequence s;
   std::string input(
@@ -792,6 +827,57 @@
   EXPECT_EQ("", decoder_.error_detail());
 }
 
+TEST_F(HttpDecoderTest, CorruptFrame) {
+  InSequence s;
+
+  struct {
+    const char* const input;
+    const char* const error_message;
+  } kTestData[] = {{"\x03"   // type (CANCEL_PUSH)
+                    "\x01"   // length
+                    "\x40",  // first byte of two-byte varint push id
+                    "Unable to read push_id"},
+                   {"\x05"   // type (PUSH_PROMISE)
+                    "\x01"   // length
+                    "\x40",  // first byte of two-byte varint push id
+                    "Unable to read push_id"},
+                   {"\x0D"   // type (MAX_PUSH_ID)
+                    "\x01"   // length
+                    "\x40",  // first byte of two-byte varint push id
+                    "Unable to read push_id"},
+                   {"\x0E"   // type (DUPLICATE_PUSH)
+                    "\x01"   // length
+                    "\x40",  // first byte of two-byte varint push id
+                    "Unable to read push_id"},
+                   {"\x07"   // type (GOAWAY)
+                    "\x01"   // length
+                    "\x40",  // first byte of two-byte varint stream id
+                    "Unable to read GOAWAY stream_id"}};
+
+  for (const auto& test_data : kTestData) {
+    {
+      HttpDecoder decoder(&visitor_);
+      EXPECT_CALL(visitor_, OnError(&decoder));
+
+      QuicStringPiece input(test_data.input);
+      decoder.ProcessInput(input.data(), input.size());
+      EXPECT_EQ(QUIC_INTERNAL_ERROR, decoder.error());
+      EXPECT_EQ(test_data.error_message, decoder.error_detail());
+    }
+    {
+      HttpDecoder decoder(&visitor_);
+      EXPECT_CALL(visitor_, OnError(&decoder));
+
+      QuicStringPiece input(test_data.input);
+      for (auto c : input) {
+        decoder.ProcessInput(&c, 1);
+      }
+      EXPECT_EQ(QUIC_INTERNAL_ERROR, decoder.error());
+      EXPECT_EQ(test_data.error_message, decoder.error_detail());
+    }
+  }
+}  // namespace test
+
 }  // namespace test
 
 }  // namespace quic