Reject forbidden frame types and incomplete frames in ALPS.

https://vasilvv.github.io/httpbis-alps/draft-vvv-httpbis-alps.html#name-use-of-alps-in-http-3

PiperOrigin-RevId: 359360218
Change-Id: Ice2650cb934ba776f38ab9465c5d4d3e3294fdb6

quic/core/http/http_decoder.h

diff --git a/quic/core/http/http_decoder.h b/quic/core/http/http_decoder.h
index 65e9f2e..fe955c3 100644
--- a/quic/core/http/http_decoder.h
+++ b/quic/core/http/http_decoder.h
@@ -150,6 +150,9 @@
 
   const std::string& error_detail() const { return error_detail_; }
 
+  // Returns true if input data processed so far ends on a frame boundary.
+  bool AtFrameBoundary() const { return state_ == STATE_READING_FRAME_TYPE; }
+
  private:
   friend test::HttpDecoderPeer;
 

quic/core/http/quic_spdy_session.cc

diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index 94e2bfc..91530d3 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -98,51 +98,74 @@
   // HttpDecoder::Visitor implementation.
   void OnError(HttpDecoder* /*decoder*/) override {}
   bool OnCancelPushFrame(const CancelPushFrame& /*frame*/) override {
-    return true;
+    error_detail_ = "CANCEL_PUSH frame forbidden";
+    return false;
   }
   bool OnMaxPushIdFrame(const MaxPushIdFrame& /*frame*/) override {
-    return true;
+    error_detail_ = "MAX_PUSH_ID frame forbidden";
+    return false;
   }
-  bool OnGoAwayFrame(const GoAwayFrame& /*frame*/) override { return true; }
+  bool OnGoAwayFrame(const GoAwayFrame& /*frame*/) override {
+    error_detail_ = "GOAWAY frame forbidden";
+    return false;
+  }
   bool OnSettingsFrameStart(QuicByteCount /*header_length*/) override {
     return true;
   }
   bool OnSettingsFrame(const SettingsFrame& /*frame*/) override { return true; }
   bool OnDataFrameStart(QuicByteCount /*header_length*/, QuicByteCount
                         /*payload_length*/) override {
-    return true;
+    error_detail_ = "DATA frame forbidden";
+    return false;
   }
   bool OnDataFramePayload(absl::string_view /*payload*/) override {
-    return true;
+    QUICHE_NOTREACHED();
+    return false;
   }
-  bool OnDataFrameEnd() override { return true; }
+  bool OnDataFrameEnd() override {
+    QUICHE_NOTREACHED();
+    return false;
+  }
   bool OnHeadersFrameStart(QuicByteCount /*header_length*/,
                            QuicByteCount /*payload_length*/) override {
-    return true;
+    error_detail_ = "HEADERS frame forbidden";
+    return false;
   }
   bool OnHeadersFramePayload(absl::string_view /*payload*/) override {
-    return true;
+    QUICHE_NOTREACHED();
+    return false;
   }
-  bool OnHeadersFrameEnd() override { return true; }
+  bool OnHeadersFrameEnd() override {
+    QUICHE_NOTREACHED();
+    return false;
+  }
   bool OnPushPromiseFrameStart(QuicByteCount /*header_length*/) override {
-    return true;
+    error_detail_ = "PUSH_PROMISE frame forbidden";
+    return false;
   }
   bool OnPushPromiseFramePushId(
       PushId /*push_id*/,
       QuicByteCount
       /*push_id_length*/,
       QuicByteCount /*header_block_length*/) override {
-    return true;
+    QUICHE_NOTREACHED();
+    return false;
   }
   bool OnPushPromiseFramePayload(absl::string_view /*payload*/) override {
-    return true;
+    QUICHE_NOTREACHED();
+    return false;
   }
