Skip to content

Commit d556e8e

Browse files
reubent-googcopybara-github
authored andcommitted
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
1 parent a6acb06 commit d556e8e

File tree

2 files changed

+35
-5
lines changed

2 files changed

+35
-5
lines changed

quiche/balsa/balsa_frame.cc

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -797,14 +797,30 @@ void BalsaFrame::ProcessHeaderLines(const Lines& lines, bool is_trailer,
797797
content_length_remaining_ = length;
798798
continue;
799799
}
800-
if ((headers->content_length_status_ != content_length_status) ||
801-
((headers->content_length_status_ ==
802-
BalsaHeadersEnums::VALID_CONTENT_LENGTH) &&
803-
(http_validation_policy().disallow_multiple_content_length ||
804-
length != headers->content_length_))) {
800+
801+
// There are multiple content-length keys in this request.
802+
803+
// First case: the second CL header conflicts with the first CL. This
804+
// request _must_ be rejected.
805+
bool found_new_invalid_content_length =
806+
headers->content_length_status_ != content_length_status;
807+
bool found_new_content_length_with_different_value =
808+
length != headers->content_length_;
809+
if (found_new_invalid_content_length ||
810+
found_new_content_length_with_different_value) {
805811
HandleError(BalsaFrameEnums::MULTIPLE_CONTENT_LENGTH_KEYS);
806812
return;
807813
}
814+
815+
// Second case: CL is duplicated but the value is valid and the same.
816+
// Optionally, reject this per the RFC or simply keep one value.
817+
if (headers->content_length_status_ ==
818+
BalsaHeadersEnums::VALID_CONTENT_LENGTH) {
819+
if (http_validation_policy().disallow_multiple_content_length) {
820+
HandleError(BalsaFrameEnums::MULTIPLE_CONTENT_LENGTH_KEYS);
821+
return;
822+
}
823+
}
808824
continue;
809825
}
810826
if (absl::EqualsIgnoreCase(key, kTransferEncoding)) {

quiche/balsa/balsa_frame_test.cc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3622,6 +3622,19 @@ TEST_F(HTTPBalsaFrameTest, ReadUntilCloseStateEnteredAsExpectedAndNotExited) {
36223622
}
36233623
}
36243624

3625+
TEST_F(HTTPBalsaFrameTest, TwoContentLengthFirstValidSecondOverflow) {
3626+
std::string header =
3627+
"HTTP/1.1 200 OK\r\n"
3628+
"content-length: 12\r\n"
3629+
"content-length: 9223372036854775807999999\r\n"
3630+
"\r\n";
3631+
balsa_frame_.set_is_request(false);
3632+
balsa_frame_.ProcessInput(header.data(), header.size());
3633+
EXPECT_TRUE(balsa_frame_.Error());
3634+
EXPECT_EQ(BalsaFrameEnums::MULTIPLE_CONTENT_LENGTH_KEYS,
3635+
balsa_frame_.ErrorCode());
3636+
}
3637+
36253638
TEST_F(HTTPBalsaFrameTest, TwoDifferentContentLengthHeadersIsAnError) {
36263639
std::string header =
36273640
"HTTP/1.1 200 OK\r\n"
@@ -3649,6 +3662,7 @@ TEST_F(HTTPBalsaFrameTest, TwoSameContentLengthHeadersIsNotAnError) {
36493662
EXPECT_EQ(BalsaFrameEnums::BALSA_NO_ERROR, balsa_frame_.ErrorCode());
36503663
EXPECT_FALSE(balsa_frame_.Error());
36513664
EXPECT_TRUE(balsa_frame_.MessageFullyRead());
3665+
EXPECT_THAT(headers_.GetAllOfHeaderAsString("Content-Length"), "1,1");
36523666
}
36533667

36543668
TEST_F(HTTPBalsaFrameTest, TwoSameContentLengthHeadersIsAnError) {

0 commit comments

Comments
 (0)