Refactor OHTTP client, gateway, and BHTTP decoder creation methods to pass the retained parameter as pointer instead of a reference. The decoder, message handler, and Envoy request headers are now passed by pointer and leverage nullability and lifetime annotations, as per go/totw/116. PiperOrigin-RevId: 854209203
diff --git a/quiche/BUILD.bazel b/quiche/BUILD.bazel index 9358904..c71830d 100644 --- a/quiche/BUILD.bazel +++ b/quiche/BUILD.bazel
@@ -63,6 +63,7 @@ deps = [ ":quiche_core", "@com_google_absl//absl/base:core_headers", + "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor",
diff --git a/quiche/binary_http/binary_http_message.cc b/quiche/binary_http/binary_http_message.cc index 3e03806..2ee5302 100644 --- a/quiche/binary_http/binary_http_message.cc +++ b/quiche/binary_http/binary_http_message.cc
@@ -530,7 +530,7 @@ absl::Status BinaryHttpRequest::IndeterminateLengthDecoder::DecodeContentTerminatedSection( QuicheDataReader& reader, absl::string_view& checkpoint) { - uint64_t length_or_content_terminator; + uint64_t length_or_content_terminator = kContentTerminator; do { if (!reader.ReadVarInt62(&length_or_content_terminator)) { return absl::OutOfRangeError("Not enough data to read section.");
diff --git a/quiche/binary_http/binary_http_message.h b/quiche/binary_http/binary_http_message.h index 7994132..da467f9 100644 --- a/quiche/binary_http/binary_http_message.h +++ b/quiche/binary_http/binary_http_message.h
@@ -11,6 +11,8 @@ #include <utility> #include <vector> +#include "absl/base/attributes.h" +#include "absl/base/nullability.h" #include "absl/container/flat_hash_map.h" #include "absl/status/status.h" #include "absl/status/statusor.h" @@ -236,9 +238,18 @@ virtual absl::Status OnTrailersDone() = 0; }; - explicit IndeterminateLengthDecoder( - MessageSectionHandler& message_section_handler) - : message_section_handler_(message_section_handler) {} + // Creates a new IndeterminateLengthDecoder. Does not take ownership of + // `message_section_handler`, which must refer to a valid handler that + // outlives this decoder. + static absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> Create( + MessageSectionHandler* absl_nonnull message_section_handler + ABSL_ATTRIBUTE_LIFETIME_BOUND) { + if (message_section_handler == nullptr) { + return absl::InvalidArgumentError("MessageSectionHandler is null"); + } + return BinaryHttpRequest::IndeterminateLengthDecoder( + message_section_handler); + } // Decodes an Indeterminate-Length BHTTP request. As the caller receives // portions of the request, the caller can call this method with the request @@ -254,6 +265,10 @@ absl::Status Decode(absl::string_view data, bool end_stream); private: + explicit IndeterminateLengthDecoder( + MessageSectionHandler* absl_nonnull message_section_handler + ABSL_ATTRIBUTE_LIFETIME_BOUND) + : message_section_handler_(*message_section_handler) {} // Carries out the decode logic from the checkpoint. Returns // OutOfRangeError if there is not enough data to decode the current // section. When a section is fully decoded, the checkpoint is updated. @@ -264,6 +279,9 @@ absl::Status DecodeContentTerminatedSection(QuicheDataReader& reader, absl::string_view& checkpoint); + // The handler to invoke when a section is decoded successfully. The + // handler can return an error if the decoded data cannot be processed + // successfully. Not owned. MessageSectionHandler& message_section_handler_; // Stores the data that could not be processed due to missing data. std::string buffer_;
diff --git a/quiche/binary_http/binary_http_message_test.cc b/quiche/binary_http/binary_http_message_test.cc index 489474d..d0e0d97 100644 --- a/quiche/binary_http/binary_http_message_test.cc +++ b/quiche/binary_http/binary_http_message_test.cc
@@ -518,8 +518,10 @@ kPadding), &request_bytes)); RequestMessageSectionTestHandler handler; - BinaryHttpRequest::IndeterminateLengthDecoder decoder(handler); - QUICHE_EXPECT_OK(decoder.Decode(request_bytes, true)); + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(&handler); + QUICHE_ASSERT_OK(decoder); + QUICHE_EXPECT_OK(decoder->Decode(request_bytes, /*end_stream=*/true)); ExpectRequestMessageSectionHandler(handler.GetMessageData()); } @@ -572,12 +574,14 @@ kPadding), &request_bytes)); - auto handler = GetMockMessageSectionHandler(); + std::unique_ptr<MockFailingMessageSectionHandler> handler = + GetMockMessageSectionHandler(); std::string error_message = "Failed to handle control data"; EXPECT_CALL(*handler, OnControlData(testing::_)) .WillOnce(testing::Return(absl::InternalError(error_message))); - auto decoder = - std::make_unique<BinaryHttpRequest::IndeterminateLengthDecoder>(*handler); + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(handler.get()); + QUICHE_ASSERT_OK(decoder); EXPECT_THAT(decoder->Decode(request_bytes, true), test::StatusIs(absl::StatusCode::kInternal, testing::HasSubstr(error_message))); @@ -586,9 +590,10 @@ error_message = "Failed to handle header"; EXPECT_CALL(*handler, OnHeader(testing::_, testing::_)) .WillOnce(testing::Return(absl::InternalError(error_message))); - decoder = - std::make_unique<BinaryHttpRequest::IndeterminateLengthDecoder>(*handler); - EXPECT_THAT(decoder->Decode(request_bytes, true), + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder2 = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(handler.get()); + QUICHE_ASSERT_OK(decoder2); + EXPECT_THAT(decoder2->Decode(request_bytes, true), test::StatusIs(absl::StatusCode::kInternal, testing::HasSubstr(error_message))); @@ -596,9 +601,10 @@ error_message = "Failed to handle headers done"; EXPECT_CALL(*handler, OnHeadersDone()) .WillOnce(testing::Return(absl::InternalError(error_message))); - decoder = - std::make_unique<BinaryHttpRequest::IndeterminateLengthDecoder>(*handler); - EXPECT_THAT(decoder->Decode(request_bytes, true), + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder3 = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(handler.get()); + QUICHE_ASSERT_OK(decoder3); + EXPECT_THAT(decoder3->Decode(request_bytes, true), test::StatusIs(absl::StatusCode::kInternal, testing::HasSubstr(error_message))); @@ -606,9 +612,10 @@ error_message = "Failed to handle body chunk"; EXPECT_CALL(*handler, OnBodyChunk(testing::_)) .WillOnce(testing::Return(absl::InternalError(error_message))); - decoder = - std::make_unique<BinaryHttpRequest::IndeterminateLengthDecoder>(*handler); - EXPECT_THAT(decoder->Decode(request_bytes, true), + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder4 = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(handler.get()); + QUICHE_ASSERT_OK(decoder4); + EXPECT_THAT(decoder4->Decode(request_bytes, true), test::StatusIs(absl::StatusCode::kInternal, testing::HasSubstr(error_message))); @@ -616,9 +623,10 @@ error_message = "Failed to handle body chunks done"; EXPECT_CALL(*handler, OnBodyChunksDone()) .WillOnce(testing::Return(absl::InternalError(error_message))); - decoder = - std::make_unique<BinaryHttpRequest::IndeterminateLengthDecoder>(*handler); - EXPECT_THAT(decoder->Decode(request_bytes, true), + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder5 = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(handler.get()); + QUICHE_ASSERT_OK(decoder5); + EXPECT_THAT(decoder5->Decode(request_bytes, true), test::StatusIs(absl::StatusCode::kInternal, testing::HasSubstr(error_message))); @@ -626,9 +634,10 @@ error_message = "Failed to handle trailer"; EXPECT_CALL(*handler, OnTrailer(testing::_, testing::_)) .WillOnce(testing::Return(absl::InternalError(error_message))); - decoder = - std::make_unique<BinaryHttpRequest::IndeterminateLengthDecoder>(*handler); - EXPECT_THAT(decoder->Decode(request_bytes, true), + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder6 = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(handler.get()); + QUICHE_ASSERT_OK(decoder6); + EXPECT_THAT(decoder6->Decode(request_bytes, true), test::StatusIs(absl::StatusCode::kInternal, testing::HasSubstr(error_message))); @@ -636,9 +645,10 @@ error_message = "Failed to handle trailers done"; EXPECT_CALL(*handler, OnTrailersDone()) .WillOnce(testing::Return(absl::InternalError(error_message))); - decoder = - std::make_unique<BinaryHttpRequest::IndeterminateLengthDecoder>(*handler); - EXPECT_THAT(decoder->Decode(request_bytes, true), + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder7 = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(handler.get()); + QUICHE_ASSERT_OK(decoder7); + EXPECT_THAT(decoder7->Decode(request_bytes, true), test::StatusIs(absl::StatusCode::kInternal, testing::HasSubstr(error_message))); } @@ -654,13 +664,15 @@ kPadding), &request_bytes)); RequestMessageSectionTestHandler handler; - BinaryHttpRequest::IndeterminateLengthDecoder decoder(handler); + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(&handler); + QUICHE_ASSERT_OK(decoder); for (uint64_t i = 0; i < request_bytes.size() - 1; i++) { QUICHE_EXPECT_OK( - decoder.Decode(absl::string_view(&request_bytes[i], 1), false)); + decoder->Decode(absl::string_view(&request_bytes[i], 1), false)); } // Decode the last byte, send end_stream. - QUICHE_EXPECT_OK(decoder.Decode( + QUICHE_EXPECT_OK(decoder->Decode( absl::string_view(&request_bytes[request_bytes.size() - 1], 1), true)); ExpectRequestMessageSectionHandler(handler.GetMessageData()); } @@ -673,23 +685,29 @@ std::string request_bytes; EXPECT_TRUE(absl::HexStringToBytes(incomplete_request_bytes, &request_bytes)); RequestMessageSectionTestHandler handler; - BinaryHttpRequest::IndeterminateLengthDecoder decoder(handler); - EXPECT_THAT(decoder.Decode(request_bytes, true), + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(&handler); + QUICHE_ASSERT_OK(decoder); + EXPECT_THAT(decoder->Decode(request_bytes, /*end_stream=*/true), test::StatusIs(absl::StatusCode::kInvalidArgument)); } TEST(IndeterminateLengthDecoder, InvalidFramingError) { RequestMessageSectionTestHandler handler; - BinaryHttpRequest::IndeterminateLengthDecoder decoder(handler); + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(&handler); + QUICHE_ASSERT_OK(decoder); std::string request_bytes; EXPECT_TRUE(absl::HexStringToBytes("00", &request_bytes)); - absl::Status status = decoder.Decode(request_bytes, false); + absl::Status status = decoder->Decode(request_bytes, /*end_stream=*/false); EXPECT_THAT(status, test::StatusIs(absl::StatusCode::kInvalidArgument)); } TEST(IndeterminateLengthDecoder, InvalidPaddingError) { RequestMessageSectionTestHandler handler; - BinaryHttpRequest::IndeterminateLengthDecoder decoder(handler); + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(&handler); + QUICHE_ASSERT_OK(decoder); std::string request_bytes; EXPECT_TRUE(absl::HexStringToBytes( absl::StrCat( @@ -699,8 +717,8 @@ kIndeterminateLengthEncodedRequestTrailers, kContentTerminator, kPadding), &request_bytes)); - QUICHE_EXPECT_OK(decoder.Decode(request_bytes, false)); - absl::Status status = decoder.Decode("\x01", false); + QUICHE_EXPECT_OK(decoder->Decode(request_bytes, /*end_stream=*/false)); + absl::Status status = decoder->Decode("\x01", /*end_stream=*/false); EXPECT_THAT(status, test::StatusIs(absl::StatusCode::kInvalidArgument)); } @@ -717,7 +735,9 @@ TEST(IndeterminateLengthDecoder, TruncatedBodyAndTrailers) { RequestMessageSectionTestHandler handler; - BinaryHttpRequest::IndeterminateLengthDecoder decoder(handler); + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(&handler); + QUICHE_ASSERT_OK(decoder); std::string request_bytes; EXPECT_TRUE(absl::HexStringToBytes( absl::StrCat( @@ -725,7 +745,7 @@ kIndeterminateLengthEncodedRequestHeaders, k8ByteContentTerminator), &request_bytes)); - QUICHE_EXPECT_OK(decoder.Decode(request_bytes, true)); + QUICHE_EXPECT_OK(decoder->Decode(request_bytes, /*end_stream=*/true)); auto message_data = handler.GetMessageData(); EXPECT_TRUE(message_data.body_chunks_done_); EXPECT_THAT(message_data.body_chunks_, testing::IsEmpty()); @@ -734,7 +754,9 @@ TEST(IndeterminateLengthDecoder, TruncatedBodyAndTrailersSplitEndStream) { RequestMessageSectionTestHandler handler; - BinaryHttpRequest::IndeterminateLengthDecoder decoder(handler); + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(&handler); + QUICHE_ASSERT_OK(decoder); std::string request_bytes; EXPECT_TRUE(absl::HexStringToBytes( absl::StrCat( @@ -742,9 +764,9 @@ kIndeterminateLengthEncodedRequestHeaders, k8ByteContentTerminator), &request_bytes)); - QUICHE_EXPECT_OK(decoder.Decode(request_bytes, false)); + QUICHE_EXPECT_OK(decoder->Decode(request_bytes, /*end_stream=*/false)); // Send `end_stream` with no data. - QUICHE_EXPECT_OK(decoder.Decode("", true)); + QUICHE_EXPECT_OK(decoder->Decode("", /*end_stream=*/true)); auto message_data = handler.GetMessageData(); EXPECT_TRUE(message_data.body_chunks_done_); EXPECT_THAT(message_data.body_chunks_, testing::IsEmpty()); @@ -915,8 +937,10 @@ EXPECT_EQ(encoded_data, expected); RequestMessageSectionTestHandler handler; - BinaryHttpRequest::IndeterminateLengthDecoder decoder(handler); - QUICHE_EXPECT_OK(decoder.Decode(encoded_data, true)); + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(&handler); + QUICHE_ASSERT_OK(decoder); + QUICHE_EXPECT_OK(decoder->Decode(encoded_data, /*end_stream=*/true)); ExpectRequestMessageSectionHandler(handler.GetMessageData()); } @@ -1246,7 +1270,9 @@ TEST(IndeterminateLengthDecoder, TruncatedTrailers) { RequestMessageSectionTestHandler handler; - BinaryHttpRequest::IndeterminateLengthDecoder decoder(handler); + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(&handler); + QUICHE_EXPECT_OK(decoder); std::string request_bytes; EXPECT_TRUE(absl::HexStringToBytes( absl::StrCat( @@ -1256,7 +1282,7 @@ k4ByteContentTerminator), &request_bytes)); - QUICHE_EXPECT_OK(decoder.Decode(request_bytes, true)); + QUICHE_EXPECT_OK(decoder->Decode(request_bytes, /*end_stream=*/true)); auto message_data = handler.GetMessageData(); EXPECT_TRUE(message_data.body_chunks_done_); std::vector<std::string> expected_body_chunks = {"chunk1", "chunk2", @@ -1267,7 +1293,9 @@ TEST(IndeterminateLengthDecoder, TruncatedTrailersSplitEndStream) { RequestMessageSectionTestHandler handler; - BinaryHttpRequest::IndeterminateLengthDecoder decoder(handler); + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(&handler); + QUICHE_EXPECT_OK(decoder); std::string request_bytes; EXPECT_TRUE(absl::HexStringToBytes( absl::StrCat( @@ -1277,9 +1305,9 @@ k4ByteContentTerminator), &request_bytes)); - QUICHE_EXPECT_OK(decoder.Decode(request_bytes, false)); + QUICHE_EXPECT_OK(decoder->Decode(request_bytes, /*end_stream=*/false)); // Send `end_stream` with no data. - QUICHE_EXPECT_OK(decoder.Decode("", true)); + QUICHE_EXPECT_OK(decoder->Decode("", /*end_stream=*/true)); auto message_data = handler.GetMessageData(); EXPECT_TRUE(message_data.body_chunks_done_); std::vector<std::string> expected_body_chunks = {"chunk1", "chunk2", @@ -1296,9 +1324,11 @@ kIndeterminateLengthEncodedRequestHeaders, k8ByteContentTerminator), &request_bytes)); RequestMessageSectionTestHandler handler; - BinaryHttpRequest::IndeterminateLengthDecoder decoder(handler); - QUICHE_EXPECT_OK(decoder.Decode(request_bytes, true)); - absl::Status status = decoder.Decode(request_bytes, false); + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(&handler); + QUICHE_EXPECT_OK(decoder); + QUICHE_EXPECT_OK(decoder->Decode(request_bytes, /*end_stream=*/true)); + absl::Status status = decoder->Decode(request_bytes, /*end_stream=*/false); EXPECT_THAT(status, test::StatusIs(absl::StatusCode::kInternal)); } @@ -1313,10 +1343,12 @@ TEST_P(InvalidEndStreamTest, InvalidEndStreamError) { const InvalidEndStreamTestCase& test_case = GetParam(); RequestMessageSectionTestHandler handler; - BinaryHttpRequest::IndeterminateLengthDecoder decoder(handler); + absl::StatusOr<BinaryHttpRequest::IndeterminateLengthDecoder> decoder = + BinaryHttpRequest::IndeterminateLengthDecoder::Create(&handler); + QUICHE_EXPECT_OK(decoder); std::string request_bytes; EXPECT_TRUE(absl::HexStringToBytes(test_case.request, &request_bytes)); - absl::Status status = decoder.Decode(request_bytes, true); + absl::Status status = decoder->Decode(request_bytes, /*end_stream=*/true); EXPECT_THAT(status, test::StatusIs(absl::StatusCode::kInvalidArgument)); }
diff --git a/quiche/oblivious_http/oblivious_http_client.cc b/quiche/oblivious_http/oblivious_http_client.cc index 197c0d7..1980068 100644 --- a/quiche/oblivious_http/oblivious_http_client.cc +++ b/quiche/oblivious_http/oblivious_http_client.cc
@@ -106,7 +106,7 @@ : ohttp_key_config_(ohttp_key_config), hpke_sender_context_(std::move(hpke_sender_context)), aead_params_(aead_params), - chunk_handler_(chunk_handler) {} + chunk_handler_(*chunk_handler) {} absl::StatusOr<ChunkedObliviousHttpClient> ChunkedObliviousHttpClient::Create( absl::string_view hpke_public_key, @@ -224,9 +224,6 @@ absl::Status ChunkedObliviousHttpClient::DecryptResponseCheckpoint( absl::string_view& response_checkpoint, bool end_stream) { - if (chunk_handler_ == nullptr) { - return absl::InternalError("Chunk handler is null."); - } QuicheDataReader reader(response_checkpoint); switch (response_current_section_) { case ResponseMessageSection::kEnd: @@ -297,7 +294,7 @@ response_chunk_counter.Increment(); absl::Status handler_status = - chunk_handler_->OnDecryptedChunk(decrypted_chunk); + chunk_handler_.OnDecryptedChunk(decrypted_chunk); if (!handler_status.ok()) { return absl::InternalError(absl::StrCat( "Chunk handler failed to process decrypted chunk: ", @@ -334,13 +331,13 @@ /*is_final_chunk=*/true)); absl::Status handler_status = - chunk_handler_->OnDecryptedChunk(decrypted_chunk); + chunk_handler_.OnDecryptedChunk(decrypted_chunk); if (!handler_status.ok()) { return absl::InternalError( absl::StrCat("Chunk handler failed to process decrypted chunk: ", handler_status.message())); } - handler_status = chunk_handler_->OnChunksDone(); + handler_status = chunk_handler_.OnChunksDone(); if (!handler_status.ok()) { return absl::InternalError( absl::StrCat("Chunk handler failed to process chunks done: ",
diff --git a/quiche/oblivious_http/oblivious_http_client.h b/quiche/oblivious_http/oblivious_http_client.h index cbf3fa6..7ac16ad 100644 --- a/quiche/oblivious_http/oblivious_http_client.h +++ b/quiche/oblivious_http/oblivious_http_client.h
@@ -170,9 +170,8 @@ ObliviousHttpRequest::Context hpke_sender_context_; ObliviousHttpResponse::CommonAeadParamsResult aead_params_; - // The handler to invoke when a chunk is decrypted successfully. This is not - // owned by this class. - ObliviousHttpChunkHandler* absl_nonnull chunk_handler_; + // The handler to invoke when a chunk is decrypted successfully. Not owned. + ObliviousHttpChunkHandler& chunk_handler_; RequestMessageSection request_current_section_ = RequestMessageSection::kHeader;
diff --git a/quiche/oblivious_http/oblivious_http_client_test.cc b/quiche/oblivious_http/oblivious_http_client_test.cc index 0b4fab1..cfb5c0c 100644 --- a/quiche/oblivious_http/oblivious_http_client_test.cc +++ b/quiche/oblivious_http/oblivious_http_client_test.cc
@@ -128,7 +128,7 @@ } absl::StatusOr<ChunkedObliviousHttpGateway> CreateChunkedObliviousHttpGateway( - ObliviousHttpChunkHandler& chunk_handler) { + ObliviousHttpChunkHandler* chunk_handler) { // Private key from // https://www.ietf.org/archive/id/draft-ietf-ohai-chunked-ohttp-06.html#appendix-A-2 constexpr absl::string_view kX25519SecretKey = @@ -392,7 +392,7 @@ TestChunkHandler gateway_chunk_handler; absl::StatusOr<ChunkedObliviousHttpGateway> chunk_gateway = - CreateChunkedObliviousHttpGateway(gateway_chunk_handler); + CreateChunkedObliviousHttpGateway(&gateway_chunk_handler); QUICHE_ASSERT_OK(chunk_gateway); if (!chunk_gateway.ok()) { return; @@ -481,7 +481,7 @@ TestChunkHandler gateway_chunk_handler; absl::StatusOr<ChunkedObliviousHttpGateway> chunk_gateway = - CreateChunkedObliviousHttpGateway(gateway_chunk_handler); + CreateChunkedObliviousHttpGateway(&gateway_chunk_handler); QUICHE_ASSERT_OK(chunk_gateway); if (!chunk_gateway.ok()) { return; @@ -509,7 +509,7 @@ } TestChunkHandler gateway_chunk_handler; absl::StatusOr<ChunkedObliviousHttpGateway> chunk_gateway = - CreateChunkedObliviousHttpGateway(gateway_chunk_handler); + CreateChunkedObliviousHttpGateway(&gateway_chunk_handler); QUICHE_ASSERT_OK(chunk_gateway); if (!chunk_gateway.ok()) { return; @@ -552,7 +552,7 @@ } TestChunkHandler gateway_chunk_handler; absl::StatusOr<ChunkedObliviousHttpGateway> chunk_gateway = - CreateChunkedObliviousHttpGateway(gateway_chunk_handler); + CreateChunkedObliviousHttpGateway(&gateway_chunk_handler); QUICHE_ASSERT_OK(chunk_gateway); if (!chunk_gateway.ok()) { return; @@ -644,7 +644,7 @@ } TestChunkHandler gateway_chunk_handler; absl::StatusOr<ChunkedObliviousHttpGateway> chunk_gateway = - CreateChunkedObliviousHttpGateway(gateway_chunk_handler); + CreateChunkedObliviousHttpGateway(&gateway_chunk_handler); QUICHE_ASSERT_OK(chunk_gateway); if (!chunk_gateway.ok()) { return; @@ -684,7 +684,7 @@ } TestChunkHandler gateway_chunk_handler; absl::StatusOr<ChunkedObliviousHttpGateway> chunk_gateway = - CreateChunkedObliviousHttpGateway(gateway_chunk_handler); + CreateChunkedObliviousHttpGateway(&gateway_chunk_handler); QUICHE_ASSERT_OK(chunk_gateway); if (!chunk_gateway.ok()) { return; @@ -726,7 +726,7 @@ } TestChunkHandler gateway_chunk_handler; absl::StatusOr<ChunkedObliviousHttpGateway> chunk_gateway = - CreateChunkedObliviousHttpGateway(gateway_chunk_handler); + CreateChunkedObliviousHttpGateway(&gateway_chunk_handler); QUICHE_ASSERT_OK(chunk_gateway); if (!chunk_gateway.ok()) { return; @@ -768,7 +768,7 @@ } TestChunkHandler gateway_chunk_handler; absl::StatusOr<ChunkedObliviousHttpGateway> chunk_gateway = - CreateChunkedObliviousHttpGateway(gateway_chunk_handler); + CreateChunkedObliviousHttpGateway(&gateway_chunk_handler); QUICHE_ASSERT_OK(chunk_gateway); if (!chunk_gateway.ok()) { return; @@ -813,7 +813,7 @@ } TestChunkHandler gateway_chunk_handler; absl::StatusOr<ChunkedObliviousHttpGateway> chunk_gateway = - CreateChunkedObliviousHttpGateway(gateway_chunk_handler); + CreateChunkedObliviousHttpGateway(&gateway_chunk_handler); QUICHE_ASSERT_OK(chunk_gateway); if (!chunk_gateway.ok()) { return; @@ -852,7 +852,7 @@ } TestChunkHandler gateway_chunk_handler; absl::StatusOr<ChunkedObliviousHttpGateway> chunk_gateway = - CreateChunkedObliviousHttpGateway(gateway_chunk_handler); + CreateChunkedObliviousHttpGateway(&gateway_chunk_handler); QUICHE_ASSERT_OK(chunk_gateway); if (!chunk_gateway.ok()) { return;
diff --git a/quiche/oblivious_http/oblivious_http_gateway.cc b/quiche/oblivious_http/oblivious_http_gateway.cc index 8480a5f..fdd8a3d 100644 --- a/quiche/oblivious_http/oblivious_http_gateway.cc +++ b/quiche/oblivious_http/oblivious_http_gateway.cc
@@ -92,16 +92,19 @@ ChunkedObliviousHttpGateway::ChunkedObliviousHttpGateway( bssl::UniquePtr<EVP_HPKE_KEY> recipient_key, const ObliviousHttpHeaderKeyConfig& ohttp_key_config, - ObliviousHttpChunkHandler& chunk_handler, QuicheRandom* quiche_random) + ObliviousHttpChunkHandler* chunk_handler, QuicheRandom* quiche_random) : server_hpke_key_(std::move(recipient_key)), ohttp_key_config_(ohttp_key_config), - chunk_handler_(chunk_handler), + chunk_handler_(*chunk_handler), quiche_random_(quiche_random) {} absl::StatusOr<ChunkedObliviousHttpGateway> ChunkedObliviousHttpGateway::Create( absl::string_view hpke_private_key, const ObliviousHttpHeaderKeyConfig& ohttp_key_config, - ObliviousHttpChunkHandler& chunk_handler, QuicheRandom* quiche_random) { + ObliviousHttpChunkHandler* chunk_handler, QuicheRandom* quiche_random) { + if (chunk_handler == nullptr) { + return absl::InvalidArgumentError("Chunk handler is null"); + } QUICHE_ASSIGN_OR_RETURN( bssl::UniquePtr<EVP_HPKE_KEY> recipient_key, CreateServerRecipientKey(hpke_private_key, ohttp_key_config));
diff --git a/quiche/oblivious_http/oblivious_http_gateway.h b/quiche/oblivious_http/oblivious_http_gateway.h index 5023ebe..a7fbe6d 100644 --- a/quiche/oblivious_http/oblivious_http_gateway.h +++ b/quiche/oblivious_http/oblivious_http_gateway.h
@@ -1,12 +1,11 @@ #ifndef QUICHE_OBLIVIOUS_HTTP_OBLIVIOUS_HTTP_GATEWAY_H_ #define QUICHE_OBLIVIOUS_HTTP_OBLIVIOUS_HTTP_GATEWAY_H_ -#include <cmath> -#include <memory> #include <optional> #include <string> -#include <utility> +#include "absl/base/attributes.h" +#include "absl/base/nullability.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" @@ -96,14 +95,16 @@ class QUICHE_EXPORT ChunkedObliviousHttpGateway { public: // Creates a ChunkedObliviousHttpGateway. Like `ObliviousHttpGateway`, - // `hpke_private_key` must outlive the gateway. `quiche_random` can be - // initialized to nullptr, in which case the default - // `QuicheRandom::GetInstance()` will be used. + // `hpke_private_key` must outlive the gateway. `chunk_handler` must outlive + // the gateway. `quiche_random` can be initialized to nullptr, in which case + // the default `QuicheRandom::GetInstance()` will be used. static absl::StatusOr<ChunkedObliviousHttpGateway> Create( absl::string_view hpke_private_key, const ObliviousHttpHeaderKeyConfig& ohttp_key_config, - ObliviousHttpChunkHandler& chunk_handler, - QuicheRandom* quiche_random = nullptr); + ObliviousHttpChunkHandler* absl_nonnull chunk_handler + ABSL_ATTRIBUTE_LIFETIME_BOUND, + QuicheRandom* absl_nullable quiche_random ABSL_ATTRIBUTE_LIFETIME_BOUND = + nullptr); // only Movable (due to `UniquePtr server_hpke_key_`). ChunkedObliviousHttpGateway(ChunkedObliviousHttpGateway&& other) = default; @@ -141,7 +142,9 @@ explicit ChunkedObliviousHttpGateway( bssl::UniquePtr<EVP_HPKE_KEY> recipient_key, const ObliviousHttpHeaderKeyConfig& ohttp_key_config, - ObliviousHttpChunkHandler& chunk_handler, QuicheRandom* quiche_random); + ObliviousHttpChunkHandler* absl_nonnull chunk_handler + ABSL_ATTRIBUTE_LIFETIME_BOUND, + QuicheRandom* absl_nullable quiche_random ABSL_ATTRIBUTE_LIFETIME_BOUND); // Initializes the checkpoint with the provided data and any buffered data. void InitializeRequestCheckpoint(absl::string_view data); @@ -167,9 +170,10 @@ // public Key configuration. // https://www.rfc-editor.org/rfc/rfc9458.html#section-3 ObliviousHttpHeaderKeyConfig ohttp_key_config_; - // The handler to invoke when a chunk is decrypted successfully. + // The handler to invoke when a chunk is decrypted successfully. Not owned. ObliviousHttpChunkHandler& chunk_handler_; - QuicheRandom* quiche_random_; + // Not owned. + QuicheRandom* absl_nullable quiche_random_; std::string request_buffer_; // Tracks the remaining data to be processed or buffered.
diff --git a/quiche/oblivious_http/oblivious_http_gateway_test.cc b/quiche/oblivious_http/oblivious_http_gateway_test.cc index 731ccf7..2501052 100644 --- a/quiche/oblivious_http/oblivious_http_gateway_test.cc +++ b/quiche/oblivious_http/oblivious_http_gateway_test.cc
@@ -134,7 +134,7 @@ GetOhttpKeyConfig( /*key_id=*/1, EVP_HPKE_DHKEM_X25519_HKDF_SHA256, EVP_HPKE_HKDF_SHA256, EVP_HPKE_AES_128_GCM), - chunk_handler, quiche_random); + &chunk_handler, quiche_random); } TEST(ChunkedObliviousHttpGateway, ProvisionKeyAndDecapsulateFullRequest) { @@ -337,7 +337,7 @@ "Invalid HPKE key", GetOhttpKeyConfig(70, EVP_HPKE_DHKEM_X25519_HKDF_SHA256, EVP_HPKE_HKDF_SHA256, EVP_HPKE_AES_256_GCM), - chunk_handler) + &chunk_handler) .status() .code(), absl::StatusCode::kInternal); @@ -346,7 +346,7 @@ /*hpke_private_key*/ "", GetOhttpKeyConfig(70, EVP_HPKE_DHKEM_X25519_HKDF_SHA256, EVP_HPKE_HKDF_SHA256, EVP_HPKE_AES_256_GCM), - chunk_handler) + &chunk_handler) .status() .code(), absl::StatusCode::kInvalidArgument);