diff --git a/test/extensions/filters/http/cache/simple_http_cache/simple_http_cache_test.cc b/test/extensions/filters/http/cache/simple_http_cache/simple_http_cache_test.cc index f2eb5ac181af0..31b981ec50c3b 100644 --- a/test/extensions/filters/http/cache/simple_http_cache/simple_http_cache_test.cc +++ b/test/extensions/filters/http/cache/simple_http_cache/simple_http_cache_test.cc @@ -343,17 +343,25 @@ TEST_F(SimpleHttpCacheTest, UpdateHeadersAndMetadata) { const std::string time_value_1 = formatter_.fromTime(time_source_.systemTime()); Http::TestResponseHeaderMapImpl response_headers{{"date", time_value_1}, {"cache-control", "public,max-age=3600"}}; + Http::TestResponseHeaderMapImpl response_headers_with_age(response_headers); + response_headers_with_age.setReferenceKey(Http::LowerCaseString("age"), "0"); + insert(request_path_1, response_headers, "body"); - EXPECT_TRUE(expectLookupSuccessWithHeaders(lookup(request_path_1).get(), response_headers)); + EXPECT_TRUE( + expectLookupSuccessWithHeaders(lookup(request_path_1).get(), response_headers_with_age)); // Update the date field in the headers time_source_.advanceTimeWait(Seconds(3601)); const SystemTime time_2 = time_source_.systemTime(); const std::string time_value_2 = formatter_.fromTime(time_2); - response_headers = Http::TestResponseHeaderMapImpl{{"date", time_value_2}, - {"cache-control", "public,max-age=3600"}}; - updateHeaders(request_path_1, response_headers, {time_2}); - EXPECT_TRUE(expectLookupSuccessWithHeaders(lookup(request_path_1).get(), response_headers)); + Http::TestResponseHeaderMapImpl response_headers_2 = Http::TestResponseHeaderMapImpl{ + {"date", time_value_2}, {"cache-control", "public,max-age=3600"}}; + Http::TestResponseHeaderMapImpl response_headers_with_age_2(response_headers_2); + response_headers_with_age_2.setReferenceKey(Http::LowerCaseString("age"), "0"); + + updateHeaders(request_path_1, response_headers_2, {time_2}); + EXPECT_TRUE( + expectLookupSuccessWithHeaders(lookup(request_path_1).get(), response_headers_with_age_2)); } TEST_F(SimpleHttpCacheTest, UpdateHeadersForMissingKey) { @@ -373,6 +381,8 @@ TEST_F(SimpleHttpCacheTest, UpdateHeadersDisabledForVaryHeaders) { {"accept", "image/*"}, {"vary", "accept"}}; insert(request_path_1, response_headers_1, "body"); + // An age header is inserted by `makeLookUpResult` + response_headers_1.setReferenceKey(Http::LowerCaseString("age"), "0"); EXPECT_TRUE(expectLookupSuccessWithHeaders(lookup(request_path_1).get(), response_headers_1)); // Update the date field in the headers @@ -384,7 +394,8 @@ TEST_F(SimpleHttpCacheTest, UpdateHeadersDisabledForVaryHeaders) { {"accept", "image/*"}, {"vary", "accept"}}; updateHeaders(request_path_1, response_headers_2, {time_2}); - + response_headers_1.setReferenceKey(Http::LowerCaseString("age"), "3600"); + // the age is still 0 because an entry is considered fresh after validation EXPECT_TRUE(expectLookupSuccessWithHeaders(lookup(request_path_1).get(), response_headers_1)); } @@ -394,6 +405,8 @@ TEST_F(SimpleHttpCacheTest, UpdateHeadersSkipEtagHeader) { Http::TestResponseHeaderMapImpl response_headers_1{ {"date", time_value_1}, {"cache-control", "public,max-age=3600"}, {"etag", "0000-0000"}}; insert(request_path_1, response_headers_1, "body"); + // An age header is inserted by `makeLookUpResult` + response_headers_1.setReferenceKey(Http::LowerCaseString("age"), "0"); EXPECT_TRUE(expectLookupSuccessWithHeaders(lookup(request_path_1).get(), response_headers_1)); // Update the date field in the headers @@ -407,7 +420,7 @@ TEST_F(SimpleHttpCacheTest, UpdateHeadersSkipEtagHeader) { {"date", time_value_2}, {"cache-control", "public,max-age=3600"}, {"etag", "0000-0000"}}; updateHeaders(request_path_1, response_headers_2, {time_2}); - + response_headers_3.setReferenceKey(Http::LowerCaseString("age"), "0"); EXPECT_TRUE(expectLookupSuccessWithHeaders(lookup(request_path_1).get(), response_headers_3)); } @@ -425,6 +438,9 @@ TEST_F(SimpleHttpCacheTest, UpdateHeadersSkipSpecificHeaders) { {"etag", "1111-1111"}, {"link", "; rel=\"preconnect\""}}; insert(request_path_1, origin_response_headers, "body"); + + // An age header is inserted by `makeLookUpResult` + origin_response_headers.setReferenceKey(Http::LowerCaseString("age"), "0"); EXPECT_TRUE( expectLookupSuccessWithHeaders(lookup(request_path_1).get(), origin_response_headers)); time_source_.advanceTimeWait(Seconds(100)); @@ -454,7 +470,6 @@ TEST_F(SimpleHttpCacheTest, UpdateHeadersSkipSpecificHeaders) { {"link", "; rel=\"preconnect\""}}; updateHeaders(request_path_1, incoming_response_headers, {time_2}); - EXPECT_TRUE( expectLookupSuccessWithHeaders(lookup(request_path_1).get(), expected_response_headers)); } @@ -471,6 +486,9 @@ TEST_F(SimpleHttpCacheTest, UpdateHeadersWithMultivalue) { {"link", "; rel=\"preconnect\""}, {"link", "; rel=\"preconnect\""}}; insert(request_path_1, response_headers_1, "body"); + + // An age header is inserted by `makeLookUpResult` + response_headers_1.setReferenceKey(Http::LowerCaseString("age"), "0"); EXPECT_TRUE(expectLookupSuccessWithHeaders(lookup(request_path_1).get(), response_headers_1)); Http::TestResponseHeaderMapImpl response_headers_2{ @@ -481,6 +499,7 @@ TEST_F(SimpleHttpCacheTest, UpdateHeadersWithMultivalue) { updateHeaders(request_path_1, response_headers_2, {time_1}); + response_headers_2.setReferenceKey(Http::LowerCaseString("age"), "0"); EXPECT_TRUE(expectLookupSuccessWithHeaders(lookup(request_path_1).get(), response_headers_2)); } diff --git a/test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc b/test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc index aeffd710edc20..661ebdc6d72b3 100644 --- a/test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc +++ b/test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc @@ -447,11 +447,9 @@ TEST_P(ExtProcIntegrationTest, GetAndSetHeaders) { [](Http::HeaderMap& headers) { headers.addCopy(LowerCaseString("x-remove-this"), "yes"); }); processRequestHeadersMessage(true, [](const HttpHeaders& headers, HeadersResponse& headers_resp) { - Http::TestRequestHeaderMapImpl expected_request_headers{{":scheme", "http"}, - {":method", "GET"}, - {"host", "host"}, - {":path", "/"}, - {"x-remove-this", "yes"}}; + Http::TestRequestHeaderMapImpl expected_request_headers{ + {":scheme", "http"}, {":method", "GET"}, {"host", "host"}, + {":path", "/"}, {"x-remove-this", "yes"}, {"x-forwarded-proto", "http"}}; EXPECT_THAT(headers.headers(), HeaderProtosEqual(expected_request_headers)); auto response_header_mutation = headers_resp.mutable_response()->mutable_header_mutation(); diff --git a/test/extensions/filters/http/ext_proc/utils.cc b/test/extensions/filters/http/ext_proc/utils.cc index c9bb93b4738d5..4bc448909b184 100644 --- a/test/extensions/filters/http/ext_proc/utils.cc +++ b/test/extensions/filters/http/ext_proc/utils.cc @@ -7,13 +7,20 @@ namespace Extensions { namespace HttpFilters { namespace ExternalProcessing { +const absl::flat_hash_set ExtProcTestUtility::ignoredHeaders() { + CONSTRUCT_ON_FIRST_USE(absl::flat_hash_set, "x-request-id", + "x-envoy-upstream-service-time"); +} + bool ExtProcTestUtility::headerProtosEqualIgnoreOrder( const Http::HeaderMap& expected, const envoy::config::core::v3::HeaderMap& actual) { // Comparing header maps is hard because they have duplicates in them. // So we're going to turn them into a HeaderMap and let Envoy do the work. Http::TestRequestHeaderMapImpl actual_headers; for (const auto& header : actual.headers()) { - actual_headers.addCopy(header.key(), header.value()); + if (!ignoredHeaders().contains(header.key())) { + actual_headers.addCopy(header.key(), header.value()); + } } return TestUtility::headerMapEqualIgnoreOrder(expected, actual_headers); } diff --git a/test/extensions/filters/http/ext_proc/utils.h b/test/extensions/filters/http/ext_proc/utils.h index 98274370d9451..ad6b0f4af1044 100644 --- a/test/extensions/filters/http/ext_proc/utils.h +++ b/test/extensions/filters/http/ext_proc/utils.h @@ -16,6 +16,11 @@ class ExtProcTestUtility { // Compare a reference header map to a proto static bool headerProtosEqualIgnoreOrder(const Http::HeaderMap& expected, const envoy::config::core::v3::HeaderMap& actual); + +private: + // These headers are present in the actual, but cannot be specified in the expected + // ignoredHeaders should not be used for equal comparison + static const absl::flat_hash_set ignoredHeaders(); }; MATCHER_P(HeaderProtosEqual, expected, "HTTP header protos match") { diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc index 865d84f3aa05c..0fa076fe35d0c 100644 --- a/test/test_common/utility.cc +++ b/test/test_common/utility.cc @@ -74,7 +74,9 @@ bool TestUtility::headerMapEqualIgnoreOrder(const Http::HeaderMap& lhs, lhs_keys.insert(key); return Http::HeaderMap::Iterate::Continue; }); - rhs.iterate([&lhs, &rhs, &rhs_keys](const Http::HeaderEntry& header) -> Http::HeaderMap::Iterate { + bool values_match = true; + rhs.iterate([&values_match, &lhs, &rhs, + &rhs_keys](const Http::HeaderEntry& header) -> Http::HeaderMap::Iterate { const std::string key{header.key().getStringView()}; // Compare with canonicalized multi-value headers. This ensures we respect order within // a header. @@ -84,12 +86,13 @@ bool TestUtility::headerMapEqualIgnoreOrder(const Http::HeaderMap& lhs, Http::HeaderUtility::getAllOfHeaderAsString(rhs, Http::LowerCaseString(key)); ASSERT(rhs_entry.result()); if (lhs_entry.result() != rhs_entry.result()) { + values_match = false; return Http::HeaderMap::Iterate::Break; } rhs_keys.insert(key); return Http::HeaderMap::Iterate::Continue; }); - return lhs_keys.size() == rhs_keys.size(); + return values_match && lhs_keys.size() == rhs_keys.size(); } bool TestUtility::buffersEqual(const Buffer::Instance& lhs, const Buffer::Instance& rhs) {