Perform content-length validation in OgHttp2Session.

This CL validates the content-length header value in HeaderValidator and allows
the value to be checked in OgHttp2Session when comparing against actual
received body lengths for a stream. If there is a mismatch between the
content-length header value and the total body length received, OgHttp2Session
invokes the visitor OnInvalidFrame() callback and enqueues a RST_STREAM.

This change better aligns oghttp2 with nghttp2 on content-length validation,
but at least one remaining difference is that nghttp2 invokes OnInvalidFrame()
(leading to connection-level error-signalling in Envoy) only when the
content-length mismatch happens with HEADERS ending the stream [1]. The
OnInvalidFrame() --> connection error is important at least for a
content-length integration test. Therefore, oghttp2 also calls OnInvalidFrame()
for content-length mismatch, but regardless of whether HEADERS or DATA ends the
stream for unity. (Ideally, content-length mismatch on one stream should not
cause a connection-level error; perhaps this can be fixed once migration to
oghttp2 is complete.)

With this CL, MultiplexedIntegrationTest.InconsistentContentLength passes with
oghttp2 (http://sponge2/663d811a-e662-44c0-b003-a4d7fcb13d42).

[1] Compare HEADERS at
http://google3/third_party/nghttp2/src/lib/nghttp2_session.c;l=3778,3790-3791;rcl=421446478
to DATA at
http://google3/third_party/nghttp2/src/lib/nghttp2_session.c;l=4961-4963;rcl=421446478.

PiperOrigin-RevId: 422597021
diff --git a/http2/adapter/header_validator.cc b/http2/adapter/header_validator.cc
index 0c1d873..57afa54 100644
--- a/http2/adapter/header_validator.cc
+++ b/http2/adapter/header_validator.cc
@@ -1,6 +1,7 @@
 #include "http2/adapter/header_validator.h"
 
 #include "absl/strings/escaping.h"
+#include "absl/strings/numbers.h"
 #include "common/platform/api/quiche_logging.h"
 
 namespace http2 {
@@ -54,6 +55,7 @@
   pseudo_headers_.clear();
   status_.clear();
   method_.clear();
+  content_length_.reset();
 }
 
 HeaderValidator::HeaderStatus HeaderValidator::ValidateSingleHeader(
@@ -98,9 +100,11 @@
       method_ = std::string(value);
     }
     pseudo_headers_.push_back(std::string(key));
-  } else if (key == "content-length" && status_ == "204" && value != "0") {
-    // There should be no body in a "204 No Content" response.
-    return HEADER_FIELD_INVALID;
+  } else if (key == "content-length") {
+    const bool success = HandleContentLength(value);
+    if (!success) {
+      return HEADER_FIELD_INVALID;
+    }
   }
   return HEADER_OK;
 }
@@ -123,5 +127,25 @@
   return false;
 }
 
+bool HeaderValidator::HandleContentLength(absl::string_view value) {
+  if (value.empty()) {
+    return false;
+  }
+
+  if (status_ == "204" && value != "0") {
+    // There should be no body in a "204 No Content" response.
+    return false;
+  }
+
+  size_t content_length = 0;
+  const bool valid = absl::SimpleAtoi(value, &content_length);
+  if (!valid) {
+    return false;
+  }
+
+  content_length_ = content_length;
+  return true;
+}
+
 }  // namespace adapter
 }  // namespace http2
diff --git a/http2/adapter/header_validator.h b/http2/adapter/header_validator.h
index b21b299..ddcabdf 100644
--- a/http2/adapter/header_validator.h
+++ b/http2/adapter/header_validator.h
@@ -46,11 +46,16 @@
   // For responses, returns the value of the ":status" header, if present.
   absl::string_view status_header() const { return status_; }
 
+  absl::optional<size_t> content_length() const { return content_length_; }
+
  private:
+  bool HandleContentLength(absl::string_view value);
+
   std::vector<std::string> pseudo_headers_;
   std::string status_;
   std::string method_;
   absl::optional<size_t> max_field_size_;
+  absl::optional<size_t> content_length_;
   bool allow_connect_ = false;
 };
 
diff --git a/http2/adapter/header_validator_test.cc b/http2/adapter/header_validator_test.cc
index 5fe05a8..0d4d7fc 100644
--- a/http2/adapter/header_validator_test.cc
+++ b/http2/adapter/header_validator_test.cc
@@ -1,12 +1,15 @@
 #include "http2/adapter/header_validator.h"
 
 #include "absl/strings/str_cat.h"
