Adds a test case where the user attempts to call SubmitTrailers() after the data frame source for a stream returned an error. PiperOrigin-RevId: 401851060
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc index 62f3682..b78827a 100644 --- a/http2/adapter/nghttp2_adapter_test.cc +++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -2223,6 +2223,70 @@ absl::StrJoin(visitor.GetMetadata(1), "")); } +TEST(NgHttp2AdapterTest, ServerSubmitsResponseWithDataSourceError) { + DataSavingVisitor visitor; + auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor); + EXPECT_FALSE(adapter->want_write()); + + const std::string frames = TestFrameSequence() + .ClientPreface() + .Headers(1, + {{":method", "GET"}, + {":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/this/is/request/one"}}, + /*fin=*/true) + .Serialize(); + testing::InSequence s; + + // Client preface (empty SETTINGS) + EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL(visitor, OnSettingsEnd()); + // Stream 1 + EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(1)); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "GET")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/this/is/request/one")); + EXPECT_CALL(visitor, OnEndHeadersForStream(1)); + EXPECT_CALL(visitor, OnEndStream(1)); + + const int64_t result = adapter->ProcessBytes(frames); + EXPECT_EQ(frames.size(), static_cast<size_t>(result)); + + auto body1 = absl::make_unique<TestDataFrameSource>(visitor, false); + body1->SimulateError(); + int submit_result = adapter->SubmitResponse( + 1, ToHeaders({{":status", "200"}, {"x-comment", "Sure, sounds good."}}), + std::move(body1)); + EXPECT_EQ(submit_result, 0); + EXPECT_TRUE(adapter->want_write()); + + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, 1, _, 0x4)); + EXPECT_CALL(visitor, OnFrameSent(HEADERS, 1, _, 0x4, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 1, _, 0x0)); + EXPECT_CALL(visitor, OnFrameSent(RST_STREAM, 1, _, 0x0, 2)); + EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::INTERNAL_ERROR)); + + int send_result = adapter->Send(); + EXPECT_EQ(0, send_result); + EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS, + spdy::SpdyFrameType::HEADERS, + spdy::SpdyFrameType::RST_STREAM})); + visitor.Clear(); + EXPECT_FALSE(adapter->want_write()); + + int trailer_result = + adapter->SubmitTrailer(1, ToHeaders({{":final-status", "a-ok"}})); + // The library does not object to the user queuing trailers, even through the + // stream has already been closed. + EXPECT_EQ(trailer_result, 0); +} + TEST(NgHttp2AdapterTest, ServerSendsInvalidTrailers) { DataSavingVisitor visitor; auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc index 4731e48..38ab9fb 100644 --- a/http2/adapter/oghttp2_adapter_test.cc +++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -1085,6 +1085,73 @@ absl::StrJoin(visitor.GetMetadata(1), "")); } +TEST(OgHttp2AdapterServerTest, ServerSubmitsResponseWithDataSourceError) { + DataSavingVisitor visitor; + OgHttp2Adapter::Options options{.perspective = Perspective::kServer}; + auto adapter = OgHttp2Adapter::Create(visitor, options); + EXPECT_FALSE(adapter->want_write()); + + const std::string frames = TestFrameSequence() + .ClientPreface() + .Headers(1, + {{":method", "GET"}, + {":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/this/is/request/one"}}, + /*fin=*/true) + .Serialize(); + testing::InSequence s; + + // Client preface (empty SETTINGS) + EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL(visitor, OnSettingsEnd()); + // Stream 1 + EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(1)); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "GET")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/this/is/request/one")); + EXPECT_CALL(visitor, OnEndHeadersForStream(1)); + EXPECT_CALL(visitor, OnEndStream(1)); + + const int64_t result = adapter->ProcessBytes(frames); + EXPECT_EQ(frames.size(), static_cast<size_t>(result)); + + auto body1 = absl::make_unique<TestDataFrameSource>(visitor, false); + body1->SimulateError(); + int submit_result = adapter->SubmitResponse( + 1, ToHeaders({{":status", "200"}, {"x-comment", "Sure, sounds good."}}), + std::move(body1)); + EXPECT_EQ(submit_result, 0); + EXPECT_TRUE(adapter->want_write()); + + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0)); + 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(HEADERS, 1, _, 0x4)); + EXPECT_CALL(visitor, OnFrameSent(HEADERS, 1, _, 0x4, 0)); + // TODO(birenroy): Send RST_STREAM INTERNAL_ERROR to the client as well. + EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::INTERNAL_ERROR)); + + int send_result = adapter->Send(); + EXPECT_EQ(0, send_result); + EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS, + spdy::SpdyFrameType::SETTINGS, + spdy::SpdyFrameType::HEADERS})); + visitor.Clear(); + EXPECT_FALSE(adapter->want_write()); + + // Since the stream has been closed, it is not possible to submit trailers for + // the stream. + int trailer_result = + adapter->SubmitTrailer(1, ToHeaders({{":final-status", "a-ok"}})); + ASSERT_LT(trailer_result, 0); + EXPECT_FALSE(adapter->want_write()); +} + TEST(OgHttp2AdapterServerTest, ServerSendsInvalidTrailers) { DataSavingVisitor visitor; OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
diff --git a/http2/adapter/test_utils.cc b/http2/adapter/test_utils.cc index 80e113e..4223b49 100644 --- a/http2/adapter/test_utils.cc +++ b/http2/adapter/test_utils.cc
@@ -27,6 +27,9 @@ std::pair<int64_t, bool> TestDataFrameSource::SelectPayloadLength( size_t max_length) { + if (return_error_) { + return {DataFrameSource::kError, false}; + } // The stream is done if there's no more data, or if |max_length| is at least // as large as the remaining data. const bool end_data = end_data_ && (current_fragment_.empty() ||
diff --git a/http2/adapter/test_utils.h b/http2/adapter/test_utils.h index ec7e95c..886d0bf 100644 --- a/http2/adapter/test_utils.h +++ b/http2/adapter/test_utils.h
@@ -83,6 +83,7 @@ void AppendPayload(absl::string_view payload); void EndData(); + void SimulateError() { return_error_ = true; } std::pair<int64_t, bool> SelectPayloadLength(size_t max_length) override; bool Send(absl::string_view frame_header, size_t payload_length) override; @@ -96,6 +97,8 @@ const bool has_fin_; // Whether |payload_fragments_| contains the final segment of data. bool end_data_ = false; + // Whether SelectPayloadLength() should return an error. + bool return_error_ = false; }; class QUICHE_NO_EXPORT TestMetadataSource : public MetadataSource {