-  bool OnPushPromiseFrameEnd() override { return true; }
+  bool OnPushPromiseFrameEnd() override {
+    QUICHE_NOTREACHED();
+    return false;
+  }
   bool OnPriorityUpdateFrameStart(QuicByteCount /*header_length*/) override {
-    return true;
+    error_detail_ = "PRIORITY_UPDATE frame forbidden";
+    return false;
   }
   bool OnPriorityUpdateFrame(const PriorityUpdateFrame& /*frame*/) override {
-    return true;
+    QUICHE_NOTREACHED();
+    return false;
   }
   bool OnAcceptChFrameStart(QuicByteCount /*header_length*/) override {
     return true;
@@ -162,8 +185,13 @@
   }
   bool OnUnknownFrameEnd() override { return true; }
 
+  const absl::optional<std::string>& error_detail() const {
+    return error_detail_;
+  }
+
  private:
   QuicSpdySession* const session_;
+  absl::optional<std::string> error_detail_;
 };
 
 }  // namespace
@@ -1053,10 +1081,18 @@
   AlpsFrameDecoder alps_frame_decoder(this);
   HttpDecoder decoder(&alps_frame_decoder);
   decoder.ProcessInput(reinterpret_cast<const char*>(alps_data), alps_length);
+  if (alps_frame_decoder.error_detail()) {
+    return alps_frame_decoder.error_detail();
+  }
+
   if (decoder.error() != QUIC_NO_ERROR) {
     return decoder.error_detail();
   }
 
+  if (!decoder.AtFrameBoundary()) {
+    return "incomplete HTTP/3 frame";
+  }
+
   return absl::nullopt;
 }
 

quic/core/http/quic_spdy_session_test.cc

diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index db263b0..c2ab731 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -3314,11 +3314,14 @@
   session_.OnStreamFrame(data3);
 }
 
-TEST_P(QuicSpdySessionTestClient, OnAlpsData) {
+TEST_P(QuicSpdySessionTestClient, AcceptChViaAlps) {
+  if (!VersionUsesHttp3(transport_version())) {
+    return;
+  }
+
   StrictMock<MockHttp3DebugVisitor> debug_visitor;
   session_.set_debug_visitor(&debug_visitor);
 
-  AcceptChFrame accept_ch_frame{{{"foo", "bar"}}};
   std::string serialized_accept_ch_frame = absl::HexStringToBytes(
       "4089"      // type (ACCEPT_CH)
       "08"        // length
@@ -3328,12 +3331,48 @@
       "626172");  // value "bar"
 
   if (GetQuicReloadableFlag(quic_parse_accept_ch_frame)) {
-    EXPECT_CALL(debug_visitor, OnAcceptChFrameReceivedViaAlps(accept_ch_frame));
+    AcceptChFrame expected_accept_ch_frame{{{"foo", "bar"}}};
+    EXPECT_CALL(debug_visitor,
+                OnAcceptChFrameReceivedViaAlps(expected_accept_ch_frame));
   }
 
-  session_.OnAlpsData(
+  auto error = session_.OnAlpsData(
       reinterpret_cast<const uint8_t*>(serialized_accept_ch_frame.data()),
       serialized_accept_ch_frame.size());
+  EXPECT_FALSE(error);
+}
+
+TEST_P(QuicSpdySessionTestClient, AlpsForbiddenFrame) {
+  if (!VersionUsesHttp3(transport_version())) {
+    return;
+  }
+
+  std::string forbidden_frame = absl::HexStringToBytes(
+      "00"        // type (DATA)
+      "03"        // length
+      "66666f");  // "foo"
+
+  auto error = session_.OnAlpsData(
+      reinterpret_cast<const uint8_t*>(forbidden_frame.data()),
+      forbidden_frame.size());
+  ASSERT_TRUE(error);
+  EXPECT_EQ("DATA frame forbidden", error.value());
+}
+
+TEST_P(QuicSpdySessionTestClient, AlpsIncompleteFrame) {
+  if (!VersionUsesHttp3(transport_version())) {
+    return;
+  }
+
+  std::string incomplete_frame = absl::HexStringToBytes(
+      "04"    // type (SETTINGS)
+      "03");  // non-zero length but empty payload
+
+  auto error = session_.OnAlpsData(
+      reinterpret_cast<const uint8_t*>(incomplete_frame.data()),
+      incomplete_frame.size());
+  ASSERT_TRUE(error);
+  EXPECT_EQ("incomplete HTTP/3 frame", error.value());
 }
 
 }  // namespace