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
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);
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);