+#include "absl/types/optional.h"
 #include "common/platform/api/quiche_test.h"
 
 namespace http2 {
 namespace adapter {
 namespace test {
 
+using ::testing::Optional;
+
 TEST(HeaderValidatorTest, HeaderNameEmpty) {
   HeaderValidator v;
   HeaderValidator::HeaderStatus status = v.ValidateSingleHeader("", "value");
@@ -286,6 +289,42 @@
   EXPECT_FALSE(v.FinishHeaderBlock(HeaderType::RESPONSE_TRAILER));
 }
 
+TEST(HeaderValidatorTest, ValidContentLength) {
+  HeaderValidator v;
+
+  v.StartHeaderBlock();
+  EXPECT_EQ(v.content_length(), absl::nullopt);
+  EXPECT_EQ(HeaderValidator::HEADER_OK,
+            v.ValidateSingleHeader("content-length", "41"));
+  EXPECT_THAT(v.content_length(), Optional(41));
+
+  v.StartHeaderBlock();
+  EXPECT_EQ(v.content_length(), absl::nullopt);
+  EXPECT_EQ(HeaderValidator::HEADER_OK,
+            v.ValidateSingleHeader("content-length", "42"));
+  EXPECT_THAT(v.content_length(), Optional(42));
+}
+
+TEST(HeaderValidatorTest, InvalidContentLength) {
+  HeaderValidator v;
+
+  v.StartHeaderBlock();
+  EXPECT_EQ(v.content_length(), absl::nullopt);
+  EXPECT_EQ(HeaderValidator::HEADER_FIELD_INVALID,
+            v.ValidateSingleHeader("content-length", ""));
+  EXPECT_EQ(v.content_length(), absl::nullopt);
+  EXPECT_EQ(HeaderValidator::HEADER_FIELD_INVALID,
+            v.ValidateSingleHeader("content-length", "nan"));
+  EXPECT_EQ(v.content_length(), absl::nullopt);
+  EXPECT_EQ(HeaderValidator::HEADER_FIELD_INVALID,
+            v.ValidateSingleHeader("content-length", "-42"));
+  EXPECT_EQ(v.content_length(), absl::nullopt);
+  // End on a positive note.
+  EXPECT_EQ(HeaderValidator::HEADER_OK,
+            v.ValidateSingleHeader("content-length", "42"));
+  EXPECT_THAT(v.content_length(), Optional(42));
+}
+
 }  // namespace test
 }  // namespace adapter
 }  // namespace http2
diff --git a/http2/adapter/http2_util.cc b/http2/adapter/http2_util.cc
index 553c0bd..f93b161 100644
--- a/http2/adapter/http2_util.cc
+++ b/http2/adapter/http2_util.cc
@@ -97,6 +97,8 @@
       return "InvalidPushPromise";
     case ConnectionError::kExceededMaxConcurrentStreams:
       return "ExceededMaxConcurrentStreams";
+    case ConnectionError::kHttpMessaging:
+      return "HttpMessaging";
   }
   return "UnknownConnectionError";
 }
diff --git a/http2/adapter/http2_visitor_interface.h b/http2/adapter/http2_visitor_interface.h
index 0e8619f..61c76b7 100644
--- a/http2/adapter/http2_visitor_interface.h
+++ b/http2/adapter/http2_visitor_interface.h
@@ -78,6 +78,9 @@
     kInvalidPushPromise,
     // The peer exceeded the max concurrent streams limit.
     kExceededMaxConcurrentStreams,
+    // The peer violated HTTP messaging semantics.
+    // TODO(b/214614028): Remove once these errors can be just RST_STREAM'd.
+    kHttpMessaging,
   };
   virtual void OnConnectionError(ConnectionError error) = 0;
 
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index 51679f4..9b2809b 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -4446,12 +4446,12 @@
   EXPECT_CALL(visitor, OnEndStream(1));
 
   // Stream 3: content-length is not a number
-  // TODO(diannahu): Let oghttp2 validate content-length.
   EXPECT_CALL(visitor, OnFrameHeader(3, _, HEADERS, 5));
   EXPECT_CALL(visitor, OnBeginHeadersForStream(3));
-  EXPECT_CALL(visitor, OnHeaderForStream(3, _, _)).Times(5);
-  EXPECT_CALL(visitor, OnEndHeadersForStream(3));
-  EXPECT_CALL(visitor, OnEndStream(3));
+  EXPECT_CALL(visitor, OnHeaderForStream(3, _, _)).Times(4);
+  EXPECT_CALL(
+      visitor,
+      OnInvalidFrame(3, Http2VisitorInterface::InvalidFrameError::kHttpHeader));
 
   const int64_t stream_result = adapter->ProcessBytes(stream_frames);
   EXPECT_EQ(stream_frames.size(), static_cast<size_t>(stream_result));
@@ -4460,12 +4460,18 @@
   EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
   EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
   EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+  EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 3, _, 0x0));
+  EXPECT_CALL(visitor,
+              OnFrameSent(RST_STREAM, 3, _, 0x0,
+                          static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR)));
+  EXPECT_CALL(visitor, OnCloseStream(3, Http2ErrorCode::HTTP2_NO_ERROR));
 
   EXPECT_TRUE(adapter->want_write());
   int result = adapter->Send();
   EXPECT_EQ(0, result);
   EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
