Add expect_final_byte_offset argument to CopyAndValidateTrailers(). This will be used in cr/231494986 where trailers sent on the request/response stream should not have the :final-offset pseudo-header. gfe-relnote: n/a. No functional change. PiperOrigin-RevId: 243730777 Change-Id: I1a5c729fb5aab3e3abdb0ce64dad5667faf794bf
diff --git a/epoll_server/platform/api/epoll_address_test_utils.h b/epoll_server/platform/api/epoll_address_test_utils.h index ae87ded..e224afb 100644 --- a/epoll_server/platform/api/epoll_address_test_utils.h +++ b/epoll_server/platform/api/epoll_address_test_utils.h
@@ -5,7 +5,7 @@ #ifndef QUICHE_EPOLL_SERVER_PLATFORM_API_EPOLL_ADDRESS_TEST_UTILS_H_ #define QUICHE_EPOLL_SERVER_PLATFORM_API_EPOLL_ADDRESS_TEST_UTILS_H_ -#include "net/epoll_server/platform/impl/epoll_address_test_utils_impl.h" +#include "net/tools/epoll_server/platform/impl/epoll_address_test_utils_impl.h" namespace epoll_server {
diff --git a/epoll_server/platform/api/epoll_bug.h b/epoll_server/platform/api/epoll_bug.h index d90c427..33a3478 100644 --- a/epoll_server/platform/api/epoll_bug.h +++ b/epoll_server/platform/api/epoll_bug.h
@@ -5,7 +5,7 @@ #ifndef QUICHE_EPOLL_SERVER_PLATFORM_API_EPOLL_BUG_H_ #define QUICHE_EPOLL_SERVER_PLATFORM_API_EPOLL_BUG_H_ -#include "net/epoll_server/platform/impl/epoll_bug_impl.h" +#include "net/tools/epoll_server/platform/impl/epoll_bug_impl.h" #define EPOLL_BUG EPOLL_BUG_IMPL
diff --git a/epoll_server/platform/api/epoll_expect_bug.h b/epoll_server/platform/api/epoll_expect_bug.h index a7795c6..313823c 100644 --- a/epoll_server/platform/api/epoll_expect_bug.h +++ b/epoll_server/platform/api/epoll_expect_bug.h
@@ -5,7 +5,7 @@ #ifndef QUICHE_EPOLL_SERVER_PLATFORM_API_EPOLL_EXPECT_BUG_H_ #define QUICHE_EPOLL_SERVER_PLATFORM_API_EPOLL_EXPECT_BUG_H_ -#include "net/epoll_server/platform/impl/epoll_expect_bug_impl.h" +#include "net/tools/epoll_server/platform/impl/epoll_expect_bug_impl.h" #define EXPECT_EPOLL_BUG EXPECT_EPOLL_BUG_IMPL
diff --git a/epoll_server/platform/api/epoll_logging.h b/epoll_server/platform/api/epoll_logging.h index bcd9b96..3cd2fac 100644 --- a/epoll_server/platform/api/epoll_logging.h +++ b/epoll_server/platform/api/epoll_logging.h
@@ -5,7 +5,7 @@ #ifndef QUICHE_EPOLL_SERVER_PLATFORM_API_EPOLL_LOGGING_H_ #define QUICHE_EPOLL_SERVER_PLATFORM_API_EPOLL_LOGGING_H_ -#include "net/epoll_server/platform/impl/epoll_logging_impl.h" +#include "net/tools/epoll_server/platform/impl/epoll_logging_impl.h" namespace epoll_server {
diff --git a/epoll_server/platform/api/epoll_test.h b/epoll_server/platform/api/epoll_test.h index e5a0524..89214b4 100644 --- a/epoll_server/platform/api/epoll_test.h +++ b/epoll_server/platform/api/epoll_test.h
@@ -5,7 +5,7 @@ #ifndef QUICHE_EPOLL_SERVER_PLATFORM_API_EPOLL_TEST_H_ #define QUICHE_EPOLL_SERVER_PLATFORM_API_EPOLL_TEST_H_ -#include "net/epoll_server/platform/impl/epoll_test_impl.h" +#include "net/tools/epoll_server/platform/impl/epoll_test_impl.h" #define EpollTest EpollTestImpl #endif // QUICHE_EPOLL_SERVER_PLATFORM_API_EPOLL_TEST_H_
diff --git a/epoll_server/platform/api/epoll_thread.h b/epoll_server/platform/api/epoll_thread.h index a41dd28..97c0b08 100644 --- a/epoll_server/platform/api/epoll_thread.h +++ b/epoll_server/platform/api/epoll_thread.h
@@ -7,7 +7,7 @@ #include <string> -#include "net/epoll_server/platform/impl/epoll_thread_impl.h" +#include "net/tools/epoll_server/platform/impl/epoll_thread_impl.h" namespace epoll_server {
diff --git a/epoll_server/platform/api/epoll_time.h b/epoll_server/platform/api/epoll_time.h index 9fefaa7..95f0407 100644 --- a/epoll_server/platform/api/epoll_time.h +++ b/epoll_server/platform/api/epoll_time.h
@@ -5,7 +5,7 @@ #ifndef QUICHE_EPOLL_SERVER_PLATFORM_API_EPOLL_TIME_H_ #define QUICHE_EPOLL_SERVER_PLATFORM_API_EPOLL_TIME_H_ -#include "net/epoll_server/platform/impl/epoll_time_impl.h" +#include "net/tools/epoll_server/platform/impl/epoll_time_impl.h" namespace epoll_server {
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc index fd77668..674b076 100644 --- a/quic/core/http/quic_spdy_stream.cc +++ b/quic/core/http/quic_spdy_stream.cc
@@ -449,7 +449,9 @@ } size_t final_byte_offset = 0; - if (!SpdyUtils::CopyAndValidateTrailers(header_list, &final_byte_offset, + if (!SpdyUtils::CopyAndValidateTrailers(header_list, + /* expect_final_byte_offset = */ true, + &final_byte_offset, &received_trailers_)) { QUIC_DLOG(ERROR) << "Trailers for stream " << id() << " are malformed."; session()->connection()->CloseConnection(
diff --git a/quic/core/http/spdy_utils.cc b/quic/core/http/spdy_utils.cc index 2d94fd4..721ae33 100644 --- a/quic/core/http/spdy_utils.cc +++ b/quic/core/http/spdy_utils.cc
@@ -86,6 +86,7 @@ } bool SpdyUtils::CopyAndValidateTrailers(const QuicHeaderList& header_list, + bool expect_final_byte_offset, size_t* final_byte_offset, SpdyHeaderBlock* trailers) { bool found_final_byte_offset = false; @@ -94,7 +95,8 @@ // Pull out the final offset pseudo header which indicates the number of // response body bytes expected. - if (!found_final_byte_offset && name == kFinalOffsetHeaderKey && + if (expect_final_byte_offset && !found_final_byte_offset && + name == kFinalOffsetHeaderKey && QuicTextUtils::StringToSizeT(p.second, final_byte_offset)) { found_final_byte_offset = true; continue; @@ -116,7 +118,7 @@ trailers->AppendValueOrAddHeader(name, p.second); } - if (!found_final_byte_offset) { + if (expect_final_byte_offset && !found_final_byte_offset) { QUIC_DLOG(ERROR) << "Required key '" << kFinalOffsetHeaderKey << "' not present"; return false;
diff --git a/quic/core/http/spdy_utils.h b/quic/core/http/spdy_utils.h index 444dd7f..dc3fabc 100644 --- a/quic/core/http/spdy_utils.h +++ b/quic/core/http/spdy_utils.h
@@ -32,7 +32,17 @@ spdy::SpdyHeaderBlock* headers); // Copies a list of headers to a SpdyHeaderBlock. + // If |expect_final_byte_offset| is true, requires exactly one header field + // with key kFinalOffsetHeaderKey and an integer value. + // If |expect_final_byte_offset| is false, no kFinalOffsetHeaderKey may be + // present. + // Returns true if parsing is successful. Returns false if the presence of + // kFinalOffsetHeaderKey does not match the value of + // |expect_final_byte_offset|, the kFinalOffsetHeaderKey value cannot be + // parsed, any other pseudo-header is present, an empty header key is present, + // or a header key contains an uppercase character. static bool CopyAndValidateTrailers(const QuicHeaderList& header_list, + bool expect_final_byte_offset, size_t* final_byte_offset, spdy::SpdyHeaderBlock* trailers);
diff --git a/quic/core/http/spdy_utils_test.cc b/quic/core/http/spdy_utils_test.cc index b7da0fd..d5c96fb 100644 --- a/quic/core/http/spdy_utils_test.cc +++ b/quic/core/http/spdy_utils_test.cc
@@ -17,6 +17,10 @@ namespace quic { namespace test { +namespace { + +const bool kExpectFinalByteOffset = true; +const bool kDoNotExpectFinalByteOffset = false; static std::unique_ptr<QuicHeaderList> FromList( const QuicHeaderList::ListType& src) { @@ -29,6 +33,8 @@ return headers; } +} // anonymous namespace + using CopyAndValidateHeaders = QuicTest; TEST_F(CopyAndValidateHeaders, NormalUsage) { @@ -202,29 +208,62 @@ auto trailers = FromList({{kFinalOffsetHeaderKey, "1234"}}); size_t final_byte_offset = 0; SpdyHeaderBlock block; - EXPECT_TRUE(SpdyUtils::CopyAndValidateTrailers(*trailers, &final_byte_offset, - &block)); + EXPECT_TRUE(SpdyUtils::CopyAndValidateTrailers( + *trailers, kExpectFinalByteOffset, &final_byte_offset, &block)); EXPECT_EQ(1234u, final_byte_offset); } -TEST_F(CopyAndValidateTrailers, EmptyTrailerList) { - // An empty trailer list will fail as required key kFinalOffsetHeaderKey is +TEST_F(CopyAndValidateTrailers, EmptyTrailerListWithFinalByteOffsetExpected) { + // An empty trailer list will fail as expected key kFinalOffsetHeaderKey is // not present. QuicHeaderList trailers; size_t final_byte_offset = 0; SpdyHeaderBlock block; - EXPECT_FALSE( - SpdyUtils::CopyAndValidateTrailers(trailers, &final_byte_offset, &block)); + EXPECT_FALSE(SpdyUtils::CopyAndValidateTrailers( + trailers, kExpectFinalByteOffset, &final_byte_offset, &block)); } -TEST_F(CopyAndValidateTrailers, FinalByteOffsetNotPresent) { - // Validation fails if required kFinalOffsetHeaderKey is not present, even if +TEST_F(CopyAndValidateTrailers, + EmptyTrailerListWithFinalByteOffsetNotExpected) { + // An empty trailer list will pass successfully if kFinalOffsetHeaderKey is + // not expected. + QuicHeaderList trailers; + size_t final_byte_offset = 0; + SpdyHeaderBlock block; + EXPECT_TRUE(SpdyUtils::CopyAndValidateTrailers( + trailers, kDoNotExpectFinalByteOffset, &final_byte_offset, &block)); + EXPECT_TRUE(block.empty()); +} + +TEST_F(CopyAndValidateTrailers, FinalByteOffsetExpectedButNotPresent) { + // Validation fails if expected kFinalOffsetHeaderKey is not present, even if // the rest of the header block is valid. auto trailers = FromList({{"key", "value"}}); size_t final_byte_offset = 0; SpdyHeaderBlock block; - EXPECT_FALSE(SpdyUtils::CopyAndValidateTrailers(*trailers, &final_byte_offset, - &block)); + EXPECT_FALSE(SpdyUtils::CopyAndValidateTrailers( + *trailers, kExpectFinalByteOffset, &final_byte_offset, &block)); +} + +TEST_F(CopyAndValidateTrailers, FinalByteOffsetNotExpectedButPresent) { + // Validation fails if kFinalOffsetHeaderKey is present but should not be, + // even if the rest of the header block is valid. + auto trailers = FromList({{"key", "value"}, {kFinalOffsetHeaderKey, "1234"}}); + size_t final_byte_offset = 0; + SpdyHeaderBlock block; + EXPECT_FALSE(SpdyUtils::CopyAndValidateTrailers( + *trailers, kDoNotExpectFinalByteOffset, &final_byte_offset, &block)); +} + +TEST_F(CopyAndValidateTrailers, FinalByteOffsetNotExpectedAndNotPresent) { + // Validation succeeds if kFinalOffsetHeaderKey is not expected and not + // present. + auto trailers = FromList({{"key", "value"}}); + size_t final_byte_offset = 0; + SpdyHeaderBlock block; + EXPECT_TRUE(SpdyUtils::CopyAndValidateTrailers( + *trailers, kDoNotExpectFinalByteOffset, &final_byte_offset, &block)); + EXPECT_THAT(block, UnorderedElementsAre(Pair("key", "value"))); } TEST_F(CopyAndValidateTrailers, EmptyName) { @@ -233,8 +272,8 @@ auto trailers = FromList({{"", "value"}, {kFinalOffsetHeaderKey, "1234"}}); size_t final_byte_offset = 0; SpdyHeaderBlock block; - EXPECT_FALSE(SpdyUtils::CopyAndValidateTrailers(*trailers, &final_byte_offset, - &block)); + EXPECT_FALSE(SpdyUtils::CopyAndValidateTrailers( + *trailers, kExpectFinalByteOffset, &final_byte_offset, &block)); } TEST_F(CopyAndValidateTrailers, PseudoHeaderInTrailers) { @@ -243,8 +282,8 @@ FromList({{":pseudo_key", "value"}, {kFinalOffsetHeaderKey, "1234"}}); size_t final_byte_offset = 0; SpdyHeaderBlock block; - EXPECT_FALSE(SpdyUtils::CopyAndValidateTrailers(*trailers, &final_byte_offset, - &block)); + EXPECT_FALSE(SpdyUtils::CopyAndValidateTrailers( + *trailers, kExpectFinalByteOffset, &final_byte_offset, &block)); } TEST_F(CopyAndValidateTrailers, DuplicateTrailers) { @@ -262,8 +301,8 @@ {"key", "non_contiguous_duplicate"}}); size_t final_byte_offset = 0; SpdyHeaderBlock block; - EXPECT_TRUE(SpdyUtils::CopyAndValidateTrailers(*trailers, &final_byte_offset, - &block)); + EXPECT_TRUE(SpdyUtils::CopyAndValidateTrailers( + *trailers, kExpectFinalByteOffset, &final_byte_offset, &block)); EXPECT_THAT( block, UnorderedElementsAre( @@ -286,8 +325,8 @@ size_t final_byte_offset = 0; SpdyHeaderBlock block; - EXPECT_TRUE( - SpdyUtils::CopyAndValidateTrailers(*headers, &final_byte_offset, &block)); + EXPECT_TRUE(SpdyUtils::CopyAndValidateTrailers( + *headers, kExpectFinalByteOffset, &final_byte_offset, &block)); EXPECT_THAT( block, UnorderedElementsAre(