[balsa] Disallow multiple Content-Length headers, even if identical. (e…

…nvoyproxy#28931) Signed-off-by: Bence Béky <[email protected]>
ohadvano · Aug 12, 2023 · 5fde158 · 5fde158
1 parent 53a1cc2
commit 5fde158
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 1 deletion.
diff --git a/source/common/http/http1/balsa_parser.cc b/source/common/http/http1/balsa_parser.cc
@@ -151,7 +151,7 @@ BalsaParser::BalsaParser(MessageType type, ParserCallbacks* connection, size_t m
   quiche::HttpValidationPolicy http_validation_policy;
   http_validation_policy.disallow_header_continuation_lines = true;
   http_validation_policy.require_header_colon = true;
-  http_validation_policy.disallow_multiple_content_length = false;
+  http_validation_policy.disallow_multiple_content_length = true;
   http_validation_policy.disallow_transfer_encoding_with_content_length = false;
   http_validation_policy.validate_transfer_encoding = false;
   http_validation_policy.require_content_length_if_body_required = false;
@@ -378,6 +378,9 @@ void BalsaParser::HandleError(BalsaFrameEnums::ErrorCode error_code) {
   case BalsaFrameEnums::INVALID_HEADER_CHARACTER:
     error_message_ = "header value contains invalid chars";
     break;
+  case BalsaFrameEnums::MULTIPLE_CONTENT_LENGTH_KEYS:
+    error_message_ = "HPE_UNEXPECTED_CONTENT_LENGTH";
+    break;
   default:
     error_message_ = BalsaFrameEnums::ErrorCodeToString(error_code);
   }

diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc
@@ -4822,5 +4822,41 @@ TEST_P(Http1ServerConnectionImplTest, Char22InHeaderValue) {
   EXPECT_EQ(status.message(), "http/1.1 protocol error: header value contains invalid chars");
 }
 
+TEST_P(Http1ClientConnectionImplTest, MultipleContentLength) {
+  initialize();
+
+  NiceMock<MockResponseDecoder> response_decoder;
+  Http::RequestEncoder& request_encoder = codec_->newStream(response_decoder);
+  TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/"}, {":authority", "host"}};
+  EXPECT_TRUE(request_encoder.encodeHeaders(headers, true).ok());
+
+  Buffer::OwnedImpl response("HTTP/1.1 200 OK\r\n"
+                             "Content-Length: 3\r\n"
+                             "Content-Length: 3\r\n"
+                             "\r\n"
+                             "foo\r\n\r\n");
+  auto status = codec_->dispatch(response);
+  EXPECT_FALSE(status.ok());
+  EXPECT_EQ(status.message(), "http/1.1 protocol error: HPE_UNEXPECTED_CONTENT_LENGTH");
+}
+
+TEST_P(Http1ServerConnectionImplTest, MultipleContentLength) {
+  initialize();
+
+  StrictMock<MockRequestDecoder> decoder;
+  EXPECT_CALL(callbacks_, newStream(_, _)).WillOnce(ReturnRef(decoder));
+  EXPECT_CALL(decoder,
+              sendLocalReply(Http::Code::BadRequest, "Bad Request", _, _, "http1.codec_error"));
+
+  Buffer::OwnedImpl buffer("GET / HTTP/1.1\r\n"
+                           "Content-Length: 3\r\n"
+                           "Content-Length: 3\r\n"
+                           "\r\n"
+                           "foo\r\n\r\n");
+  auto status = codec_->dispatch(buffer);
+  EXPECT_FALSE(status.ok());
+  EXPECT_EQ(status.message(), "http/1.1 protocol error: HPE_UNEXPECTED_CONTENT_LENGTH");
+}
+
 } // namespace Http
 } // namespace Envoy