-                                            spdy::SpdyFrameType::SETTINGS}));
+                                            spdy::SpdyFrameType::SETTINGS,
+                                            spdy::SpdyFrameType::RST_STREAM}));
 }
 
 TEST(OgHttp2AdapterServerTest, ServerHandlesContentLengthMismatch) {
@@ -4505,7 +4511,8 @@
   EXPECT_CALL(visitor, OnSettingsEnd());
 
   // Stream 1: content-length is larger than actual data
-  // TODO(diannahu): Let oghttp2 validate content-length.
+  // All data and then OnInvalidFrame() are delivered to the visitor. Note that
+  // nghttp2 does not deliver OnInvalidFrame().
   EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
   EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
   EXPECT_CALL(visitor, OnHeaderForStream(1, _, _)).Times(5);
@@ -4513,26 +4520,33 @@
   EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 1));
   EXPECT_CALL(visitor, OnBeginDataForStream(1, 1));
   EXPECT_CALL(visitor, OnDataForStream(1, "h"));
-  EXPECT_CALL(visitor, OnEndStream(1));
+  EXPECT_CALL(visitor,
+              OnInvalidFrame(
+                  1, Http2VisitorInterface::InvalidFrameError::kHttpMessaging));
 
   // Stream 3: content-length is smaller than actual data
-  // TODO(diannahu): Let oghttp2 validate content-length.
+  // The beginning of data is delivered to the visitor, but not the actual data.
+  // OnInvalidFrame() is delivered (unlike with nghttp2).
   EXPECT_CALL(visitor, OnFrameHeader(3, _, HEADERS, 4));
   EXPECT_CALL(visitor, OnBeginHeadersForStream(3));
   EXPECT_CALL(visitor, OnHeaderForStream(3, _, _)).Times(5);
   EXPECT_CALL(visitor, OnEndHeadersForStream(3));
   EXPECT_CALL(visitor, OnFrameHeader(3, _, DATA, 1));
   EXPECT_CALL(visitor, OnBeginDataForStream(3, 5));
-  EXPECT_CALL(visitor, OnDataForStream(3, "howdy"));
-  EXPECT_CALL(visitor, OnEndStream(3));
+  EXPECT_CALL(visitor,
+              OnInvalidFrame(
+                  3, Http2VisitorInterface::InvalidFrameError::kHttpMessaging));
 
   // Stream 5: content-length is invalid and HEADERS ends the stream
-  // TODO(diannahu): Let oghttp2 validate content-length.
+  // Only oghttp2 invokes OnEndHeadersForStream(), but both oghttp2 and nghttp2
+  // invoke OnInvalidFrame().
   EXPECT_CALL(visitor, OnFrameHeader(5, _, HEADERS, 5));
   EXPECT_CALL(visitor, OnBeginHeadersForStream(5));
   EXPECT_CALL(visitor, OnHeaderForStream(5, _, _)).Times(5);
   EXPECT_CALL(visitor, OnEndHeadersForStream(5));
-  EXPECT_CALL(visitor, OnEndStream(5));
+  EXPECT_CALL(visitor,
+              OnInvalidFrame(
+                  5, Http2VisitorInterface::InvalidFrameError::kHttpMessaging));
 
   const int64_t stream_result = adapter->ProcessBytes(stream_frames);
   EXPECT_EQ(stream_frames.size(), static_cast<size_t>(stream_result));
@@ -4541,12 +4555,30 @@
   EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
   EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
   EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+  EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 1, _, 0x0));
+  EXPECT_CALL(visitor,
+              OnFrameSent(RST_STREAM, 1, _, 0x0,
+                          static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR)));
+  EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR));
+  EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 3, _, 0x0));
+  EXPECT_CALL(visitor,
+              OnFrameSent(RST_STREAM, 3, _, 0x0,
+                          static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR)));
+  EXPECT_CALL(visitor, OnCloseStream(3, Http2ErrorCode::HTTP2_NO_ERROR));
+  EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 5, _, 0x0));
+  EXPECT_CALL(visitor,
+              OnFrameSent(RST_STREAM, 5, _, 0x0,
+                          static_cast<int>(Http2ErrorCode::PROTOCOL_ERROR)));
+  EXPECT_CALL(visitor, OnCloseStream(5, Http2ErrorCode::HTTP2_NO_ERROR));
 
   EXPECT_TRUE(adapter->want_write());
   int result = adapter->Send();
   EXPECT_EQ(0, result);
   EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
