commit 9e42729500679d45d1ede11fb504ab8e1e6584aa
author birenroy <birenroy@google.com> Thu Feb 29 07:56:44 2024 -0800
committer Copybara-Service <copybara-worker@google.com> Thu Feb 29 07:57:34 2024 -0800
tree 06730a0e242fcd722a353a1479750f24e1d9a6a2
parent 02047e04d076d36226e92847a94acf33544f10d6

Refactors error handling in PassthroughHeadersHandler.

Protected by refactoring, no functional change expected.

PiperOrigin-RevId: 611471355

quiche/http2/adapter/oghttp2_session.cc

diff --git a/quiche/http2/adapter/oghttp2_session.cc b/quiche/http2/adapter/oghttp2_session.cc
index 8f36e85..e12c9a4 100644
--- a/quiche/http2/adapter/oghttp2_session.cc
+++ b/quiche/http2/adapter/oghttp2_session.cc
@@ -216,12 +216,13 @@
 }
 
 void OgHttp2Session::PassthroughHeadersHandler::OnHeaderBlockStart() {
+  error_encountered_ = false;
   result_ = Http2VisitorInterface::HEADER_OK;
   const bool status = visitor_.OnBeginHeadersForStream(stream_id_);
   if (!status) {
     QUICHE_VLOG(1)
         << "Visitor rejected header block, returning HEADER_CONNECTION_ERROR";
-    result_ = Http2VisitorInterface::HEADER_CONNECTION_ERROR;
+    SetResult(Http2VisitorInterface::HEADER_CONNECTION_ERROR);
   }
   validator_->StartHeaderBlock();
 }
@@ -242,7 +243,7 @@
 
 void OgHttp2Session::PassthroughHeadersHandler::OnHeader(
     absl::string_view key, absl::string_view value) {
-  if (result_ != Http2VisitorInterface::HEADER_OK) {
+  if (error_encountered_) {
     QUICHE_VLOG(2) << "Early return; status not HEADER_OK";
     return;
   }
@@ -254,28 +255,31 @@
   if (validation_result != HeaderValidator::HEADER_OK) {
     QUICHE_VLOG(2) << "Header validation failed with result "
                    << static_cast<int>(validation_result);
-    result_ = InterpretHeaderStatus(validation_result);
+    SetResult(InterpretHeaderStatus(validation_result));
     return;
   }
-  result_ = visitor_.OnHeaderForStream(stream_id_, key, value);
+  const Http2VisitorInterface::OnHeaderResult result =
+      visitor_.OnHeaderForStream(stream_id_, key, value);
+  SetResult(result);
 }
 
 void OgHttp2Session::PassthroughHeadersHandler::OnHeaderBlockEnd(
     size_t /* uncompressed_header_bytes */,
     size_t /* compressed_header_bytes */) {
-  if (result_ == Http2VisitorInterface::HEADER_OK) {
+  const bool frame_contains_fin = frame_contains_fin_;
+  frame_contains_fin_ = false;
+  if (!error_encountered_) {
     if (!validator_->FinishHeaderBlock(type_)) {
       QUICHE_VLOG(1) << "FinishHeaderBlock returned false; returning "
-                        "HEADER_HTTP_MESSAGING";
-      result_ = Http2VisitorInterface::HEADER_HTTP_MESSAGING;
+                     << "HEADER_HTTP_MESSAGING";
+      SetResult(Http2VisitorInterface::HEADER_HTTP_MESSAGING);
     }
   }
-  if (frame_contains_fin_ && IsResponse(type_) &&
-      StatusIs1xx(status_header())) {
+  if (frame_contains_fin && IsResponse(type_) && StatusIs1xx(status_header())) {
     QUICHE_VLOG(1) << "Unexpected end of stream without final headers";
-    result_ = Http2VisitorInterface::HEADER_HTTP_MESSAGING;
+    SetResult(Http2VisitorInterface::HEADER_HTTP_MESSAGING);
   }
-  if (result_ == Http2VisitorInterface::HEADER_OK) {
+  if (!error_encountered_) {
     const bool result = visitor_.OnEndHeadersForStream(stream_id_);
     if (!result) {
       session_.fatal_visitor_callback_failure_ = true;
@@ -284,7 +288,6 @@
   } else {
     session_.OnHeaderStatus(stream_id_, result_);
   }
-  frame_contains_fin_ = false;
 }
 
 // TODO(diannahu): Add checks for request methods.
@@ -306,6 +309,14 @@
   return true;
 }
 
+void OgHttp2Session::PassthroughHeadersHandler::SetResult(
+    Http2VisitorInterface::OnHeaderResult result) {
+  if (result != Http2VisitorInterface::HEADER_OK) {
+    error_encountered_ = true;
+    result_ = result;
+  }
+}
+
 // A visitor that extracts an int64_t from each type of a ProcessBytesResult.
 struct OgHttp2Session::ProcessBytesResultVisitor {
   int64_t operator()(const int64_t bytes) const { return bytes; }

quiche/http2/adapter/oghttp2_session.h

diff --git a/quiche/http2/adapter/oghttp2_session.h b/quiche/http2/adapter/oghttp2_session.h
index da88c68..25d3f9a 100644
--- a/quiche/http2/adapter/oghttp2_session.h
+++ b/quiche/http2/adapter/oghttp2_session.h
@@ -266,6 +266,7 @@
 
     void set_stream_id(Http2StreamId stream_id) {
       stream_id_ = stream_id;
+      error_encountered_ = false;
       result_ = Http2VisitorInterface::HEADER_OK;
     }
 
@@ -296,6 +297,8 @@
     bool CanReceiveBody() const;
 
    private:
+    void SetResult(Http2VisitorInterface::OnHeaderResult result);
+
     OgHttp2Session& session_;
     Http2VisitorInterface& visitor_;
     Http2StreamId stream_id_ = 0;
@@ -305,6 +308,7 @@
     std::unique_ptr<HeaderValidatorBase> validator_;
     HeaderType type_ = HeaderType::RESPONSE;
     bool frame_contains_fin_ = false;
+    bool error_encountered_ = false;
   };
 
   struct QUICHE_EXPORT ProcessBytesResultVisitor;