Remove BalsaFrame::set_balsa_trailer() and BalsaFrame::trailer(). This CL removes BalsaFrame::set_balsa_trailer(), which now has no remaining usages, in favor of BalsaFrame::EnableTrailers(). This CL also removes BalsaFrame::trailer(), which now has no remaining usages. This CL will enable the removal of BalsaVisitorInterface::ProcessTrailers(). Thanks to ++bnc@ for migrating BalsaParser in Envoy to the EnableTrailers() API (e.g., [1, 2]), which was also helpful/important for enabling the deprecation of set_balsa_trailer(). [1] http://google3/third_party/envoy/src/source/common/http/http1/balsa_parser.h;l=48;rcl=581237808 [2] http://google3/third_party/envoy/src/source/common/http/http1/balsa_parser.cc;l=169,282;rcl=581237808 PiperOrigin-RevId: 590662859
diff --git a/quiche/balsa/balsa_frame.cc b/quiche/balsa/balsa_frame.cc index 8263b35..a1d04b6 100644 --- a/quiche/balsa/balsa_frame.cc +++ b/quiche/balsa/balsa_frame.cc
@@ -82,9 +82,6 @@ trailer_lines_.clear(); start_of_trailer_line_ = 0; trailer_length_ = 0; - if (trailer_ != nullptr) { - trailer_->Clear(); - } if (trailers_ != nullptr) { trailers_->Clear(); } @@ -1254,7 +1251,7 @@ const char c = *current; ++current; ++trailer_length_; - if (GetTrailers() != nullptr) { + if (trailers_ != nullptr) { // Reuse the header length limit for trailer, which is just a bunch // of headers. if (trailer_length_ > max_header_length_) { @@ -1270,22 +1267,19 @@ } if (HeaderFramingFound(c) != 0) { parse_state_ = BalsaFrameEnums::MESSAGE_FULLY_READ; - if (BalsaHeaders* trailers = GetTrailers(); trailers != nullptr) { - trailers->WriteFromFramer(on_entry, current - on_entry); - trailers->DoneWritingFromFramer(); - ProcessHeaderLines(trailer_lines_, true /*is_trailer*/, trailers); + if (trailers_ != nullptr) { + trailers_->WriteFromFramer(on_entry, current - on_entry); + trailers_->DoneWritingFromFramer(); + ProcessHeaderLines(trailer_lines_, true /*is_trailer*/, + trailers_.get()); if (parse_state_ == BalsaFrameEnums::ERROR) { return current - input; } - if (trailers_ != nullptr) { - visitor_->OnTrailers(std::move(trailers_)); + visitor_->OnTrailers(std::move(trailers_)); - // Allows trailers to be delivered without another call to - // EnableTrailers() in case the framer is Reset(). - trailers_ = std::make_unique<BalsaHeaders>(); - } else { - visitor_->ProcessTrailers(*trailer_); - } + // Allows trailers to be delivered without another call to + // EnableTrailers() in case the framer is Reset(). + trailers_ = std::make_unique<BalsaHeaders>(); } visitor_->OnTrailerInput( absl::string_view(on_entry, current - on_entry)); @@ -1293,8 +1287,8 @@ return current - input; } } - if (BalsaHeaders* trailers = GetTrailers(); trailers != nullptr) { - trailers->WriteFromFramer(on_entry, current - on_entry); + if (trailers_ != nullptr) { + trailers_->WriteFromFramer(on_entry, current - on_entry); } visitor_->OnTrailerInput( absl::string_view(on_entry, current - on_entry)); @@ -1364,13 +1358,6 @@ HandleError(BalsaFrameEnums::HEADERS_TOO_LONG); } -BalsaHeaders* BalsaFrame::GetTrailers() const { - if (trailers_ != nullptr) { - return trailers_.get(); - } - return trailer_; -} - const int32_t BalsaFrame::kValidTerm1; const int32_t BalsaFrame::kValidTerm1Mask; const int32_t BalsaFrame::kValidTerm2;
diff --git a/quiche/balsa/balsa_frame.h b/quiche/balsa/balsa_frame.h index ad42ca2..d50fd65 100644 --- a/quiche/balsa/balsa_frame.h +++ b/quiche/balsa/balsa_frame.h
@@ -63,7 +63,6 @@ headers_(nullptr), start_of_trailer_line_(0), trailer_length_(0), - trailer_(nullptr), invalid_chars_level_(InvalidCharsLevel::kOff), use_interim_headers_callback_(false) {} @@ -102,39 +101,11 @@ } } - // The method set_balsa_trailer() clears `trailer` and attaches it to the - // framer. This is a required step before the framer will process any input - // message data. To detach the trailer object from the framer, use - // set_balsa_trailer(nullptr). - // TODO(b/134507471): Remove this method in favor of `EnableTrailers()`. - void set_balsa_trailer(BalsaHeaders* trailer) { - if (trailers_ != nullptr) { - QUICHE_LOG(DFATAL) << "set_balsa_trailer() called with trailers_!"; - return; - } - if (trailer != nullptr && is_request()) { - QUICHE_CODE_COUNT(balsa_trailer_in_request); - } - - if (trailer_ != trailer) { - trailer_ = trailer; - } - if (trailer_ != nullptr) { - // Clear the trailer if it is non-null, even if the new trailer is - // the same as the old. - trailer_->Clear(); - } - } - // Enables the framer to process trailers and deliver them in - // `BalsaVisitorInterface::OnTrailers()`. Mutually exclusive with - // `set_balsa_trailer()`. If neither method is called, minimal - // trailers parsing will be performed (just enough to advance past trailers). - // TODO(b/134507471): Update comment with removal of `set_balsa_trailer()`. + // `BalsaVisitorInterface::OnTrailers()`. If this method is not called and + // trailers are received, only minimal trailers parsing will be performed + // (just enough to advance past trailers). void EnableTrailers() { - if (trailer_ != nullptr) { - QUICHE_LOG(DFATAL) << "EnableTrailers() called with trailer_!"; - } if (is_request()) { QUICHE_CODE_COUNT(balsa_trailer_in_request); } @@ -200,9 +171,6 @@ const BalsaHeaders* headers() const { return headers_; } BalsaHeaders* mutable_headers() { return headers_; } - const BalsaHeaders* trailer() const { return trailer_; } - BalsaHeaders* mutable_trailer() { return trailer_; } - size_t BytesSafeToSplice() const; void BytesSpliced(size_t bytes_spliced); @@ -304,9 +272,6 @@ void HandleHeadersTooLongError(); - // TODO(b/134507471): Remove. - BalsaHeaders* GetTrailers() const; - bool last_char_was_slash_r_; bool saw_non_newline_char_; bool start_was_space_; @@ -337,11 +302,8 @@ size_t start_of_trailer_line_; size_t trailer_length_; - // At most one of these members is populated. Neither is reset to nullptr in - // Reset(). - // TODO(b/134507471): Remove `trailer_` and update comment. + // Cleared but not reset to nullptr in Reset(). std::unique_ptr<BalsaHeaders> trailers_; - BalsaHeaders* trailer_; InvalidCharsLevel invalid_chars_level_; // This is not reset in Reset().
diff --git a/quiche/balsa/balsa_frame_test.cc b/quiche/balsa/balsa_frame_test.cc index 36a8bf8..0196044 100644 --- a/quiche/balsa/balsa_frame_test.cc +++ b/quiche/balsa/balsa_frame_test.cc
@@ -588,6 +588,7 @@ balsa_frame_.set_balsa_headers(&headers_); balsa_frame_.set_balsa_visitor(&visitor_mock_); balsa_frame_.set_is_request(true); + balsa_frame_.EnableTrailers(); } void VerifyFirstLineParsing(const std::string& firstline, @@ -601,30 +602,6 @@ NiceMock<BalsaVisitorMock> visitor_mock_; }; -// TODO(b/134507471): Remove this test class and merge with `HTTPBalsaFrameTest` -// with removal of `BalsaFrame::set_balsa_trailer()`. -class HTTPBalsaFrameWithTrailersTest - : public HTTPBalsaFrameTest, - public testing::WithParamInterface<bool> { - protected: - void SetUp() override { - HTTPBalsaFrameTest::SetUp(); - - if (GetParam()) { - balsa_frame_.EnableTrailers(); - } else { - balsa_frame_.set_balsa_trailer(&trailer_); - } - } - - // TODO(b/134507471): Remove with removal of - // `BalsaFrame::set_balsa_trailer()`. - BalsaHeaders trailer_; -}; - -INSTANTIATE_TEST_SUITE_P(WithAndWithoutNewTrailersHandling, - HTTPBalsaFrameWithTrailersTest, testing::Bool()); - // Test correct return value for HeaderFramingFound. TEST_F(HTTPBalsaFrameTest, TestHeaderFramingFound) { // Pattern \r\n\r\n should match kValidTerm1. @@ -1242,8 +1219,7 @@ balsa_frame_.ProcessInput(message.data(), message.size())); } -TEST_P(HTTPBalsaFrameWithTrailersTest, - NothingBadHappensWhenNoVisitorIsAssigned) { +TEST_F(HTTPBalsaFrameTest, NothingBadHappensWhenNoVisitorIsAssigned) { std::string headers = "GET / HTTP/1.1\r\n" "Connection: close\r\n" @@ -1268,17 +1244,9 @@ balsa_frame_.ProcessInput(trailer.data(), trailer.size())); EXPECT_TRUE(balsa_frame_.MessageFullyRead()); EXPECT_EQ(BalsaFrameEnums::BALSA_NO_ERROR, balsa_frame_.ErrorCode()); - if (!GetParam()) { - // With the new trailers handling, the trailers are delivered only via a - // visitor callback. - const absl::string_view crass = trailer_.GetHeader("crass"); - EXPECT_EQ("monkeys", crass); - const absl::string_view funky = trailer_.GetHeader("funky"); - EXPECT_EQ("monkeys", funky); - } } -TEST_P(HTTPBalsaFrameWithTrailersTest, RequestWithTrailers) { +TEST_F(HTTPBalsaFrameTest, RequestWithTrailers) { std::string headers = "GET / HTTP/1.1\r\n" "Connection: close\r\n" @@ -1319,19 +1287,9 @@ EXPECT_TRUE(balsa_frame_.MessageFullyRead()); EXPECT_EQ(BalsaFrameEnums::BALSA_NO_ERROR, balsa_frame_.ErrorCode()); - - if (!GetParam()) { - // With the new trailers handling, the trailers are delivered only via a - // visitor callback, which was checked above. - const absl::string_view crass = trailer_.GetHeader("crass"); - EXPECT_EQ("monkeys", crass); - const absl::string_view funky = trailer_.GetHeader("funky"); - EXPECT_EQ("monkeys", funky); - } } -TEST_P(HTTPBalsaFrameWithTrailersTest, - NothingBadHappensWhenNoVisitorIsAssignedInResponse) { +TEST_F(HTTPBalsaFrameTest, NothingBadHappensWhenNoVisitorIsAssignedInResponse) { std::string headers = "HTTP/1.1 502 Bad Gateway\r\n" "Connection: close\r\n" @@ -1357,14 +1315,6 @@ balsa_frame_.ProcessInput(trailer.data(), trailer.size())); EXPECT_TRUE(balsa_frame_.MessageFullyRead()); EXPECT_EQ(BalsaFrameEnums::BALSA_NO_ERROR, balsa_frame_.ErrorCode()); - if (!GetParam()) { - // With the new trailers handling, the trailers are delivered only via a - // visitor callback. - const absl::string_view crass = trailer_.GetHeader("crass"); - EXPECT_EQ("monkeys", crass); - const absl::string_view funky = trailer_.GetHeader("funky"); - EXPECT_EQ("monkeys", funky); - } } TEST_F(HTTPBalsaFrameTest, TransferEncodingIdentityIsIgnored) { @@ -1414,7 +1364,7 @@ EXPECT_EQ(BalsaFrameEnums::BALSA_NO_ERROR, balsa_frame_.ErrorCode()); } -TEST_P(HTTPBalsaFrameWithTrailersTest, +TEST_F(HTTPBalsaFrameTest, NothingBadHappensWhenAVisitorIsChangedToNULLInMidParsingInTrailer) { std::string headers = "HTTP/1.1 503 Server Not Available\r\n" @@ -1442,14 +1392,6 @@ balsa_frame_.ProcessInput(trailer.data(), trailer.size())); EXPECT_TRUE(balsa_frame_.MessageFullyRead()); EXPECT_EQ(BalsaFrameEnums::BALSA_NO_ERROR, balsa_frame_.ErrorCode()); - if (!GetParam()) { - // With the new trailers handling, the trailers are delivered only via a - // visitor callback. - const absl::string_view crass = trailer_.GetHeader("crass"); - EXPECT_EQ("monkeys", crass); - const absl::string_view funky = trailer_.GetHeader("funky"); - EXPECT_EQ("monkeys", funky); - } } TEST_F(HTTPBalsaFrameTest, @@ -1761,7 +1703,7 @@ EXPECT_EQ(message_body_data, body_data); } -TEST_P(HTTPBalsaFrameWithTrailersTest, +TEST_F(HTTPBalsaFrameTest, VisitorInvokedProperlyForRequestWithTransferEncodingAndTrailers) { std::string message_headers = "DELETE /search?q=fo \t HTTP/1.1 \t \r\n" @@ -2241,7 +2183,7 @@ EXPECT_EQ(message_body_data, body_data); } -TEST_P(HTTPBalsaFrameWithTrailersTest, +TEST_F(HTTPBalsaFrameTest, VisitorInvokedProperlyForResponseWithTransferEncodingAndTrailers) { std::string message_headers = "HTTP/1.1 \t 200 Ok all is well\r\n" @@ -2308,17 +2250,10 @@ EXPECT_EQ(message_body, body_input); EXPECT_EQ(message_body_data, body_data); - - if (!GetParam()) { - // With the new trailers handling, the trailers are delivered only via a - // visitor callback, which was checked above. - const absl::string_view a_trailer_key = trailer_.GetHeader("a_trailer_key"); - EXPECT_EQ("and a trailer value", a_trailer_key); - } } -TEST_P( - HTTPBalsaFrameWithTrailersTest, +TEST_F( + HTTPBalsaFrameTest, VisitorInvokedProperlyForResponseWithTransferEncodingAndTrailersBytePer) { std::string message_headers = "HTTP/1.1 \t 200 Ok all is well\r\n" @@ -2387,13 +2322,6 @@ EXPECT_EQ(message_body, body_input); EXPECT_EQ(message_body_data, body_data); EXPECT_EQ(trailer_data, trailer_input); - - if (!GetParam()) { - // With the new trailers handling, the trailers are delivered only via a - // visitor callback, which was checked above. - const absl::string_view a_trailer_key = trailer_.GetHeader("a_trailer_key"); - EXPECT_EQ("and a trailer value", a_trailer_key); - } } TEST(HTTPBalsaFrame, @@ -2488,104 +2416,6 @@ } } -// TODO(b/134507471): Remove this test. -TEST( - HTTPBalsaFrame, - VisitorInvokedProperlyForResponseWithTransferEncodingAndTrailersRandomOld) { - TestSeed seed; - seed.Initialize(GetQuicheCommandLineFlag(FLAGS_randseed)); - RandomEngine rng; - rng.seed(seed.GetSeed()); - for (int i = 0; i < 1000; ++i) { - std::string message_headers = - "HTTP/1.1 \t 200 Ok all is well\r\n" - "trAnsfer-eNcoding: chunked\r\n" - "\r\n"; - std::string message_body = - "A chunkjed extension \r\n" - "01234567890 more crud including numbers 123123\r\n" - "3f\n" - "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\n" - "0 last one\r\n"; - std::string trailer_data = - "a_trailer_key: and a trailer value\r\n" - "\r\n"; - std::string message_body_data = - "0123456789" - "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"; - - std::string message = - (std::string(message_headers) + std::string(message_body) + - std::string(trailer_data)); - - FakeHeaders fake_headers; - fake_headers.AddKeyValue("trAnsfer-eNcoding", "chunked"); - FakeHeaders fake_headers_in_trailer; - fake_headers_in_trailer.AddKeyValue("a_trailer_key", "and a trailer value"); - - StrictMock<BalsaVisitorMock> visitor_mock; - - BalsaHeaders headers; - BalsaHeaders trailer; - BalsaFrame balsa_frame; - balsa_frame.set_is_request(false); - balsa_frame.set_balsa_headers(&headers); - balsa_frame.set_balsa_trailer(&trailer); - balsa_frame.set_balsa_visitor(&visitor_mock); - - { - InSequence s1; - EXPECT_CALL(visitor_mock, OnResponseFirstLineInput( - "HTTP/1.1 \t 200 Ok all is well", - "HTTP/1.1", "200", "Ok all is well")); - EXPECT_CALL(visitor_mock, ProcessHeaders(fake_headers)); - EXPECT_CALL(visitor_mock, HeaderDone()); - EXPECT_CALL(visitor_mock, ProcessTrailers(fake_headers_in_trailer)); - EXPECT_CALL(visitor_mock, MessageDone()); - } - EXPECT_CALL(visitor_mock, OnHeaderInput(message_headers)); - std::string body_input; - EXPECT_CALL(visitor_mock, OnRawBodyInput(_)) - .WillRepeatedly([&body_input](absl::string_view input) { - absl::StrAppend(&body_input, input); - }); - std::string body_data; - EXPECT_CALL(visitor_mock, OnBodyChunkInput(_)) - .WillRepeatedly([&body_data](absl::string_view input) { - absl::StrAppend(&body_data, input); - }); - std::string trailer_input; - EXPECT_CALL(visitor_mock, OnTrailerInput(_)) - .WillRepeatedly([&trailer_input](absl::string_view input) { - absl::StrAppend(&trailer_input, input); - }); - EXPECT_CALL(visitor_mock, OnChunkLength(_)).Times(AtLeast(1)); - EXPECT_CALL(visitor_mock, OnChunkExtensionInput(_)).Times(AtLeast(1)); - - size_t count = 0; - size_t total_processed = 0; - for (size_t j = 0; j < message.size();) { - auto dist = std::uniform_int_distribution<>(0, message.size() - j + 1); - count = dist(rng); - size_t processed = balsa_frame.ProcessInput(message.data() + j, count); - ASSERT_GE(count, processed); - total_processed += processed; - j += processed; - } - EXPECT_EQ(message.size(), total_processed); - EXPECT_TRUE(balsa_frame.MessageFullyRead()); - EXPECT_FALSE(balsa_frame.Error()); - EXPECT_EQ(BalsaFrameEnums::BALSA_NO_ERROR, balsa_frame.ErrorCode()); - - EXPECT_EQ(message_body, body_input); - EXPECT_EQ(message_body_data, body_data); - EXPECT_EQ(trailer_data, trailer_input); - - const absl::string_view a_trailer_key = trailer.GetHeader("a_trailer_key"); - EXPECT_EQ("and a trailer value", a_trailer_key); - } -} - TEST_F(HTTPBalsaFrameTest, AppropriateActionTakenWhenHeadersTooLongWithTooMuchInput) { const absl::string_view message = @@ -2668,16 +2498,10 @@ Mock::VerifyAndClearExpectations(&visitor_mock_); } - // TODO(b/134507471): Remove `use_new_trailers_handling`. void TestInvalidTrailerFormat(const std::string& trailer, - bool invalid_name_char, - bool use_new_trailers_handling) { + bool invalid_name_char) { balsa_frame_.set_is_request(false); - if (use_new_trailers_handling) { - balsa_frame_.EnableTrailers(); - } else { - balsa_frame_.set_balsa_trailer(&trailer_); - } + balsa_frame_.EnableTrailers(); std::string headers = "HTTP/1.0 200 ok\r\n" @@ -2725,8 +2549,6 @@ } BalsaHeaders headers_; - // TODO(b/134507471): Remove. - BalsaHeaders trailer_; BalsaFrame balsa_frame_; StrictMock<BalsaVisitorMock> visitor_mock_; }; @@ -2804,8 +2626,7 @@ std::string trailer = ":monkeys\n" "\r\n"; - TestInvalidTrailerFormat(trailer, /*invalid_name_char=*/false, - /*use_new_trailers_handling=*/true); + TestInvalidTrailerFormat(trailer, /*invalid_name_char=*/false); balsa_frame_.Reset(); @@ -2813,8 +2634,7 @@ " \r\n" "test: test\r\n" "\r\n"; - TestInvalidTrailerFormat(trailer2, /*invalid_name_char=*/true, - /*use_new_trailers_handling=*/true); + TestInvalidTrailerFormat(trailer2, /*invalid_name_char=*/true); balsa_frame_.Reset(); @@ -2822,34 +2642,7 @@ "a: b\r\n" ": test\r\n" "\r\n"; - TestInvalidTrailerFormat(trailer3, /*invalid_name_char=*/false, - /*use_new_trailers_handling=*/true); -} - -TEST_F(BalsaFrameParsingTest, InvalidTrailerFormatOld) { - std::string trailer = - ":monkeys\n" - "\r\n"; - TestInvalidTrailerFormat(trailer, /*invalid_name_char=*/false, - /*use_new_trailers_handling=*/false); - - balsa_frame_.Reset(); - - std::string trailer2 = - " \r\n" - "test: test\r\n" - "\r\n"; - TestInvalidTrailerFormat(trailer2, /*invalid_name_char=*/true, - /*use_new_trailers_handling=*/false); - - balsa_frame_.Reset(); - - std::string trailer3 = - "a: b\r\n" - ": test\r\n" - "\r\n"; - TestInvalidTrailerFormat(trailer3, /*invalid_name_char=*/false, - /*use_new_trailers_handling=*/false); + TestInvalidTrailerFormat(trailer3, /*invalid_name_char=*/false); } TEST_F(HTTPBalsaFrameTest, @@ -3508,7 +3301,7 @@ } // Missing colon is a warning, not an error. -TEST_P(HTTPBalsaFrameWithTrailersTest, TrailerMissingColon) { +TEST_F(HTTPBalsaFrameTest, TrailerMissingColon) { std::string headers = "HTTP/1.0 302 Redirect\r\n" "transfer-encoding: chunked\r\n" @@ -3539,21 +3332,13 @@ EXPECT_TRUE(balsa_frame_.MessageFullyRead()); EXPECT_FALSE(balsa_frame_.Error()); EXPECT_EQ(BalsaFrameEnums::TRAILER_MISSING_COLON, balsa_frame_.ErrorCode()); - if (!GetParam()) { - // With the new trailers handling, the trailers are delivered only via a - // visitor callback, which was checked above. - EXPECT_FALSE(trailer_.HasHeader("crass")); - EXPECT_TRUE(trailer_.HasHeader("crass_monkeys")); - const absl::string_view crass_monkeys = trailer_.GetHeader("crass_monkeys"); - EXPECT_TRUE(crass_monkeys.empty()); - } } // This tests multiple headers in trailer. We currently do not and have no plan // to support Trailer field in headers to limit valid field-name in trailer. // Test that we aren't confused by the non-alphanumeric characters in the // trailer, especially ':'. -TEST_P(HTTPBalsaFrameWithTrailersTest, MultipleHeadersInTrailer) { +TEST_F(HTTPBalsaFrameTest, MultipleHeadersInTrailer) { std::string headers = "HTTP/1.1 200 OK\r\n" "transfer-encoding: chunked\r\n" @@ -3635,19 +3420,10 @@ EXPECT_EQ(BalsaFrameEnums::BALSA_NO_ERROR, balsa_frame_.ErrorCode()); EXPECT_EQ(chunks, body_input); - - if (!GetParam()) { - // With the new trailers handling, the trailers are delivered only via a - // visitor callback, which was checked above. - for (iter = trailer.begin(); iter != trailer.end(); ++iter) { - const absl::string_view value = trailer_.GetHeader(iter->first); - EXPECT_EQ(iter->second, value); - } - } } // Test if trailer is not set (the common case), everything will be fine. -TEST_P(HTTPBalsaFrameWithTrailersTest, NothingBadHappensWithNULLTrailer) { +TEST_F(HTTPBalsaFrameTest, NothingBadHappensWithNULLTrailer) { std::string headers = "HTTP/1.1 200 OK\r\n" "transfer-encoding: chunked\r\n" @@ -3667,9 +3443,6 @@ balsa_frame.set_balsa_headers(&headers_); balsa_frame.set_is_request(false); balsa_frame.set_balsa_visitor(nullptr); - if (!GetParam()) { - balsa_frame.set_balsa_trailer(nullptr); - } ASSERT_EQ(headers.size(), balsa_frame.ProcessInput(headers.data(), headers.size())); @@ -3683,7 +3456,7 @@ } // Test Reset() correctly resets trailer related states. -TEST_P(HTTPBalsaFrameWithTrailersTest, FrameAndResetAndFrameAgain) { +TEST_F(HTTPBalsaFrameTest, FrameAndResetAndFrameAgain) { std::string headers = "HTTP/1.1 200 OK\r\n" "transfer-encoding: chunked\r\n" @@ -3714,13 +3487,6 @@ EXPECT_FALSE(balsa_frame_.Error()); EXPECT_EQ(BalsaFrameEnums::BALSA_NO_ERROR, balsa_frame_.ErrorCode()); - if (!GetParam()) { - // With the new trailers handling, the trailers are delivered only via a - // visitor callback, which was checked above. - absl::string_view value = trailer_.GetHeader("k"); - EXPECT_EQ("v", value); - } - balsa_frame_.Reset(); headers = @@ -3750,15 +3516,6 @@ EXPECT_TRUE(balsa_frame_.MessageFullyRead()); EXPECT_FALSE(balsa_frame_.Error()); EXPECT_EQ(BalsaFrameEnums::BALSA_NO_ERROR, balsa_frame_.ErrorCode()); - - if (!GetParam()) { - // With the new trailers handling, the trailers are delivered only via a - // visitor callback, which was checked above. - absl::string_view value = trailer_.GetHeader("k"); - EXPECT_TRUE(value.empty()); - value = trailer_.GetHeader("nk"); - EXPECT_EQ("nv", value); - } } TEST_F(HTTPBalsaFrameTest, TrackInvalidChars) { @@ -3952,7 +3709,7 @@ // Test gibberish in headers and trailer. GFE does not crash but garbage in // garbage out. -TEST_P(HTTPBalsaFrameWithTrailersTest, GibberishInHeadersAndTrailer) { +TEST_F(HTTPBalsaFrameTest, GibberishInHeadersAndTrailer) { // Use static_cast<char> for values exceeding SCHAR_MAX to make sure this // compiles on platforms where char is signed. const char kGibberish1[] = {static_cast<char>(138), static_cast<char>(175), @@ -4006,22 +3763,11 @@ EXPECT_EQ(kGibberish2, field_value); field_value = headers_.GetHeader("foo"); EXPECT_EQ("bar : eeep : baz", field_value); - - if (!GetParam()) { - // With the new trailers handling, the trailers are delivered only via a - // visitor callback, which was checked above. - field_value = trailer_.GetHeader("k"); - EXPECT_EQ("v", field_value); - field_value = trailer_.GetHeader(kGibberish1); - EXPECT_EQ(kGibberish2, field_value); - field_value = trailer_.GetHeader("foo"); - EXPECT_EQ("bar : eeep : baz", field_value); - } } // Note we reuse the header length limit because trailer is just multiple // headers. -TEST_P(HTTPBalsaFrameWithTrailersTest, TrailerTooLong) { +TEST_F(HTTPBalsaFrameTest, TrailerTooLong) { std::string headers = "HTTP/1.0 200 ok\r\n" "transfer-encoding: chunked\r\n" @@ -4055,44 +3801,6 @@ EXPECT_EQ(BalsaFrameEnums::TRAILER_TOO_LONG, balsa_frame_.ErrorCode()); } -// If the `trailer_` object in the framer is set to `nullptr`, -// ProcessTrailers() will not be called. -TEST_P(HTTPBalsaFrameWithTrailersTest, - NoProcessTrailersCallWhenFramerHasNullTrailerObject) { - if (GetParam()) { - // EnableTrailers() cannot be undone. NothingBadHappensWithNULLTrailer - // covers The case where EnableTrailers() is never called. - return; - } - - std::string headers = - "HTTP/1.0 200 ok\r\n" - "transfer-encoding: chunked\r\n" - "\r\n"; - - std::string chunks = - "3\r\n" - "123\r\n" - "0\r\n"; - std::string trailer = - "trailer_key : trailer_value\n" - "\r\n"; - - balsa_frame_.set_is_request(false); - balsa_frame_.set_balsa_trailer(nullptr); - - EXPECT_CALL(visitor_mock_, ProcessTrailers(_)).Times(0); - ASSERT_EQ(headers.size(), - balsa_frame_.ProcessInput(headers.data(), headers.size())); - ASSERT_EQ(chunks.size(), - balsa_frame_.ProcessInput(chunks.data(), chunks.size())); - EXPECT_EQ(trailer.size(), - balsa_frame_.ProcessInput(trailer.data(), trailer.size())); - EXPECT_TRUE(balsa_frame_.MessageFullyRead()); - EXPECT_FALSE(balsa_frame_.Error()); - EXPECT_EQ(BalsaFrameEnums::BALSA_NO_ERROR, balsa_frame_.ErrorCode()); -} - TEST_F(HTTPBalsaFrameTest, Parse100ContinueNoContinueHeadersNoCallback) { std::string continue_headers = "HTTP/1.1 100 Continue\r\n"