Avoid validating content-length values for 304 responses in oghttp2. According to https://httpwg.org/specs/rfc7230.html#rfc.section.3.3.2, a server may send the content-length header in a 304 response, which does not have a body. The content-length header in this case indicates the body length the server would have sent in the corresponding 200 response. Therefore, this CL updates oghttp2 to not check for received content-length header value mismatch in 304 responses. This update is important for several integration test cases. PiperOrigin-RevId: 422905007
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc index 206d8e2..ff92a65 100644 --- a/http2/adapter/nghttp2_adapter_test.cc +++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -390,6 +390,60 @@ spdy::SpdyFrameType::RST_STREAM})); } +TEST(NgHttp2AdapterTest, ClientHandles304WithContentLength) { + DataSavingVisitor visitor; + auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor); + + testing::InSequence s; + + const std::vector<Header> headers = + ToHeaders({{":method", "GET"}, + {":scheme", "http"}, + {":authority", "example.com"}, + {":path", "/this/is/request/one"}}); + + const int32_t stream_id = adapter->SubmitRequest(headers, nullptr, nullptr); + ASSERT_GT(stream_id, 0); + + EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id, _, 0x5)); + EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id, _, 0x5, 0)); + + int result = adapter->Send(); + EXPECT_EQ(0, result); + visitor.Clear(); + + const std::string stream_frames = + TestFrameSequence() + .ServerPreface() + .Headers(1, {{":status", "304"}, {"content-length", "2"}}, + /*fin=*/true) + .Serialize(); + + // Server preface (empty SETTINGS) + EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL(visitor, OnSettingsEnd()); + + EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(1)); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":status", "304")); + EXPECT_CALL(visitor, OnHeaderForStream(1, "content-length", "2")); + EXPECT_CALL(visitor, OnEndHeadersForStream(1)); + EXPECT_CALL(visitor, OnEndStream(1)); + EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR)); + + const int64_t stream_result = adapter->ProcessBytes(stream_frames); + EXPECT_EQ(stream_frames.size(), static_cast<size_t>(stream_result)); + + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0)); + + EXPECT_TRUE(adapter->want_write()); + result = adapter->Send(); + EXPECT_EQ(0, result); + EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS})); +} + TEST(NgHttp2AdapterTest, ClientHandlesTrailers) { DataSavingVisitor visitor; auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc index d3f235d..ee67139 100644 --- a/http2/adapter/oghttp2_adapter_test.cc +++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -469,6 +469,63 @@ spdy::SpdyFrameType::RST_STREAM})); } +TEST(OgHttp2AdapterClientTest, ClientHandles304WithContentLength) { + DataSavingVisitor visitor; + OgHttp2Adapter::Options options{.perspective = Perspective::kClient}; + auto adapter = OgHttp2Adapter::Create(visitor, options); + + testing::InSequence s; + + const std::vector<Header> headers = + ToHeaders({{":method", "GET"}, + {":scheme", "http"}, + {":authority", "example.com"}, + {":path", "/this/is/request/one"}}); + + const int32_t stream_id = adapter->SubmitRequest(headers, nullptr, nullptr); + ASSERT_GT(stream_id, 0); + + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0)); + EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id, _, 0x5)); + EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id, _, 0x5, 0)); + + int result = adapter->Send(); + EXPECT_EQ(0, result); + visitor.Clear(); + + const std::string stream_frames = + TestFrameSequence() + .ServerPreface() + .Headers(1, {{":status", "304"}, {"content-length", "2"}}, + /*fin=*/true) + .Serialize(); + + // Server preface (empty SETTINGS) + EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0)); + EXPECT_CALL(visitor, OnSettingsStart()); + EXPECT_CALL(visitor, OnSettingsEnd()); + + EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(1)); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":status", "304")); + EXPECT_CALL(visitor, OnHeaderForStream(1, "content-length", "2")); + EXPECT_CALL(visitor, OnEndHeadersForStream(1)); + EXPECT_CALL(visitor, OnEndStream(1)); + EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::HTTP2_NO_ERROR)); + + const int64_t stream_result = adapter->ProcessBytes(stream_frames); + EXPECT_EQ(stream_frames.size(), static_cast<size_t>(stream_result)); + + EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1)); + EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0)); + + EXPECT_TRUE(adapter->want_write()); + result = adapter->Send(); + EXPECT_EQ(0, result); + EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS})); +} + TEST(OgHttp2AdapterClientTest, ClientHandlesTrailers) { DataSavingVisitor visitor; OgHttp2Adapter::Options options{.perspective = Perspective::kClient};
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc index 4c14b1b..547ea15 100644 --- a/http2/adapter/oghttp2_session.cc +++ b/http2/adapter/oghttp2_session.cc
@@ -1119,7 +1119,10 @@ it->second.received_header_type = headers_handler_.header_type(); } if (headers_handler_.header_type() == HeaderType::REQUEST || - headers_handler_.header_type() == HeaderType::RESPONSE) { + (headers_handler_.header_type() == HeaderType::RESPONSE && + headers_handler_.status_header() != "304")) { + // 304 response content-length values should be ignored: + // https://httpwg.org/specs/rfc7230.html#rfc.section.3.3.2 it->second.remaining_content_length = headers_handler_.content_length(); } headers_handler_.set_stream_id(0);