-                                            spdy::SpdyFrameType::SETTINGS}));
+                                            spdy::SpdyFrameType::SETTINGS,
+                                            spdy::SpdyFrameType::RST_STREAM,
+                                            spdy::SpdyFrameType::RST_STREAM,
+                                            spdy::SpdyFrameType::RST_STREAM}));
 }
 
 }  // namespace
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index cc37780..ad1fd9d 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -1002,7 +1002,8 @@
 
 void OgHttp2Session::OnDataFrameHeader(spdy::SpdyStreamId stream_id,
                                        size_t length, bool /*fin*/) {
-  if (!stream_map_.contains(stream_id) || streams_reset_.contains(stream_id)) {
+  auto iter = stream_map_.find(stream_id);
+  if (iter == stream_map_.end() || streams_reset_.contains(stream_id)) {
     // The stream does not exist; it could be an error or a benign close, e.g.,
     // getting data for a stream this connection recently closed.
     if (static_cast<Http2StreamId>(stream_id) > highest_processed_stream_id_) {
@@ -1018,6 +1019,16 @@
     fatal_visitor_callback_failure_ = true;
     decoder_.StopProcessing();
   }
+
+  // Validate against the content-length if it exists.
+  if (iter->second.remaining_content_length.has_value()) {
+    if (length > *iter->second.remaining_content_length) {
+      HandleContentLengthError(stream_id);
+      iter->second.remaining_content_length.reset();
+    } else {
+      *iter->second.remaining_content_length -= length;
+    }
+  }
 }
 
 void OgHttp2Session::OnStreamFrameData(spdy::SpdyStreamId stream_id,
@@ -1044,10 +1055,20 @@
   auto iter = stream_map_.find(stream_id);
   if (iter != stream_map_.end()) {
     iter->second.half_closed_remote = true;
-    if (!streams_reset_.contains(stream_id)) {
-      visitor_.OnEndStream(stream_id);
+    if (streams_reset_.contains(stream_id)) {
+      return;
     }
+
+    // Validate against the content-length if it exists.
+    if (iter->second.remaining_content_length.has_value() &&
+        *iter->second.remaining_content_length != 0) {
+      HandleContentLengthError(stream_id);
+      return;
+    }
+
+    visitor_.OnEndStream(stream_id);
   }
+
   auto queued_frames_iter = queued_frames_.find(stream_id);
   const bool no_queued_frames = queued_frames_iter == queued_frames_.end() ||
                                 queued_frames_iter->second == 0;
@@ -1096,6 +1117,7 @@
     } else {
       it->second.received_header_type = headers_handler_.header_type();
     }
+    it->second.remaining_content_length = headers_handler_.content_length();
     headers_handler_.set_stream_id(0);
   }
 }
@@ -1668,5 +1690,19 @@
   }
 }
 
+void OgHttp2Session::HandleContentLengthError(Http2StreamId stream_id) {
+  EnqueueFrame(absl::make_unique<spdy::SpdyRstStreamIR>(
+      stream_id, spdy::ERROR_CODE_PROTOCOL_ERROR));
+  // TODO(b/214614028): The OnInvalidFrame() is for nghttp2 parity, but parity
+  // causes this error to be connection-level in Envoy. Revisit after migration.
+  const bool ok = visitor_.OnInvalidFrame(
+      stream_id, Http2VisitorInterface::InvalidFrameError::kHttpMessaging);
+  if (!ok) {
+    fatal_visitor_callback_failure_ = true;
+    LatchErrorAndNotify(Http2ErrorCode::REFUSED_STREAM,
+                        ConnectionError::kHttpMessaging);
+  }
+}
+
 }  // namespace adapter
 }  // namespace http2
diff --git a/http2/adapter/oghttp2_session.h b/http2/adapter/oghttp2_session.h
index 3cdcd12..aac2688 100644
--- a/http2/adapter/oghttp2_session.h
+++ b/http2/adapter/oghttp2_session.h
@@ -219,6 +219,7 @@
     void* user_data = nullptr;
     int32_t send_window = kInitialFlowControlWindowSize;
     absl::optional<HeaderType> received_header_type;
+    absl::optional<size_t> remaining_content_length;
     bool half_closed_local = false;
     bool half_closed_remote = false;
     // Indicates that `outbound_body` temporarily cannot produce data.
@@ -258,6 +259,9 @@
                     type_ == HeaderType::RESPONSE_100);
       return validator_.status_header();
     }
+    absl::optional<size_t> content_length() const {
+      return validator_.content_length();
+    }
     void AllowConnect() { validator_.AllowConnect(); }
     void SetMaxFieldSize(uint32_t field_size) {
       validator_.SetMaxFieldSize(field_size);
@@ -385,6 +389,8 @@
 
   void DecrementQueuedFrameCount(uint32_t stream_id, uint8_t frame_type);
 
+  void HandleContentLengthError(Http2StreamId stream_id);
+
   // Receives events when inbound frames are parsed.
   Http2VisitorInterface& visitor_;