Adds handling of multiple Content-Length values. According to RFC 7230 section 3.3.2, endpoints receiving a message with multiple Content-Length fields must either reject the message or replace the header field with a single value. PiperOrigin-RevId: 447759063
diff --git a/quiche/http2/adapter/header_validator.cc b/quiche/http2/adapter/header_validator.cc index cfd14f1..2cc5410 100644 --- a/quiche/http2/adapter/header_validator.cc +++ b/quiche/http2/adapter/header_validator.cc
@@ -194,9 +194,16 @@ } } } else if (key == "content-length") { - const bool success = HandleContentLength(value); - if (!success) { - return HEADER_FIELD_INVALID; + const ContentLengthStatus status = HandleContentLength(value); + switch (status) { + case CONTENT_LENGTH_ERROR: + return HEADER_FIELD_INVALID; + case CONTENT_LENGTH_SKIP: + return HEADER_SKIP; + case CONTENT_LENGTH_OK: + return HEADER_OK; + default: + return HEADER_FIELD_INVALID; } } else if (key == "te" && value != "trailers") { return HEADER_FIELD_INVALID; @@ -227,28 +234,33 @@ return false; } -bool HeaderValidator::HandleContentLength(absl::string_view value) { +HeaderValidator::ContentLengthStatus HeaderValidator::HandleContentLength( + absl::string_view value) { if (value.empty()) { - return false; + return CONTENT_LENGTH_ERROR; } if (status_ == "204" && value != "0") { // There should be no body in a "204 No Content" response. - return false; + return CONTENT_LENGTH_ERROR; } if (!status_.empty() && status_[0] == '1' && value != "0") { // There should also be no body in a 1xx response. - return false; + return CONTENT_LENGTH_ERROR; } size_t content_length = 0; const bool valid = absl::SimpleAtoi(value, &content_length); if (!valid) { - return false; + return CONTENT_LENGTH_ERROR; } + if (content_length_.has_value()) { + return content_length == content_length_.value() ? CONTENT_LENGTH_SKIP + : CONTENT_LENGTH_ERROR; + } content_length_ = content_length; - return true; + return CONTENT_LENGTH_OK; } // Returns whether `authority` contains only characters from the `host` ABNF
diff --git a/quiche/http2/adapter/header_validator.h b/quiche/http2/adapter/header_validator.h index c037783..aba6bec 100644 --- a/quiche/http2/adapter/header_validator.h +++ b/quiche/http2/adapter/header_validator.h
@@ -39,6 +39,7 @@ enum HeaderStatus { HEADER_OK, + HEADER_SKIP, HEADER_FIELD_INVALID, HEADER_FIELD_TOO_LONG, }; @@ -55,7 +56,12 @@ absl::optional<size_t> content_length() const { return content_length_; } private: - bool HandleContentLength(absl::string_view value); + enum ContentLengthStatus { + CONTENT_LENGTH_OK, + CONTENT_LENGTH_SKIP, // Used to handle duplicate content length values. + CONTENT_LENGTH_ERROR, + }; + ContentLengthStatus HandleContentLength(absl::string_view value); bool ValidateAndSetAuthority(absl::string_view authority); std::vector<std::string> pseudo_headers_;
diff --git a/quiche/http2/adapter/header_validator_test.cc b/quiche/http2/adapter/header_validator_test.cc index 444b700..d49566a 100644 --- a/quiche/http2/adapter/header_validator_test.cc +++ b/quiche/http2/adapter/header_validator_test.cc
@@ -428,6 +428,30 @@ EXPECT_TRUE(v.FinishHeaderBlock(HeaderType::RESPONSE)); } +TEST(HeaderValidatorTest, ResponseWithMultipleIdenticalContentLength) { + HeaderValidator v; + + v.StartHeaderBlock(); + EXPECT_EQ(HeaderValidator::HEADER_OK, + v.ValidateSingleHeader(":status", "200")); + EXPECT_EQ(HeaderValidator::HEADER_OK, + v.ValidateSingleHeader("content-length", "13")); + EXPECT_EQ(HeaderValidator::HEADER_SKIP, + v.ValidateSingleHeader("content-length", "13")); +} + +TEST(HeaderValidatorTest, ResponseWithMultipleDifferingContentLength) { + HeaderValidator v; + + v.StartHeaderBlock(); + EXPECT_EQ(HeaderValidator::HEADER_OK, + v.ValidateSingleHeader(":status", "200")); + EXPECT_EQ(HeaderValidator::HEADER_OK, + v.ValidateSingleHeader("content-length", "13")); + EXPECT_EQ(HeaderValidator::HEADER_FIELD_INVALID, + v.ValidateSingleHeader("content-length", "17")); +} + TEST(HeaderValidatorTest, Response204WithContentLengthZero) { HeaderValidator v;
diff --git a/quiche/http2/adapter/nghttp2_adapter_test.cc b/quiche/http2/adapter/nghttp2_adapter_test.cc index 5f84e15..9a61424 100644 --- a/quiche/http2/adapter/nghttp2_adapter_test.cc +++ b/quiche/http2/adapter/nghttp2_adapter_test.cc
@@ -5281,6 +5281,72 @@ EXPECT_FALSE(adapter->want_write()); } +TEST(NgHttp2AdapterTest, ServerHandlesMultipleContentLength) { + DataSavingVisitor visitor; + auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor); + EXPECT_FALSE(adapter->want_write()); + + const std::string frames = TestFrameSequence() + .ClientPreface() + .Headers(1, + {{":method", "POST"}, + {":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/1"}, + {"content-length", "7"}, + {"content-length", "7"}}, + /*fin=*/false) + .Headers(3, + {{":method", "POST"}, + {":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/3"}, + {"content-length", "11"}, + {"content-length", "13"}}, + /*fin=*/false) + .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, 4)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(1)); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "POST")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/1")); + EXPECT_CALL(visitor, OnHeaderForStream(1, "content-length", "7")); + // nghttp2 does not like duplicate Content-Length headers. + EXPECT_CALL( + visitor, + OnErrorDebug("Invalid HTTP header field was received: frame type: 1, " + "stream: 1, name: [content-length], value: [7]")); + EXPECT_CALL( + visitor, + OnInvalidFrame(1, Http2VisitorInterface::InvalidFrameError::kHttpHeader)); + // Stream 3 + EXPECT_CALL(visitor, OnFrameHeader(3, _, HEADERS, 4)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(3)); + EXPECT_CALL(visitor, OnHeaderForStream(3, ":method", "POST")); + EXPECT_CALL(visitor, OnHeaderForStream(3, ":scheme", "https")); + EXPECT_CALL(visitor, OnHeaderForStream(3, ":authority", "example.com")); + EXPECT_CALL(visitor, OnHeaderForStream(3, ":path", "/3")); + EXPECT_CALL(visitor, OnHeaderForStream(3, "content-length", "11")); + EXPECT_CALL( + visitor, + OnErrorDebug("Invalid HTTP header field was received: frame type: 1, " + "stream: 3, name: [content-length], value: [13]")); + EXPECT_CALL( + visitor, + OnInvalidFrame(3, Http2VisitorInterface::InvalidFrameError::kHttpHeader)); + + const int64_t result = adapter->ProcessBytes(frames); + EXPECT_EQ(frames.size(), static_cast<size_t>(result)); +} + TEST(NgHttp2AdapterTest, ServerSendsInvalidTrailers) { DataSavingVisitor visitor; auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
diff --git a/quiche/http2/adapter/oghttp2_adapter_test.cc b/quiche/http2/adapter/oghttp2_adapter_test.cc index 3acba23..048ea93 100644 --- a/quiche/http2/adapter/oghttp2_adapter_test.cc +++ b/quiche/http2/adapter/oghttp2_adapter_test.cc
@@ -4531,6 +4531,63 @@ EXPECT_FALSE(adapter->want_write()); } +TEST(OgHttp2AdapterTest, ServerHandlesMultipleContentLength) { + DataSavingVisitor visitor; + OgHttp2Adapter::Options 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", "POST"}, + {":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/1"}, + {"content-length", "7"}, + {"content-length", "7"}}, + /*fin=*/false) + .Headers(3, + {{":method", "POST"}, + {":scheme", "https"}, + {":authority", "example.com"}, + {":path", "/3"}, + {"content-length", "11"}, + {"content-length", "13"}}, + /*fin=*/false) + .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, 4)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(1)); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "POST")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com")); + EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/1")); + EXPECT_CALL(visitor, OnHeaderForStream(1, "content-length", "7")); + EXPECT_CALL(visitor, OnEndHeadersForStream(1)); + // Stream 3 + EXPECT_CALL(visitor, OnFrameHeader(3, _, HEADERS, 4)); + EXPECT_CALL(visitor, OnBeginHeadersForStream(3)); + EXPECT_CALL(visitor, OnHeaderForStream(3, ":method", "POST")); + EXPECT_CALL(visitor, OnHeaderForStream(3, ":scheme", "https")); + EXPECT_CALL(visitor, OnHeaderForStream(3, ":authority", "example.com")); + EXPECT_CALL(visitor, OnHeaderForStream(3, ":path", "/3")); + EXPECT_CALL(visitor, OnHeaderForStream(3, "content-length", "11")); + EXPECT_CALL( + visitor, + OnInvalidFrame(3, Http2VisitorInterface::InvalidFrameError::kHttpHeader)); + + const int64_t result = adapter->ProcessBytes(frames); + EXPECT_EQ(frames.size(), static_cast<size_t>(result)); +} + TEST(OgHttp2AdapterTest, ServerSendsInvalidTrailers) { DataSavingVisitor visitor; OgHttp2Adapter::Options options;
diff --git a/quiche/http2/adapter/oghttp2_session.cc b/quiche/http2/adapter/oghttp2_session.cc index 07d88ab..c53c8f2 100644 --- a/quiche/http2/adapter/oghttp2_session.cc +++ b/quiche/http2/adapter/oghttp2_session.cc
@@ -231,6 +231,7 @@ HeaderValidator::HeaderStatus status) { switch (status) { case HeaderValidator::HEADER_OK: + case HeaderValidator::HEADER_SKIP: return Http2VisitorInterface::HEADER_OK; case HeaderValidator::HEADER_FIELD_INVALID: return Http2VisitorInterface::HEADER_FIELD_INVALID; @@ -248,6 +249,9 @@ } const HeaderValidator::HeaderStatus validation_result = validator_.ValidateSingleHeader(key, value); + if (validation_result == HeaderValidator::HEADER_SKIP) { + return; + } if (validation_result != HeaderValidator::HEADER_OK) { QUICHE_VLOG(2) << "Header validation failed with result " << static_cast<int>(validation_result);