Pure refactor of `MULTIPLE_CONTENT_LENGTH_KEYS` logic Make the cases that can lead to this error more clear since we'll be changing one of them in a follow up CL. Also add a test case for when we already received a valid content-length but encounter an invalid one (it fails with MULTIPLE_CONTENT_LENGTH_KEYS instead of the overflow). Also delete some repetitive invocations in test that don't add any value. Protected by pure refactor. PiperOrigin-RevId: 875760693
diff --git a/quiche/balsa/balsa_frame.cc b/quiche/balsa/balsa_frame.cc index 80baeb1..ded79f7 100644 --- a/quiche/balsa/balsa_frame.cc +++ b/quiche/balsa/balsa_frame.cc
@@ -797,14 +797,30 @@ content_length_remaining_ = length; continue; } - if ((headers->content_length_status_ != content_length_status) || - ((headers->content_length_status_ == - BalsaHeadersEnums::VALID_CONTENT_LENGTH) && - (http_validation_policy().disallow_multiple_content_length || - length != headers->content_length_))) { + + // There are multiple content-length keys in this request. + + // First case: the second CL header conflicts with the first CL. This + // request _must_ be rejected. + bool found_new_invalid_content_length = + headers->content_length_status_ != content_length_status; + bool found_new_content_length_with_different_value = + length != headers->content_length_; + if (found_new_invalid_content_length || + found_new_content_length_with_different_value) { HandleError(BalsaFrameEnums::MULTIPLE_CONTENT_LENGTH_KEYS); return; } + + // Second case: CL is duplicated but the value is valid and the same. + // Optionally, reject this per the RFC or simply keep one value. + if (headers->content_length_status_ == + BalsaHeadersEnums::VALID_CONTENT_LENGTH) { + if (http_validation_policy().disallow_multiple_content_length) { + HandleError(BalsaFrameEnums::MULTIPLE_CONTENT_LENGTH_KEYS); + return; + } + } continue; } if (absl::EqualsIgnoreCase(key, kTransferEncoding)) {
diff --git a/quiche/balsa/balsa_frame_test.cc b/quiche/balsa/balsa_frame_test.cc index c1bf58c..65db151 100644 --- a/quiche/balsa/balsa_frame_test.cc +++ b/quiche/balsa/balsa_frame_test.cc
@@ -3622,6 +3622,19 @@ } } +TEST_F(HTTPBalsaFrameTest, TwoContentLengthFirstValidSecondOverflow) { + std::string header = + "HTTP/1.1 200 OK\r\n" + "content-length: 12\r\n" + "content-length: 9223372036854775807999999\r\n" + "\r\n"; + balsa_frame_.set_is_request(false); + balsa_frame_.ProcessInput(header.data(), header.size()); + EXPECT_TRUE(balsa_frame_.Error()); + EXPECT_EQ(BalsaFrameEnums::MULTIPLE_CONTENT_LENGTH_KEYS, + balsa_frame_.ErrorCode()); +} + TEST_F(HTTPBalsaFrameTest, TwoDifferentContentLengthHeadersIsAnError) { std::string header = "HTTP/1.1 200 OK\r\n" @@ -3649,6 +3662,7 @@ EXPECT_EQ(BalsaFrameEnums::BALSA_NO_ERROR, balsa_frame_.ErrorCode()); EXPECT_FALSE(balsa_frame_.Error()); EXPECT_TRUE(balsa_frame_.MessageFullyRead()); + EXPECT_THAT(headers_.GetAllOfHeaderAsString("Content-Length"), "1,1"); } TEST_F(HTTPBalsaFrameTest, TwoSameContentLengthHeadersIsAnError) {