commit fe3e5329dd01b7ba87caf2cc5c4f7ebb82e4cbd7
author dschinazi <dschinazi@google.com> Fri Dec 12 17:37:14 2025 -0800
committer Copybara-Service <copybara-worker@google.com> Fri Dec 12 17:38:15 2025 -0800
tree 2f1dc33d0cb6e55811ca0f8b7ef1a2eaf01109bc
parent 6e1b79c86bf53285e407bf6f2a58a980bffdb2de

Fix issues in MasqueOhttpClient::HandleOhttpResponse

Prior to this change, we forgot to remove the request from pending_ohttp_requests_ in some error cases. This CL fixes that. This CL also refactors the function to prevent subclasses from skipping the superclass implementation.

PiperOrigin-RevId: 843891434

quiche/quic/masque/masque_ohttp_client.cc

diff --git a/quiche/quic/masque/masque_ohttp_client.cc b/quiche/quic/masque/masque_ohttp_client.cc
index a0546da..20e46d2 100644
--- a/quiche/quic/masque/masque_ohttp_client.cc
+++ b/quiche/quic/masque/masque_ohttp_client.cc
@@ -280,7 +280,7 @@
   }
 }
 
-absl::Status MasqueOhttpClient::HandleOhttpResponse(
+absl::Status MasqueOhttpClient::ProcessOhttpResponse(
     RequestId request_id, const absl::StatusOr<Message>& response) {
   auto it = pending_ohttp_requests_.find(request_id);
   if (it == pending_ohttp_requests_.end()) {
@@ -289,16 +289,18 @@
     return absl::InternalError(
         "Received unexpected response for unknown request");
   }
-  if (!response.ok()) {
-    QUICHE_LOG(ERROR) << "Failed to fetch OHTTP response: "
-                      << response.status();
-    return response.status();
+  absl::Status status = response.status();
+  if (status.ok()) {
+    status = CheckStatusAndContentType(*response, "message/ohttp-res");
+    if (status.ok()) {
+      status = HandleOhttpResponse(request_id, response);
+      if (status.ok()) {
+        absl::StatusOr<BinaryHttpResponse> binary_response =
+            TryExtractBinaryResponse(request_id, it->second, *response);
+        status = HandleBinaryResponse(binary_response);
+      }
+    }
   }
-  QUICHE_RETURN_IF_ERROR(
-      CheckStatusAndContentType(*response, "message/ohttp-res"));
-  absl::StatusOr<BinaryHttpResponse> binary_response =
-      TryExtractBinaryResponse(request_id, it->second, *response);
-  absl::Status status = HandleBinaryResponse(binary_response);
   pending_ohttp_requests_.erase(it);
   return status;
 }
@@ -314,7 +316,7 @@
       Abort(status);
     }
   } else {
-    auto status = HandleOhttpResponse(request_id, response);
+    auto status = ProcessOhttpResponse(request_id, response);
     if (!status.ok()) {
       QUICHE_LOG(ERROR) << "Failed to handle OHTTP response: " << status;
       Abort(status);

quiche/quic/masque/masque_ohttp_client.h

diff --git a/quiche/quic/masque/masque_ohttp_client.h b/quiche/quic/masque/masque_ohttp_client.h
index b364702..b8c5736 100644
--- a/quiche/quic/masque/masque_ohttp_client.h
+++ b/quiche/quic/masque/masque_ohttp_client.h
@@ -77,13 +77,17 @@
       RequestId request_id, quiche::ObliviousHttpRequest::Context& context,
       const Message& response);
   virtual absl::Status HandleOhttpResponse(
-      RequestId request_id, const absl::StatusOr<Message>& response);
+      RequestId request_id, const absl::StatusOr<Message>& response) {
+    return response.status();
+  }
   virtual absl::Status HandleBinaryResponse(
       const absl::StatusOr<quiche::BinaryHttpResponse>& binary_response) {
     return binary_response.status();
   }
 
  private:
+  absl::Status ProcessOhttpResponse(RequestId request_id,
+                                    const absl::StatusOr<Message>& response);
   absl::Status CheckStatusAndContentType(const Message& response,
                                          const std::string& content_type);