From 7f3e8a85ff1dbc3f3aec54dad7b5450aa53a98cd Mon Sep 17 00:00:00 2001 From: Yosry Ahmed Date: Wed, 12 Aug 2020 15:38:11 -0700 Subject: [PATCH] caching: Improved the tests and coverage of the CacheFilter tree (#12544) Signed-off-by: Yosry Ahmed --- .../filters/http/cache/cache_filter.cc | 1 + .../filters/http/cache/cache_headers_utils.cc | 25 - .../filters/http/cache/cache_headers_utils.h | 3 - .../filters/http/cache/cacheability_utils.cc | 6 +- .../filters/http/cache/http_cache.cc | 22 - .../filters/http/cache/http_cache.h | 4 - test/extensions/filters/http/cache/BUILD | 16 +- .../cache/cache_filter_integration_test.cc | 253 +++--- .../filters/http/cache/cache_filter_test.cc | 729 ++++++++++++++++-- .../http/cache/cache_headers_utils_test.cc | 9 +- .../http/cache/cacheability_utils_test.cc | 156 ++-- test/extensions/filters/http/cache/common.h | 134 ++++ .../filters/http/cache/config_test.cc | 1 + .../filters/http/cache/http_cache_test.cc | 342 +++++--- .../http/cache/simple_http_cache/BUILD | 1 + .../simple_http_cache_test.cc | 1 + tools/spelling/spelling_dictionary.txt | 1 + 17 files changed, 1287 insertions(+), 417 deletions(-) create mode 100644 test/extensions/filters/http/cache/common.h diff --git a/source/extensions/filters/http/cache/cache_filter.cc b/source/extensions/filters/http/cache/cache_filter.cc index 18ab3e625c26..007a2a6b6010 100644 --- a/source/extensions/filters/http/cache/cache_filter.cc +++ b/source/extensions/filters/http/cache/cache_filter.cc @@ -262,6 +262,7 @@ void CacheFilter::processSuccessfulValidation(Http::ResponseHeaderMap& response_ } } +// TODO(yosrym93): Write a test that exercises this when SimpleHttpCache implements updateHeaders bool CacheFilter::shouldUpdateCachedEntry(const Http::ResponseHeaderMap& response_headers) const { ASSERT(isResponseNotModified(response_headers), "shouldUpdateCachedEntry must only be called with 304 responses"); diff --git a/source/extensions/filters/http/cache/cache_headers_utils.cc b/source/extensions/filters/http/cache/cache_headers_utils.cc index 27d08bde0088..7d6559104258 100644 --- a/source/extensions/filters/http/cache/cache_headers_utils.cc +++ b/source/extensions/filters/http/cache/cache_headers_utils.cc @@ -106,31 +106,6 @@ ResponseCacheControl::ResponseCacheControl(absl::string_view cache_control_heade } } -std::ostream& operator<<(std::ostream& os, const OptionalDuration& duration) { - return duration.has_value() ? os << duration.value().count() : os << " "; -} - -std::ostream& operator<<(std::ostream& os, const RequestCacheControl& request_cache_control) { - return os << "{" - << "must_validate: " << request_cache_control.must_validate_ << ", " - << "no_store: " << request_cache_control.no_store_ << ", " - << "no_transform: " << request_cache_control.no_transform_ << ", " - << "only_if_cached: " << request_cache_control.only_if_cached_ << ", " - << "max_age: " << request_cache_control.max_age_ << ", " - << "min_fresh: " << request_cache_control.min_fresh_ << ", " - << "max_stale: " << request_cache_control.max_stale_ << "}"; -} - -std::ostream& operator<<(std::ostream& os, const ResponseCacheControl& response_cache_control) { - return os << "{" - << "must_validate: " << response_cache_control.must_validate_ << ", " - << "no_store: " << response_cache_control.no_store_ << ", " - << "no_transform: " << response_cache_control.no_transform_ << ", " - << "no_stale: " << response_cache_control.no_stale_ << ", " - << "public: " << response_cache_control.is_public_ << ", " - << "max_age: " << response_cache_control.max_age_ << "}"; -} - bool operator==(const RequestCacheControl& lhs, const RequestCacheControl& rhs) { return (lhs.must_validate_ == rhs.must_validate_) && (lhs.no_store_ == rhs.no_store_) && (lhs.no_transform_ == rhs.no_transform_) && (lhs.only_if_cached_ == rhs.only_if_cached_) && diff --git a/source/extensions/filters/http/cache/cache_headers_utils.h b/source/extensions/filters/http/cache/cache_headers_utils.h index 8a185d88b40d..79193ce4f21b 100644 --- a/source/extensions/filters/http/cache/cache_headers_utils.h +++ b/source/extensions/filters/http/cache/cache_headers_utils.h @@ -82,9 +82,6 @@ struct ResponseCacheControl { OptionalDuration max_age_; }; -std::ostream& operator<<(std::ostream& os, const OptionalDuration& duration); -std::ostream& operator<<(std::ostream& os, const RequestCacheControl& request_cache_control); -std::ostream& operator<<(std::ostream& os, const ResponseCacheControl& response_cache_control); bool operator==(const RequestCacheControl& lhs, const RequestCacheControl& rhs); bool operator==(const ResponseCacheControl& lhs, const ResponseCacheControl& rhs); diff --git a/source/extensions/filters/http/cache/cacheability_utils.cc b/source/extensions/filters/http/cache/cacheability_utils.cc index 784c37953c9d..029569521c85 100644 --- a/source/extensions/filters/http/cache/cacheability_utils.cc +++ b/source/extensions/filters/http/cache/cacheability_utils.cc @@ -61,9 +61,11 @@ bool CacheabilityUtils::isCacheableResponse(const Http::ResponseHeaderMap& heade ResponseCacheControl response_cache_control(cache_control); // Only cache responses with explicit validation data, either: - // max-age or s-maxage cache-control directives with date header. - // expires header. + // "no-cache" cache-control directive + // "max-age" or "s-maxage" cache-control directives with date header + // expires header const bool has_validation_data = + response_cache_control.must_validate_ || (headers.Date() && response_cache_control.max_age_.has_value()) || headers.get(Http::Headers::get().Expires); diff --git a/source/extensions/filters/http/cache/http_cache.cc b/source/extensions/filters/http/cache/http_cache.cc index 43943444c897..236f20d3d619 100644 --- a/source/extensions/filters/http/cache/http_cache.cc +++ b/source/extensions/filters/http/cache/http_cache.cc @@ -22,28 +22,6 @@ namespace Extensions { namespace HttpFilters { namespace Cache { -std::ostream& operator<<(std::ostream& os, CacheEntryStatus status) { - switch (status) { - case CacheEntryStatus::Ok: - return os << "Ok"; - case CacheEntryStatus::Unusable: - return os << "Unusable"; - case CacheEntryStatus::RequiresValidation: - return os << "RequiresValidation"; - case CacheEntryStatus::FoundNotModified: - return os << "FoundNotModified"; - case CacheEntryStatus::NotSatisfiableRange: - return os << "NotSatisfiableRange"; - case CacheEntryStatus::SatisfiableRange: - return os << "SatisfiableRange"; - } - NOT_REACHED_GCOVR_EXCL_LINE; -} - -std::ostream& operator<<(std::ostream& os, const AdjustedByteRange& range) { - return os << "[" << range.begin() << "," << range.end() << ")"; -} - LookupRequest::LookupRequest(const Http::RequestHeaderMap& request_headers, SystemTime timestamp) : timestamp_(timestamp) { // These ASSERTs check prerequisites. A request without these headers can't be looked up in cache; diff --git a/source/extensions/filters/http/cache/http_cache.h b/source/extensions/filters/http/cache/http_cache.h index 907a8d02be96..9a2736994ea6 100644 --- a/source/extensions/filters/http/cache/http_cache.h +++ b/source/extensions/filters/http/cache/http_cache.h @@ -41,8 +41,6 @@ enum class CacheEntryStatus { SatisfiableRange, }; -std::ostream& operator<<(std::ostream& os, CacheEntryStatus status); - // Byte range from an HTTP request. class RawByteRange { public: @@ -112,8 +110,6 @@ inline bool operator==(const AdjustedByteRange& lhs, const AdjustedByteRange& rh return lhs.begin() == rhs.begin() && lhs.end() == rhs.end(); } -std::ostream& operator<<(std::ostream& os, const AdjustedByteRange& range); - // Adjusts request_range_spec to fit a cached response of size content_length, putting the results // in response_ranges. Returns true if response_ranges is satisfiable (empty is considered // satisfiable, as it denotes the entire body). diff --git a/test/extensions/filters/http/cache/BUILD b/test/extensions/filters/http/cache/BUILD index db5d5ea50fd5..c17f76a3a206 100644 --- a/test/extensions/filters/http/cache/BUILD +++ b/test/extensions/filters/http/cache/BUILD @@ -1,4 +1,4 @@ -load("//bazel:envoy_build_system.bzl", "envoy_package") +load("//bazel:envoy_build_system.bzl", "envoy_cc_test_library", "envoy_package") load( "//test/extensions:extensions_build_system.bzl", "envoy_extension_cc_test", @@ -8,11 +8,22 @@ licenses(["notice"]) # Apache 2 envoy_package() +envoy_cc_test_library( + name = "common", + hdrs = ["common.h"], + deps = [ + "//source/extensions/filters/http/cache:cache_headers_utils_lib", + "//source/extensions/filters/http/cache:http_cache_lib", + "//source/extensions/filters/http/cache/simple_http_cache:simple_http_cache_lib", + ], +) + envoy_extension_cc_test( name = "cache_headers_utils_test", srcs = ["cache_headers_utils_test.cc"], extension_name = "envoy.filters.http.cache", deps = [ + ":common", "//include/envoy/http:header_map_interface", "//source/common/http:header_map_lib", "//source/extensions/filters/http/cache:cache_headers_utils_lib", @@ -25,6 +36,7 @@ envoy_extension_cc_test( srcs = ["http_cache_test.cc"], extension_name = "envoy.filters.http.cache", deps = [ + ":common", "//source/extensions/filters/http/cache:http_cache_lib", "//source/extensions/filters/http/cache/simple_http_cache:simple_http_cache_lib", "//test/mocks/http:http_mocks", @@ -38,6 +50,7 @@ envoy_extension_cc_test( srcs = ["cache_filter_test.cc"], extension_name = "envoy.filters.http.cache", deps = [ + ":common", "//source/extensions/filters/http/cache:cache_filter_lib", "//source/extensions/filters/http/cache/simple_http_cache:simple_http_cache_lib", "//test/mocks/server:factory_context_mocks", @@ -75,7 +88,6 @@ envoy_extension_cc_test( "cache_filter_integration_test.cc", ], extension_name = "envoy.filters.http.cache", - tags = ["fails_on_windows"], deps = [ "//source/extensions/filters/http/cache:config", "//source/extensions/filters/http/cache:http_cache_lib", diff --git a/test/extensions/filters/http/cache/cache_filter_integration_test.cc b/test/extensions/filters/http/cache/cache_filter_integration_test.cc index d4113c78c7a8..fde9cd63fe4c 100644 --- a/test/extensions/filters/http/cache/cache_filter_integration_test.cc +++ b/test/extensions/filters/http/cache/cache_filter_integration_test.cc @@ -5,6 +5,7 @@ namespace Envoy { namespace Extensions { namespace HttpFilters { namespace Cache { +namespace { // TODO(toddmgreer): Expand integration test to include age header values, // expiration, range headers, HEAD requests, trailers, config customizations, @@ -51,45 +52,51 @@ TEST_P(CacheIntegrationTest, MissInsertHit) { {":path", absl::StrCat("/", protocolTestParamsToString({GetParam(), 0}))}, {":scheme", "http"}, {":authority", "MissInsertHit"}}; - Http::TestResponseHeaderMapImpl response_headers = {{":status", "200"}, - {"date", formatter_.now(simTime())}, - {"cache-control", "public,max-age=3600"}, - {"content-length", "42"}}; + + const std::string response_body(42, 'a'); + Http::TestResponseHeaderMapImpl response_headers = { + {":status", "200"}, + {"date", formatter_.now(simTime())}, + {"cache-control", "public,max-age=3600"}, + {"content-length", std::to_string(response_body.size())}}; // Send first request, and get response from upstream. { - IntegrationStreamDecoderPtr request = codec_client_->makeHeaderOnlyRequest(request_headers); + IntegrationStreamDecoderPtr response_decoder = + codec_client_->makeHeaderOnlyRequest(request_headers); waitForNextUpstreamRequest(); upstream_request_->encodeHeaders(response_headers, /*end_stream=*/false); // send 42 'a's - upstream_request_->encodeData(42, true); + upstream_request_->encodeData(response_body, /*end_stream=*/true); // Wait for the response to be read by the codec client. - request->waitForEndStream(); - EXPECT_TRUE(request->complete()); - EXPECT_THAT(request->headers(), IsSupersetOfHeaders(response_headers)); - EXPECT_EQ(request->headers().get(Http::Headers::get().Age), nullptr); - EXPECT_EQ(request->body(), std::string(42, 'a')); - EXPECT_EQ(waitForAccessLog(access_log_name_), - fmt::format("- via_upstream{}", TestEnvironment::newLine)); + response_decoder->waitForEndStream(); + EXPECT_TRUE(response_decoder->complete()); + EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(response_headers)); + EXPECT_EQ(response_decoder->headers().get(Http::Headers::get().Age), nullptr); + EXPECT_EQ(response_decoder->body(), response_body); + EXPECT_THAT(waitForAccessLog(access_log_name_), testing::HasSubstr("- via_upstream")); } // Advance time, to verify the original date header is preserved. simTime().advanceTimeWait(std::chrono::seconds(10)); // Send second request, and get response from cache. - IntegrationStreamDecoderPtr request = codec_client_->makeHeaderOnlyRequest(request_headers); - request->waitForEndStream(); - EXPECT_TRUE(request->complete()); - EXPECT_THAT(request->headers(), IsSupersetOfHeaders(response_headers)); - EXPECT_EQ(request->body(), std::string(42, 'a')); - EXPECT_NE(request->headers().get(Http::Headers::get().Age), nullptr); - // Advance time to force a log flush. - simTime().advanceTimeWait(std::chrono::seconds(1)); - EXPECT_EQ(waitForAccessLog(access_log_name_, 1), - fmt::format("RFCF cache.response_from_cache_filter{}", TestEnvironment::newLine)); + { + IntegrationStreamDecoderPtr response_decoder = + codec_client_->makeHeaderOnlyRequest(request_headers); + response_decoder->waitForEndStream(); + EXPECT_TRUE(response_decoder->complete()); + EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(response_headers)); + EXPECT_EQ(response_decoder->body(), response_body); + EXPECT_NE(response_decoder->headers().get(Http::Headers::get().Age), nullptr); + // Advance time to force a log flush. + simTime().advanceTimeWait(std::chrono::seconds(1)); + EXPECT_THAT(waitForAccessLog(access_log_name_, 1), + testing::HasSubstr("RFCF cache.response_from_cache_filter")); + } } -TEST_P(CacheIntegrationTest, SuccessfulValidation) { +TEST_P(CacheIntegrationTest, ExpiredValidated) { useAccessLog("%RESPONSE_FLAGS% %RESPONSE_CODE_DETAILS%"); // Set system time to cause Envoy's cached formatted time to match time on this thread. simTime().setSystemTime(std::chrono::hours(1)); @@ -100,14 +107,15 @@ TEST_P(CacheIntegrationTest, SuccessfulValidation) { {":method", "GET"}, {":path", absl::StrCat("/", protocolTestParamsToString({GetParam(), 0}))}, {":scheme", "http"}, - {":authority", "SuccessfulValidation"}}; + {":authority", "ExpiredValidated"}}; - const std::string original_response_date = formatter_.now(simTime()); - Http::TestResponseHeaderMapImpl response_headers = {{":status", "200"}, - {"date", original_response_date}, - {"cache-control", "max-age=0"}, - {"content-length", "42"}, - {"etag", "abc123"}}; + const std::string response_body(42, 'a'); + Http::TestResponseHeaderMapImpl response_headers = { + {":status", "200"}, + {"date", formatter_.now(simTime())}, + {"cache-control", "max-age=10"}, // expires after 10 s + {"content-length", std::to_string(response_body.size())}, + {"etag", "abc123"}}; // Send first request, and get response from upstream. { @@ -116,54 +124,57 @@ TEST_P(CacheIntegrationTest, SuccessfulValidation) { waitForNextUpstreamRequest(); upstream_request_->encodeHeaders(response_headers, /*end_stream=*/false); // send 42 'a's - upstream_request_->encodeData(42, true); + upstream_request_->encodeData(response_body, true); // Wait for the response to be read by the codec client. response_decoder->waitForEndStream(); EXPECT_TRUE(response_decoder->complete()); EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(response_headers)); EXPECT_EQ(response_decoder->headers().get(Http::Headers::get().Age), nullptr); - EXPECT_EQ(response_decoder->body(), std::string(42, 'a')); - EXPECT_EQ(waitForAccessLog(access_log_name_), "- via_upstream\n"); + EXPECT_EQ(response_decoder->body(), response_body); + EXPECT_THAT(waitForAccessLog(access_log_name_), testing::HasSubstr("- via_upstream")); } - simTime().advanceTimeWait(std::chrono::seconds(10)); - const std::string not_modified_date = formatter_.now(simTime()); - - // Send second request, the cached response should be validated then served. - IntegrationStreamDecoderPtr response_decoder = - codec_client_->makeHeaderOnlyRequest(request_headers); - waitForNextUpstreamRequest(); - - // Check for injected conditional headers -- no "Last-Modified" header so should fallback to - // "Date". - Http::TestRequestHeaderMapImpl injected_headers = {{"if-none-match", "abc123"}, - {"if-modified-since", original_response_date}}; - EXPECT_THAT(upstream_request_->headers(), IsSupersetOfHeaders(injected_headers)); - - // Create a 304 (not modified) response -> cached response is valid. - Http::TestResponseHeaderMapImpl not_modified_response_headers = {{":status", "304"}, - {"date", not_modified_date}}; - upstream_request_->encodeHeaders(not_modified_response_headers, /*end_stream=*/true); - - // The original response headers should be updated with 304 response headers. - response_headers.setDate(not_modified_date); - - // Wait for the response to be read by the codec client. - response_decoder->waitForEndStream(); - - // Check that the served response is the cached response. - EXPECT_TRUE(response_decoder->complete()); - EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(response_headers)); - EXPECT_EQ(response_decoder->body(), std::string(42, 'a')); - // Check that age header exists as this is a cached response. - EXPECT_NE(response_decoder->headers().get(Http::Headers::get().Age), nullptr); - - // Advance time to force a log flush. - simTime().advanceTimeWait(std::chrono::seconds(1)); - EXPECT_EQ(waitForAccessLog(access_log_name_, 1), "RFCF cache.response_from_cache_filter\n"); + // Advance time for the cached response to be stale (expired) + // Also to make sure response date header gets updated with the 304 date + simTime().advanceTimeWait(std::chrono::seconds(11)); + + // Send second request, the cached response should be validate then served + { + IntegrationStreamDecoderPtr response_decoder = + codec_client_->makeHeaderOnlyRequest(request_headers); + waitForNextUpstreamRequest(); + + // Check for injected precondition headers + const Http::TestRequestHeaderMapImpl injected_headers = {{"if-none-match", "abc123"}}; + EXPECT_THAT(upstream_request_->headers(), IsSupersetOfHeaders(injected_headers)); + + // Create a 304 (not modified) response -> cached response is valid + const std::string not_modified_date = formatter_.now(simTime()); + const Http::TestResponseHeaderMapImpl not_modified_response_headers = { + {":status", "304"}, {"date", not_modified_date}}; + upstream_request_->encodeHeaders(not_modified_response_headers, /*end_stream=*/true); + + // The original response headers should be updated with 304 response headers + response_headers.setDate(not_modified_date); + + // Wait for the response to be read by the codec client. + response_decoder->waitForEndStream(); + + // Check that the served response is the cached response + EXPECT_TRUE(response_decoder->complete()); + EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(response_headers)); + EXPECT_EQ(response_decoder->body(), response_body); + // Check that age header exists as this is a cached response + EXPECT_NE(response_decoder->headers().get(Http::Headers::get().Age), nullptr); + + // Advance time to force a log flush. + simTime().advanceTimeWait(std::chrono::seconds(1)); + EXPECT_THAT(waitForAccessLog(access_log_name_, 1), + testing::HasSubstr("RFCF cache.response_from_cache_filter")); + } } -TEST_P(CacheIntegrationTest, UnsuccessfulValidation) { +TEST_P(CacheIntegrationTest, ExpiredFetchedNewResponse) { useAccessLog("%RESPONSE_FLAGS% %RESPONSE_CODE_DETAILS%"); // Set system time to cause Envoy's cached formatted time to match time on this thread. simTime().setSystemTime(std::chrono::hours(1)); @@ -174,65 +185,73 @@ TEST_P(CacheIntegrationTest, UnsuccessfulValidation) { {":method", "GET"}, {":path", absl::StrCat("/", protocolTestParamsToString({GetParam(), 0}))}, {":scheme", "http"}, - {":authority", "UnsuccessfulValidation"}}; - - Http::TestResponseHeaderMapImpl original_response_headers = {{":status", "200"}, - {"date", formatter_.now(simTime())}, - {"cache-control", "max-age=0"}, - {"content-length", "10"}, - {"etag", "a1"}}; + {":authority", "ExpiredFetchedNewResponse"}}; // Send first request, and get response from upstream. { + const std::string response_body(10, 'a'); + Http::TestResponseHeaderMapImpl response_headers = { + {":status", "200"}, + {"date", formatter_.now(simTime())}, + {"cache-control", "max-age=10"}, // expires after 10 s + {"content-length", std::to_string(response_body.size())}, + {"etag", "a1"}}; + IntegrationStreamDecoderPtr response_decoder = codec_client_->makeHeaderOnlyRequest(request_headers); waitForNextUpstreamRequest(); - upstream_request_->encodeHeaders(original_response_headers, /*end_stream=*/false); + upstream_request_->encodeHeaders(response_headers, /*end_stream=*/false); // send 10 'a's - upstream_request_->encodeData(10, true); + upstream_request_->encodeData(response_body, /*end_stream=*/true); // Wait for the response to be read by the codec client. response_decoder->waitForEndStream(); EXPECT_TRUE(response_decoder->complete()); - EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(original_response_headers)); + EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(response_headers)); EXPECT_EQ(response_decoder->headers().get(Http::Headers::get().Age), nullptr); - EXPECT_EQ(response_decoder->body(), std::string(10, 'a')); - EXPECT_EQ(waitForAccessLog(access_log_name_), "- via_upstream\n"); + EXPECT_EQ(response_decoder->body(), response_body); + EXPECT_THAT(waitForAccessLog(access_log_name_), testing::HasSubstr("- via_upstream")); } - simTime().advanceTimeWait(std::chrono::seconds(10)); - // Any response with status other than 304 should be passed to the client as-is. - Http::TestResponseHeaderMapImpl updated_response_headers = {{":status", "200"}, - {"date", formatter_.now(simTime())}, - {"cache-control", "max-age=0"}, - {"content-length", "20"}, - {"etag", "a2"}}; - - // Send second request, validation of the cached response should be attempted but should fail. - IntegrationStreamDecoderPtr response_decoder = - codec_client_->makeHeaderOnlyRequest(request_headers); - waitForNextUpstreamRequest(); - - // Check for injected precondition headers. - Http::TestRequestHeaderMapImpl injected_headers = {{"if-none-match", "a1"}}; - EXPECT_THAT(upstream_request_->headers(), IsSupersetOfHeaders(injected_headers)); - - // Reply with the updated response -> cached response is invalid. - upstream_request_->encodeHeaders(updated_response_headers, /*end_stream=*/false); - // send 20 'a's - upstream_request_->encodeData(20, true); - - // Wait for the response to be read by the codec client. - response_decoder->waitForEndStream(); - // Check that the served response is the updated response. - EXPECT_TRUE(response_decoder->complete()); - EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(updated_response_headers)); - EXPECT_EQ(response_decoder->body(), std::string(20, 'a')); - // Check that age header does not exist as this is not a cached response. - EXPECT_EQ(response_decoder->headers().get(Http::Headers::get().Age), nullptr); - - // Advance time to force a log flush. - simTime().advanceTimeWait(std::chrono::seconds(1)); - EXPECT_EQ(waitForAccessLog(access_log_name_, 1), "- via_upstream\n"); + // Advance time for the cached response to be stale (expired) + // Also to make sure response date header gets updated with the 304 date + simTime().advanceTimeWait(std::chrono::seconds(11)); + + // Send second request, validation of the cached response should be attempted but should fail + // The new response should be served + { + const std::string response_body(20, 'a'); + Http::TestResponseHeaderMapImpl response_headers = { + {":status", "200"}, + {"date", formatter_.now(simTime())}, + {"content-length", std::to_string(response_body.size())}, + {"etag", "a2"}}; + + IntegrationStreamDecoderPtr response_decoder = + codec_client_->makeHeaderOnlyRequest(request_headers); + waitForNextUpstreamRequest(); + + // Check for injected precondition headers + Http::TestRequestHeaderMapImpl injected_headers = {{"if-none-match", "a1"}}; + EXPECT_THAT(upstream_request_->headers(), IsSupersetOfHeaders(injected_headers)); + + // Reply with the updated response -> cached response is invalid + upstream_request_->encodeHeaders(response_headers, /*end_stream=*/false); + // send 20 'a's + upstream_request_->encodeData(response_body, /*end_stream=*/true); + + // Wait for the response to be read by the codec client. + response_decoder->waitForEndStream(); + // Check that the served response is the updated response + EXPECT_TRUE(response_decoder->complete()); + EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(response_headers)); + EXPECT_EQ(response_decoder->body(), response_body); + // Check that age header does not exist as this is not a cached response + EXPECT_EQ(response_decoder->headers().get(Http::Headers::get().Age), nullptr); + + // Advance time to force a log flush. + simTime().advanceTimeWait(std::chrono::seconds(1)); + EXPECT_THAT(waitForAccessLog(access_log_name_), testing::HasSubstr("- via_upstream")); + } } // Send the same GET request with body and trailers twice, then check that the response @@ -273,6 +292,8 @@ TEST_P(CacheIntegrationTest, GetRequestWithBodyAndTrailers) { EXPECT_EQ(response->body(), std::string(42, 'a')); } } + +} // namespace } // namespace Cache } // namespace HttpFilters } // namespace Extensions diff --git a/test/extensions/filters/http/cache/cache_filter_test.cc b/test/extensions/filters/http/cache/cache_filter_test.cc index 0ab4034cc799..e64998e7c227 100644 --- a/test/extensions/filters/http/cache/cache_filter_test.cc +++ b/test/extensions/filters/http/cache/cache_filter_test.cc @@ -1,6 +1,11 @@ +#include "envoy/http/header_map.h" + +#include "common/http/headers.h" + #include "extensions/filters/http/cache/cache_filter.h" #include "extensions/filters/http/cache/simple_http_cache/simple_http_cache.h" +#include "test/extensions/filters/http/cache/common.h" #include "test/mocks/server/factory_context.h" #include "test/test_common/simulated_time_system.h" #include "test/test_common/utility.h" @@ -13,41 +18,6 @@ namespace HttpFilters { namespace Cache { namespace { -// Wrapper for SimpleHttpCache that delays the onHeaders callback from getHeaders, for verifying -// that CacheFilter works correctly whether the onHeaders call happens immediately, or after -// getHeaders and decodeHeaders return. -class DelayedCache : public SimpleHttpCache { -public: - // HttpCache - LookupContextPtr makeLookupContext(LookupRequest&& request) override { - return std::make_unique( - SimpleHttpCache::makeLookupContext(std::move(request)), delayed_cb_); - } - InsertContextPtr makeInsertContext(LookupContextPtr&& lookup_context) override { - return SimpleHttpCache::makeInsertContext( - std::move(dynamic_cast(*lookup_context).context_)); - } - - std::function delayed_cb_; - -private: - class DelayedLookupContext : public LookupContext { - public: - DelayedLookupContext(LookupContextPtr&& context, std::function& delayed_cb) - : context_(std::move(context)), delayed_cb_(delayed_cb) {} - void getHeaders(LookupHeadersCallback&& cb) override { - delayed_cb_ = [this, cb]() mutable { context_->getHeaders(std::move(cb)); }; - } - void getBody(const AdjustedByteRange& range, LookupBodyCallback&& cb) override { - context_->getBody(range, std::move(cb)); - } - void getTrailers(LookupTrailersCallback&& cb) override { context_->getTrailers(std::move(cb)); } - - LookupContextPtr context_; - std::function& delayed_cb_; - }; -}; - class CacheFilterTest : public ::testing::Test { protected: CacheFilter makeFilter(HttpCache& cache) { @@ -73,17 +43,117 @@ class CacheFilterTest : public ::testing::Test { NiceMock encoder_callbacks_; }; +TEST_F(CacheFilterTest, UncacheableRequest) { + request_headers_.setHost("UncacheableRequest"); + + // POST requests are uncacheable + request_headers_.setMethod(Http::Headers::get().MethodValues.Post); + + for (int i = 0; i < 2; i++) { + // Create filter for request i + CacheFilter filter = makeFilter(simple_cache_); + + // Decode request i header + // Make sure the filter did not encode any headers or data + EXPECT_CALL(decoder_callbacks_, encodeHeaders_).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); + // In the first request was cached the second request would return StopAllIterationAndWatermark + EXPECT_EQ(filter.decodeHeaders(request_headers_, true), Http::FilterHeadersStatus::Continue); + ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); + + // Encode response header + EXPECT_EQ(filter.encodeHeaders(response_headers_, true), Http::FilterHeadersStatus::Continue); + filter.onDestroy(); + } +} + +TEST_F(CacheFilterTest, UncacheableResponse) { + request_headers_.setHost("UncacheableResponse"); + + // Responses with "Cache-Control: no-store" are uncacheable + response_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-store"); + + for (int i = 0; i < 2; i++) { + // Create filter for request i + CacheFilter filter = makeFilter(simple_cache_); + + // Decode request i header + // Make sure the filter did not encode any headers or data + EXPECT_CALL(decoder_callbacks_, encodeHeaders_).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); + // In the first request was cached the second request would return StopAllIterationAndWatermark + EXPECT_EQ(filter.decodeHeaders(request_headers_, true), Http::FilterHeadersStatus::Continue); + ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); + + // Encode response header + EXPECT_EQ(filter.encodeHeaders(response_headers_, true), Http::FilterHeadersStatus::Continue); + filter.onDestroy(); + } +} + +TEST_F(CacheFilterTest, ImmediateMiss) { + for (int request = 1; request <= 2; request++) { + // Each iteration a request is sent to a different host, therefore the second one is a miss + request_headers_.setHost("ImmediateMiss" + std::to_string(request)); + + // Create filter for request 1 + CacheFilter filter = makeFilter(simple_cache_); + + // Decode request 1 header + // Make sure the filter did not encode any headers or data + EXPECT_CALL(decoder_callbacks_, encodeHeaders_).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); + EXPECT_EQ(filter.decodeHeaders(request_headers_, true), Http::FilterHeadersStatus::Continue); + ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); + + // Encode response header + EXPECT_EQ(filter.encodeHeaders(response_headers_, true), Http::FilterHeadersStatus::Continue); + filter.onDestroy(); + } +} + +TEST_F(CacheFilterTest, DelayedMiss) { + for (int request = 1; request <= 2; request++) { + // Each iteration a request is sent to a different host, therefore the second one is a miss + request_headers_.setHost("DelayedMiss" + std::to_string(request)); + + // Create filter for request 1 + CacheFilter filter = makeFilter(delayed_cache_); + + // Decode request 1 header + // No hit will be found, so the filter should call continueDecoding + EXPECT_CALL(decoder_callbacks_, continueDecoding); + // Make sure the filter did not encode any headers or data + EXPECT_CALL(decoder_callbacks_, encodeHeaders_).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); + // The filter should stop iteration waiting for cache response + EXPECT_EQ(filter.decodeHeaders(request_headers_, true), + Http::FilterHeadersStatus::StopAllIterationAndWatermark); + + // Make the delayed callback to call onHeaders + delayed_cache_.delayed_headers_cb_(); + + ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); + + // Encode response header + EXPECT_EQ(filter.encodeHeaders(response_headers_, true), Http::FilterHeadersStatus::Continue); + filter.onDestroy(); + } +} + TEST_F(CacheFilterTest, ImmediateHitNoBody) { request_headers_.setHost("ImmediateHitNoBody"); - ON_CALL(decoder_callbacks_, dispatcher()).WillByDefault(ReturnRef(context_.dispatcher_)); - ON_CALL(context_.dispatcher_, post(_)).WillByDefault(::testing::InvokeArgument<0>()); { // Create filter for request 1. CacheFilter filter = makeFilter(simple_cache_); - // Decode request 1 header. + // Decode request 1 header + // Make sure the filter did not encode any headers or data + EXPECT_CALL(decoder_callbacks_, encodeHeaders_).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); EXPECT_EQ(filter.decodeHeaders(request_headers_, true), Http::FilterHeadersStatus::Continue); + ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); // Encode response header. EXPECT_EQ(filter.encodeHeaders(response_headers_, true), Http::FilterHeadersStatus::Continue); @@ -96,8 +166,12 @@ TEST_F(CacheFilterTest, ImmediateHitNoBody) { // Decode request 2 header. EXPECT_CALL(decoder_callbacks_, encodeHeaders_(testing::AllOf(IsSupersetOfHeaders(response_headers_), - HeaderHasValueRef("age", "0")), + HeaderHasValueRef(Http::Headers::get().Age, "0")), true)); + // Make sure the filter did not encode any data + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); + // Make sure decoding does not continue + EXPECT_CALL(decoder_callbacks_, continueDecoding).Times(0); EXPECT_EQ(filter.decodeHeaders(request_headers_, true), Http::FilterHeadersStatus::StopAllIterationAndWatermark); ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); @@ -106,19 +180,25 @@ TEST_F(CacheFilterTest, ImmediateHitNoBody) { } TEST_F(CacheFilterTest, DelayedHitNoBody) { - request_headers_.setHost("ImmediateHitNoBody"); - ON_CALL(decoder_callbacks_, dispatcher()).WillByDefault(ReturnRef(context_.dispatcher_)); - ON_CALL(context_.dispatcher_, post(_)).WillByDefault(::testing::InvokeArgument<0>()); + request_headers_.setHost("DelayedHitNoBody"); { // Create filter for request 1. CacheFilter filter = makeFilter(delayed_cache_); - // Decode request 1 header. + // Decode request 1 header + // No hit will be found, so the filter should call continueDecoding + EXPECT_CALL(decoder_callbacks_, continueDecoding); + // Make sure the filter did not encode any headers or data + EXPECT_CALL(decoder_callbacks_, encodeHeaders_).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); + // The filter should stop iteration waiting for cache response EXPECT_EQ(filter.decodeHeaders(request_headers_, true), Http::FilterHeadersStatus::StopAllIterationAndWatermark); - EXPECT_CALL(decoder_callbacks_, continueDecoding); - delayed_cache_.delayed_cb_(); + + // Make the delayed callback to call onHeaders + delayed_cache_.delayed_headers_cb_(); + ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); // Encode response header. @@ -129,14 +209,18 @@ TEST_F(CacheFilterTest, DelayedHitNoBody) { // Create filter for request 2. CacheFilter filter = makeFilter(delayed_cache_); - // Decode request 2 header. - EXPECT_EQ(filter.decodeHeaders(request_headers_, true), - Http::FilterHeadersStatus::StopAllIterationAndWatermark); + // Decode request 2 header EXPECT_CALL(decoder_callbacks_, encodeHeaders_(testing::AllOf(IsSupersetOfHeaders(response_headers_), - HeaderHasValueRef("age", "0")), + HeaderHasValueRef(Http::Headers::get().Age, "0")), true)); - delayed_cache_.delayed_cb_(); + // Make sure the filter did not encode any data + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); + // Make sure decoding does not continue + EXPECT_CALL(decoder_callbacks_, continueDecoding).Times(0); + EXPECT_EQ(filter.decodeHeaders(request_headers_, true), + Http::FilterHeadersStatus::StopAllIterationAndWatermark); + delayed_cache_.delayed_headers_cb_(); ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); filter.onDestroy(); } @@ -144,15 +228,16 @@ TEST_F(CacheFilterTest, DelayedHitNoBody) { TEST_F(CacheFilterTest, ImmediateHitBody) { request_headers_.setHost("ImmediateHitBody"); - ON_CALL(decoder_callbacks_, dispatcher()).WillByDefault(ReturnRef(context_.dispatcher_)); - ON_CALL(context_.dispatcher_, post(_)).WillByDefault(::testing::InvokeArgument<0>()); const std::string body = "abc"; { // Create filter for request 1. CacheFilter filter = makeFilter(simple_cache_); - // Decode request 1 header. + // Decode request 1 header + // Make sure the filter did not encode any headers or data + EXPECT_CALL(decoder_callbacks_, encodeHeaders_).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); EXPECT_EQ(filter.decodeHeaders(request_headers_, true), Http::FilterHeadersStatus::Continue); // Encode response header. @@ -169,14 +254,403 @@ TEST_F(CacheFilterTest, ImmediateHitBody) { // Decode request 2 header EXPECT_CALL(decoder_callbacks_, encodeHeaders_(testing::AllOf(IsSupersetOfHeaders(response_headers_), - HeaderHasValueRef("age", "0")), + HeaderHasValueRef(Http::Headers::get().Age, "0")), + false)); + EXPECT_CALL( + decoder_callbacks_, + encodeData(testing::Property(&Buffer::Instance::toString, testing::Eq(body)), true)); + // Make sure decoding does not continue + EXPECT_CALL(decoder_callbacks_, continueDecoding).Times(0); + EXPECT_EQ(filter.decodeHeaders(request_headers_, true), + Http::FilterHeadersStatus::StopAllIterationAndWatermark); + ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); + filter.onDestroy(); + } +} + +TEST_F(CacheFilterTest, DelayedHitBody) { + request_headers_.setHost("DelayedHitBody"); + const std::string body = "abc"; + + { + // Create filter for request 1 + CacheFilter filter = makeFilter(delayed_cache_); + + // Decode request 1 header + // No hit will be found, so the filter should call continueDecoding + EXPECT_CALL(decoder_callbacks_, continueDecoding); + // Make sure the filter did not encode any headers or data + EXPECT_CALL(decoder_callbacks_, encodeHeaders_).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); + // The filter should stop iteration waiting for cache response + EXPECT_EQ(filter.decodeHeaders(request_headers_, true), + Http::FilterHeadersStatus::StopAllIterationAndWatermark); + + // Make the delayed callback to call onHeaders + delayed_cache_.delayed_headers_cb_(); + + ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); + + // Encode response header + Buffer::OwnedImpl buffer(body); + response_headers_.setContentLength(body.size()); + EXPECT_EQ(filter.encodeHeaders(response_headers_, false), Http::FilterHeadersStatus::Continue); + EXPECT_EQ(filter.encodeData(buffer, true), Http::FilterDataStatus::Continue); + filter.onDestroy(); + } + { + // Create filter for request 2 + CacheFilter filter = makeFilter(delayed_cache_); + + // Decode request 2 header + + EXPECT_CALL(decoder_callbacks_, + encodeHeaders_(testing::AllOf(IsSupersetOfHeaders(response_headers_), + HeaderHasValueRef(Http::Headers::get().Age, "0")), false)); EXPECT_CALL( decoder_callbacks_, encodeData(testing::Property(&Buffer::Instance::toString, testing::Eq(body)), true)); + + // Make sure decoding does not continue + EXPECT_CALL(decoder_callbacks_, continueDecoding).Times(0); + EXPECT_EQ(filter.decodeHeaders(request_headers_, true), + Http::FilterHeadersStatus::StopAllIterationAndWatermark); + + // Make the delayed callbacks to call onHeaders & onBody + delayed_cache_.delayed_headers_cb_(); + delayed_cache_.delayed_body_cb_(); + ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); + filter.onDestroy(); + } +} + +TEST_F(CacheFilterTest, ImmediateSuccessfulValidation) { + request_headers_.setHost("ImmediateSuccessfulValidation"); + const std::string body = "abc"; + + { + // Create filter for request 1 + CacheFilter filter = makeFilter(simple_cache_); + + // Decode request 1 header + // Make sure the filter did not encode any headers or data + EXPECT_CALL(decoder_callbacks_, encodeHeaders_).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); + EXPECT_EQ(filter.decodeHeaders(request_headers_, true), Http::FilterHeadersStatus::Continue); + + // Encode response + + // Add Etag & Last-Modified headers to the response for validation + response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, "abc123"); + response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, + formatter_.now(time_source_)); + + Buffer::OwnedImpl buffer(body); + response_headers_.setContentLength(body.size()); + EXPECT_EQ(filter.encodeHeaders(response_headers_, false), Http::FilterHeadersStatus::Continue); + EXPECT_EQ(filter.encodeData(buffer, true), Http::FilterDataStatus::Continue); + filter.onDestroy(); + } + { + // Create filter for request 2 + CacheFilter filter = makeFilter(simple_cache_); + + // Make request require validation + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); + + // Decode request 2 header + // Make sure the filter did not encode any headers or data (during decoding) + EXPECT_CALL(decoder_callbacks_, encodeHeaders_).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); + EXPECT_EQ(filter.decodeHeaders(request_headers_, true), Http::FilterHeadersStatus::Continue); + + // Make sure validation conditional headers are added + const Http::TestRequestHeaderMapImpl injected_headers = { + {"if-none-match", "abc123"}, {"if-modified-since", formatter_.now(time_source_)}}; + EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); + + ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); + + // Encode 304 response + // Advance time to make sure the cached date is updated with the 304 date + time_source_.advanceTimeWait(std::chrono::seconds(10)); + const std::string not_modified_date = formatter_.now(time_source_); + Http::TestResponseHeaderMapImpl not_modified_response_headers = {{":status", "304"}, + {"date", not_modified_date}}; + + // Check that the cached response body is encoded + Buffer::OwnedImpl buffer(body); + EXPECT_CALL( + encoder_callbacks_, + addEncodedData(testing::Property(&Buffer::Instance::toString, testing::Eq(body)), true)); + + // The cache is immediate so everything should be done before encodeHeaders returns + EXPECT_EQ(filter.encodeHeaders(not_modified_response_headers, true), + Http::FilterHeadersStatus::Continue); + + // Check for the cached response headers with updated date + Http::TestResponseHeaderMapImpl updated_response_headers = response_headers_; + updated_response_headers.setDate(not_modified_date); + EXPECT_THAT(not_modified_response_headers, + testing::AllOf(IsSupersetOfHeaders(updated_response_headers), + HeaderHasValueRef(Http::Headers::get().Age, "0"))); + + // The cache is immediate so everything should be done before CacheFilter::encodeData + EXPECT_EQ(filter.encodeData(buffer, true), Http::FilterDataStatus::Continue); + + ::testing::Mock::VerifyAndClearExpectations(&encoder_callbacks_); + + filter.onDestroy(); + } +} + +TEST_F(CacheFilterTest, DelayedSuccessfulValidation) { + request_headers_.setHost("DelayedSuccessfulValidation"); + const std::string body = "abc"; + + { + // Create filter for request 1 + CacheFilter filter = makeFilter(delayed_cache_); + + // Decode request 1 header + // No hit will be found, so the filter should call continueDecoding + EXPECT_CALL(decoder_callbacks_, continueDecoding); + // Make sure the filter did not encode any headers or data + EXPECT_CALL(decoder_callbacks_, encodeHeaders_).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); + // The filter should stop iteration waiting for cache response + EXPECT_EQ(filter.decodeHeaders(request_headers_, true), + Http::FilterHeadersStatus::StopAllIterationAndWatermark); + + // Make the delayed callback to call onHeaders + delayed_cache_.delayed_headers_cb_(); + + ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); + + // Encode response + + // Add Etag & Last-Modified headers to the response for validation + response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, "abc123"); + response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, + formatter_.now(time_source_)); + + Buffer::OwnedImpl buffer(body); + response_headers_.setContentLength(body.size()); + EXPECT_EQ(filter.encodeHeaders(response_headers_, false), Http::FilterHeadersStatus::Continue); + EXPECT_EQ(filter.encodeData(buffer, true), Http::FilterDataStatus::Continue); + filter.onDestroy(); + } + { + // Create filter for request 2 + CacheFilter filter = makeFilter(delayed_cache_); + + // Make request require validation + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); + + // Decode request 2 header + // No hit will be found, so the filter should call continueDecoding + EXPECT_CALL(decoder_callbacks_, continueDecoding); + // Make sure the filter did not encode any headers or data + EXPECT_CALL(decoder_callbacks_, encodeHeaders_).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); + // The filter should stop iteration waiting for cache response + EXPECT_EQ(filter.decodeHeaders(request_headers_, true), + Http::FilterHeadersStatus::StopAllIterationAndWatermark); + + // Make the delayed callback to call onHeaders + delayed_cache_.delayed_headers_cb_(); + + // Make sure validation conditional headers are added + const Http::TestRequestHeaderMapImpl injected_headers = { + {"if-none-match", "abc123"}, {"if-modified-since", formatter_.now(time_source_)}}; + EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); + + ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); + + // Encode 304 response + // Advance time to make sure the cached date is updated with the 304 date + time_source_.advanceTimeWait(std::chrono::seconds(10)); + const std::string not_modified_date = formatter_.now(time_source_); + Http::TestResponseHeaderMapImpl not_modified_response_headers = {{":status", "304"}, + {"date", not_modified_date}}; + + // Check that the cached response body is encoded + Buffer::OwnedImpl buffer(body); + EXPECT_CALL( + encoder_callbacks_, + addEncodedData(testing::Property(&Buffer::Instance::toString, testing::Eq(body)), true)); + + // The cache is delayed so encoding iteration should be stopped until the body is encoded + // StopIteration does not stop encodeData of the same filter from being called + EXPECT_EQ(filter.encodeHeaders(not_modified_response_headers, true), + Http::FilterHeadersStatus::StopIteration); + + // Check for the cached response headers with updated date + Http::TestResponseHeaderMapImpl updated_response_headers = response_headers_; + updated_response_headers.setDate(not_modified_date); + EXPECT_THAT(not_modified_response_headers, + testing::AllOf(IsSupersetOfHeaders(updated_response_headers), + HeaderHasValueRef(Http::Headers::get().Age, "0"))); + + // A 304 response should not have a body, so encodeData should not be called + // However, if a body is present by mistake, encodeData should stop iteration until + // encoding the cached response is done + EXPECT_EQ(filter.encodeData(buffer, true), Http::FilterDataStatus::StopIterationAndBuffer); + + // Delayed call to onBody to encode cached response + delayed_cache_.delayed_body_cb_(); + + ::testing::Mock::VerifyAndClearExpectations(&encoder_callbacks_); + + filter.onDestroy(); + } +} + +TEST_F(CacheFilterTest, ImmediateUnsuccessfulValidation) { + request_headers_.setHost("ImmediateUnsuccessfulValidation"); + + { + // Create filter for request 1 + CacheFilter filter = makeFilter(simple_cache_); + + // Decode request 1 header + // Make sure the filter did not encode any headers or data + EXPECT_CALL(decoder_callbacks_, encodeHeaders_).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); + EXPECT_EQ(filter.decodeHeaders(request_headers_, true), Http::FilterHeadersStatus::Continue); + + // Encode response + + // Add Etag & Last-Modified headers to the response for validation + response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, "abc123"); + response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, + formatter_.now(time_source_)); + + const std::string body = "abc"; + Buffer::OwnedImpl buffer(body); + response_headers_.setContentLength(body.size()); + EXPECT_EQ(filter.encodeHeaders(response_headers_, false), Http::FilterHeadersStatus::Continue); + EXPECT_EQ(filter.encodeData(buffer, true), Http::FilterDataStatus::Continue); + filter.onDestroy(); + } + { + // Create filter for request 2 + CacheFilter filter = makeFilter(simple_cache_); + + // Make request require validation + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); + + // Decode request 2 header + // Make sure the filter did not encode any headers or data (during decoding) + EXPECT_CALL(decoder_callbacks_, encodeHeaders_).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); + EXPECT_EQ(filter.decodeHeaders(request_headers_, true), Http::FilterHeadersStatus::Continue); + + // Make sure validation conditional headers are added + const Http::TestRequestHeaderMapImpl injected_headers = { + {"if-none-match", "abc123"}, {"if-modified-since", formatter_.now(time_source_)}}; + EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); + + ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); + + // Encode new response + // Change the status code to make sure new headers are served, not the cached ones + response_headers_.setStatus(201); + + // Check that no cached data is encoded + EXPECT_CALL(encoder_callbacks_, addEncodedData).Times(0); + + // The cache is immediate so everything should be done before encodeHeaders returns + EXPECT_EQ(filter.encodeHeaders(response_headers_, false), Http::FilterHeadersStatus::Continue); + + // Check for the cached response headers with updated date + EXPECT_THAT(response_headers_, HeaderHasValueRef(Http::Headers::get().Status, "201")); + + ::testing::Mock::VerifyAndClearExpectations(&encoder_callbacks_); + + filter.onDestroy(); + } +} + +TEST_F(CacheFilterTest, DelayedUnuccessfulValidation) { + request_headers_.setHost("DelayedUnuccessfulValidation"); + + { + // Create filter for request 1 + CacheFilter filter = makeFilter(delayed_cache_); + + // Decode request 1 header + // No hit will be found, so the filter should call continueDecoding + EXPECT_CALL(decoder_callbacks_, continueDecoding); + // Make sure the filter did not encode any headers or data + EXPECT_CALL(decoder_callbacks_, encodeHeaders_).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); + // The filter should stop iteration waiting for cache response + EXPECT_EQ(filter.decodeHeaders(request_headers_, true), + Http::FilterHeadersStatus::StopAllIterationAndWatermark); + + // Make the delayed callback to call onHeaders + delayed_cache_.delayed_headers_cb_(); + + ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); + + // Encode response + + // Add Etag & Last-Modified headers to the response for validation + response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, "abc123"); + response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, + formatter_.now(time_source_)); + + const std::string body = "abc"; + Buffer::OwnedImpl buffer(body); + response_headers_.setContentLength(body.size()); + EXPECT_EQ(filter.encodeHeaders(response_headers_, false), Http::FilterHeadersStatus::Continue); + EXPECT_EQ(filter.encodeData(buffer, true), Http::FilterDataStatus::Continue); + filter.onDestroy(); + } + { + // Create filter for request 2 + CacheFilter filter = makeFilter(delayed_cache_); + + // Make request require validation + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); + + // Decode request 2 header + // No hit will be found, so the filter should call continueDecoding + EXPECT_CALL(decoder_callbacks_, continueDecoding); + // Make sure the filter did not encode any headers or data + EXPECT_CALL(decoder_callbacks_, encodeHeaders_).Times(0); + EXPECT_CALL(decoder_callbacks_, encodeData).Times(0); + // The filter should stop iteration waiting for cache response EXPECT_EQ(filter.decodeHeaders(request_headers_, true), Http::FilterHeadersStatus::StopAllIterationAndWatermark); + + // Make the delayed callback to call onHeaders + delayed_cache_.delayed_headers_cb_(); + + // Make sure validation conditional headers are added + const Http::TestRequestHeaderMapImpl injected_headers = { + {"if-none-match", "abc123"}, {"if-modified-since", formatter_.now(time_source_)}}; + EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); + ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); + + // Encode new response + // Change the status code to make sure new headers are served, not the cached ones + response_headers_.setStatus(201); + + // Check that no cached response body is encoded + EXPECT_CALL(encoder_callbacks_, addEncodedData).Times(0); + + // The delayed cache has no effect here as nothing is fetched from cache + EXPECT_EQ(filter.encodeHeaders(response_headers_, false), Http::FilterHeadersStatus::Continue); + + // Check for the cached response headers with updated date + EXPECT_THAT(response_headers_, HeaderHasValueRef(Http::Headers::get().Status, "201")); + + ::testing::Mock::VerifyAndClearExpectations(&encoder_callbacks_); + filter.onDestroy(); } } @@ -200,6 +674,153 @@ TEST_F(CacheFilterTest, GetRequestWithBodyAndTrailers) { } } +// A new type alias for a different type of tests that use the exact same class +using ValidationHeadersTest = CacheFilterTest; + +TEST_F(ValidationHeadersTest, EtagAndLastModified) { + request_headers_.setHost("EtagAndLastModified"); + const std::string etag = "abc123"; + + // Make request 1 to insert the response into cache + { + CacheFilter filter = makeFilter(simple_cache_); + filter.decodeHeaders(request_headers_, true); + + // Add validation headers to the response + response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, etag); + response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, + formatter_.now(time_source_)); + + filter.encodeHeaders(response_headers_, true); + } + // Make request 2 to test for added conditional headers + { + CacheFilter filter = makeFilter(simple_cache_); + + // Make sure the request requires validation + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); + filter.decodeHeaders(request_headers_, true); + + // Make sure validation conditional headers are added + const Http::TestRequestHeaderMapImpl injected_headers = { + {"if-none-match", "abc123"}, {"if-modified-since", formatter_.now(time_source_)}}; + EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); + } +} + +TEST_F(ValidationHeadersTest, EtagOnly) { + request_headers_.setHost("EtagOnly"); + const std::string etag = "abc123"; + + // Make request 1 to insert the response into cache + { + CacheFilter filter = makeFilter(simple_cache_); + filter.decodeHeaders(request_headers_, true); + + // Add validation headers to the response + response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, etag); + + filter.encodeHeaders(response_headers_, true); + } + // Make request 2 to test for added conditional headers + { + CacheFilter filter = makeFilter(simple_cache_); + + // Make sure the request requires validation + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); + filter.decodeHeaders(request_headers_, true); + + // Make sure validation conditional headers are added + // If-Modified-Since falls back to date + const Http::TestRequestHeaderMapImpl injected_headers = { + {"if-none-match", "abc123"}, {"if-modified-since", formatter_.now(time_source_)}}; + EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); + } +} + +TEST_F(ValidationHeadersTest, LastModifiedOnly) { + request_headers_.setHost("LastModifiedOnly"); + + // Make request 1 to insert the response into cache + { + CacheFilter filter = makeFilter(simple_cache_); + filter.decodeHeaders(request_headers_, true); + + // Add validation headers to the response + response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, + formatter_.now(time_source_)); + + filter.encodeHeaders(response_headers_, true); + } + // Make request 2 to test for added conditional headers + { + CacheFilter filter = makeFilter(simple_cache_); + + // Make sure the request requires validation + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); + filter.decodeHeaders(request_headers_, true); + + // Make sure validation conditional headers are added + const Http::TestRequestHeaderMapImpl injected_headers = { + {"if-modified-since", formatter_.now(time_source_)}}; + EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); + } +} + +TEST_F(ValidationHeadersTest, NoEtagOrLastModified) { + request_headers_.setHost("NoEtagOrLastModified"); + + // Make request 1 to insert the response into cache + { + CacheFilter filter = makeFilter(simple_cache_); + filter.decodeHeaders(request_headers_, true); + filter.encodeHeaders(response_headers_, true); + } + // Make request 2 to test for added conditional headers + { + CacheFilter filter = makeFilter(simple_cache_); + + // Make sure the request requires validation + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); + filter.decodeHeaders(request_headers_, true); + + // Make sure validation conditional headers are added + // If-Modified-Since falls back to date + const Http::TestRequestHeaderMapImpl injected_headers = { + {"if-modified-since", formatter_.now(time_source_)}}; + EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); + } +} + +TEST_F(ValidationHeadersTest, InvalidLastModified) { + request_headers_.setHost("InvalidLastModified"); + + // Make request 1 to insert the response into cache + { + CacheFilter filter = makeFilter(simple_cache_); + filter.decodeHeaders(request_headers_, true); + + // Add validation headers to the response + response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, "invalid-date"); + + filter.encodeHeaders(response_headers_, true); + } + // Make request 2 to test for added conditional headers + { + CacheFilter filter = makeFilter(simple_cache_); + + // Make sure the request requires validation + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); + filter.decodeHeaders(request_headers_, true); + + // Make sure validation conditional headers are added + // If-Modified-Since falls back to date + const Http::TestRequestHeaderMapImpl injected_headers = { + {"if-modified-since", formatter_.now(time_source_)}}; + EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); + } +} + } // namespace } // namespace Cache } // namespace HttpFilters diff --git a/test/extensions/filters/http/cache/cache_headers_utils_test.cc b/test/extensions/filters/http/cache/cache_headers_utils_test.cc index dd3f0a78e52b..c9a0f2df7137 100644 --- a/test/extensions/filters/http/cache/cache_headers_utils_test.cc +++ b/test/extensions/filters/http/cache/cache_headers_utils_test.cc @@ -9,6 +9,7 @@ #include "extensions/filters/http/cache/cache_headers_utils.h" +#include "test/extensions/filters/http/cache/common.h" #include "test/test_common/utility.h" #include "gtest/gtest.h" @@ -302,13 +303,19 @@ TEST_P(ResponseCacheControlTest, ResponseCacheControlTest) { INSTANTIATE_TEST_SUITE_P(Ok, HttpTimeTest, testing::ValuesIn(HttpTimeTest::getOkTestCases())); -TEST_P(HttpTimeTest, Ok) { +TEST_P(HttpTimeTest, OkFormats) { const Http::TestResponseHeaderMapImpl response_headers{{"date", GetParam()}}; // Manually confirmed that 784111777 is 11/6/94, 8:46:37. EXPECT_EQ(784111777, SystemTime::clock::to_time_t(CacheHeadersUtils::httpTime(response_headers.Date()))); } +TEST(HttpTime, InvalidFormat) { + const std::string invalid_format_date = "Sunday, 06-11-1994 08:49:37"; + const Http::TestResponseHeaderMapImpl response_headers{{"date", invalid_format_date}}; + EXPECT_EQ(CacheHeadersUtils::httpTime(response_headers.Date()), SystemTime()); +} + TEST(HttpTime, Null) { EXPECT_EQ(CacheHeadersUtils::httpTime(nullptr), SystemTime()); } void testReadAndRemoveLeadingDigits(absl::string_view input, int64_t expected, diff --git a/test/extensions/filters/http/cache/cacheability_utils_test.cc b/test/extensions/filters/http/cache/cacheability_utils_test.cc index f4647e8bfc3f..598edf2d2147 100644 --- a/test/extensions/filters/http/cache/cacheability_utils_test.cc +++ b/test/extensions/filters/http/cache/cacheability_utils_test.cc @@ -12,121 +12,137 @@ namespace HttpFilters { namespace Cache { namespace { -class IsCacheableRequestTest : public testing::TestWithParam { +class IsCacheableRequestTest : public testing::Test { protected: - const Http::TestRequestHeaderMapImpl cacheable_request_headers_ = {{":path", "/"}, - {":method", "GET"}, - {"x-forwarded-proto", "http"}, - {":authority", "test.com"}}; + Http::TestRequestHeaderMapImpl request_headers_ = {{":path", "/"}, + {":method", "GET"}, + {"x-forwarded-proto", "http"}, + {":authority", "test.com"}}; +}; + +class RequestConditionalHeadersTest : public testing::TestWithParam { +protected: + Http::TestRequestHeaderMapImpl request_headers_ = {{":path", "/"}, + {":method", "GET"}, + {"x-forwarded-proto", "http"}, + {":authority", "test.com"}}; + std::string conditionalHeader() const { return GetParam(); } }; class IsCacheableResponseTest : public testing::Test { protected: std::string cache_control_ = "max-age=3600"; - const Http::TestResponseHeaderMapImpl cacheable_response_headers_ = { - {":status", "200"}, - {"date", "Sun, 06 Nov 1994 08:49:37 GMT"}, - {"cache-control", cache_control_}}; + Http::TestResponseHeaderMapImpl response_headers_ = {{":status", "200"}, + {"date", "Sun, 06 Nov 1994 08:49:37 GMT"}, + {"cache-control", cache_control_}}; }; TEST_F(IsCacheableRequestTest, CacheableRequest) { - EXPECT_TRUE(CacheabilityUtils::isCacheableRequest(cacheable_request_headers_)); + EXPECT_TRUE(CacheabilityUtils::isCacheableRequest(request_headers_)); } TEST_F(IsCacheableRequestTest, PathHeader) { - Http::TestRequestHeaderMapImpl request_headers = cacheable_request_headers_; - EXPECT_TRUE(CacheabilityUtils::isCacheableRequest(request_headers)); - request_headers.removePath(); - EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers)); + EXPECT_TRUE(CacheabilityUtils::isCacheableRequest(request_headers_)); + request_headers_.removePath(); + EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers_)); } TEST_F(IsCacheableRequestTest, HostHeader) { - Http::TestRequestHeaderMapImpl request_headers = cacheable_request_headers_; - EXPECT_TRUE(CacheabilityUtils::isCacheableRequest(request_headers)); - request_headers.removeHost(); - EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers)); + EXPECT_TRUE(CacheabilityUtils::isCacheableRequest(request_headers_)); + request_headers_.removeHost(); + EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers_)); } TEST_F(IsCacheableRequestTest, MethodHeader) { const Http::HeaderValues& header_values = Http::Headers::get(); - Http::TestRequestHeaderMapImpl request_headers = cacheable_request_headers_; - EXPECT_TRUE(CacheabilityUtils::isCacheableRequest(request_headers)); - request_headers.setMethod(header_values.MethodValues.Post); - EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers)); - request_headers.setMethod(header_values.MethodValues.Put); - EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers)); - request_headers.removeMethod(); - EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers)); + EXPECT_TRUE(CacheabilityUtils::isCacheableRequest(request_headers_)); + request_headers_.setMethod(header_values.MethodValues.Post); + EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers_)); + request_headers_.setMethod(header_values.MethodValues.Put); + EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers_)); + request_headers_.removeMethod(); + EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers_)); } TEST_F(IsCacheableRequestTest, ForwardedProtoHeader) { - Http::TestRequestHeaderMapImpl request_headers = cacheable_request_headers_; - EXPECT_TRUE(CacheabilityUtils::isCacheableRequest(request_headers)); - request_headers.setForwardedProto("ftp"); - EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers)); - request_headers.removeForwardedProto(); - EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers)); + EXPECT_TRUE(CacheabilityUtils::isCacheableRequest(request_headers_)); + request_headers_.setForwardedProto("ftp"); + EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers_)); + request_headers_.removeForwardedProto(); + EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers_)); } TEST_F(IsCacheableRequestTest, AuthorizationHeader) { - Http::TestRequestHeaderMapImpl request_headers = cacheable_request_headers_; - EXPECT_TRUE(CacheabilityUtils::isCacheableRequest(request_headers)); - request_headers.setCopy(Http::CustomHeaders::get().Authorization, - "basic YWxhZGRpbjpvcGVuc2VzYW1l"); - EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers)); + EXPECT_TRUE(CacheabilityUtils::isCacheableRequest(request_headers_)); + request_headers_.setReferenceKey(Http::CustomHeaders::get().Authorization, + "basic YWxhZGRpbjpvcGVuc2VzYW1l"); + EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers_)); } -INSTANTIATE_TEST_SUITE_P(ConditionalHeaders, IsCacheableRequestTest, +INSTANTIATE_TEST_SUITE_P(ConditionalHeaders, RequestConditionalHeadersTest, testing::Values("if-match", "if-none-match", "if-modified-since", - "if-unmodified-since", "if-range")); - -TEST_P(IsCacheableRequestTest, ConditionalHeaders) { - Http::TestRequestHeaderMapImpl request_headers = cacheable_request_headers_; - EXPECT_TRUE(CacheabilityUtils::isCacheableRequest(request_headers)); - request_headers.setCopy(Http::LowerCaseString{GetParam()}, "test-value"); - EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers)); + "if-unmodified-since", "if-range"), + [](const auto& info) { + std::string test_name = info.param; + absl::c_replace_if( + test_name, [](char c) { return !std::isalnum(c); }, '_'); + return test_name; + }); + +TEST_P(RequestConditionalHeadersTest, ConditionalHeaders) { + EXPECT_TRUE(CacheabilityUtils::isCacheableRequest(request_headers_)); + request_headers_.setCopy(Http::LowerCaseString{conditionalHeader()}, "test-value"); + EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers_)); } TEST_F(IsCacheableResponseTest, CacheableResponse) { - EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(cacheable_response_headers_)); + EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers_)); } TEST_F(IsCacheableResponseTest, UncacheableStatusCode) { - Http::TestResponseHeaderMapImpl response_headers = cacheable_response_headers_; - EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers)); - response_headers.setStatus("700"); - EXPECT_FALSE(CacheabilityUtils::isCacheableResponse(response_headers)); - response_headers.removeStatus(); - EXPECT_FALSE(CacheabilityUtils::isCacheableResponse(response_headers)); + EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers_)); + response_headers_.setStatus("700"); + EXPECT_FALSE(CacheabilityUtils::isCacheableResponse(response_headers_)); + response_headers_.removeStatus(); + EXPECT_FALSE(CacheabilityUtils::isCacheableResponse(response_headers_)); } TEST_F(IsCacheableResponseTest, ValidationData) { - Http::TestResponseHeaderMapImpl response_headers = cacheable_response_headers_; - EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers)); - response_headers.setCopy(Http::CustomHeaders::get().CacheControl, "s-maxage=1000"); - EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers)); - response_headers.setCopy(Http::CustomHeaders::get().CacheControl, "public, no-transform"); - EXPECT_FALSE(CacheabilityUtils::isCacheableResponse(response_headers)); - response_headers.remove(Http::CustomHeaders::get().CacheControl); - EXPECT_FALSE(CacheabilityUtils::isCacheableResponse(response_headers)); - response_headers.setCopy(Http::Headers::get().Expires, "Sun, 06 Nov 1994 09:49:37 GMT"); - EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers)); + EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers_)); + // No cache control headers or expires header + response_headers_.remove(Http::CustomHeaders::get().CacheControl); + EXPECT_FALSE(CacheabilityUtils::isCacheableResponse(response_headers_)); + // No max-age data or expires header + response_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, + "public, no-transform"); + EXPECT_FALSE(CacheabilityUtils::isCacheableResponse(response_headers_)); + // Max-age data available + response_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "s-maxage=1000"); + EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers_)); + // Max-age data available with no date + response_headers_.removeDate(); + EXPECT_FALSE(CacheabilityUtils::isCacheableResponse(response_headers_)); + // No date, but the response requires revalidation anyway + response_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); + EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers_)); + // No cache control headers or date, but there is an expires header + response_headers_.setReferenceKey(Http::Headers::get().Expires, "Sun, 06 Nov 1994 09:49:37 GMT"); + EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers_)); } TEST_F(IsCacheableResponseTest, ResponseNoStore) { - Http::TestResponseHeaderMapImpl response_headers = cacheable_response_headers_; - EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers)); + EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers_)); std::string cache_control_no_store = absl::StrCat(cache_control_, ", no-store"); - response_headers.setCopy(Http::CustomHeaders::get().CacheControl, cache_control_no_store); - EXPECT_FALSE(CacheabilityUtils::isCacheableResponse(response_headers)); + response_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, + cache_control_no_store); + EXPECT_FALSE(CacheabilityUtils::isCacheableResponse(response_headers_)); } TEST_F(IsCacheableResponseTest, ResponsePrivate) { - Http::TestResponseHeaderMapImpl response_headers = cacheable_response_headers_; - EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers)); + EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers_)); std::string cache_control_private = absl::StrCat(cache_control_, ", private"); - response_headers.setCopy(Http::CustomHeaders::get().CacheControl, cache_control_private); - EXPECT_FALSE(CacheabilityUtils::isCacheableResponse(response_headers)); + response_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, cache_control_private); + EXPECT_FALSE(CacheabilityUtils::isCacheableResponse(response_headers_)); } } // namespace diff --git a/test/extensions/filters/http/cache/common.h b/test/extensions/filters/http/cache/common.h new file mode 100644 index 000000000000..6a94861bd8db --- /dev/null +++ b/test/extensions/filters/http/cache/common.h @@ -0,0 +1,134 @@ +#pragma once + +#include "extensions/filters/http/cache/cache_headers_utils.h" +#include "extensions/filters/http/cache/http_cache.h" +#include "extensions/filters/http/cache/simple_http_cache/simple_http_cache.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Cache { + +// Wrapper for SimpleHttpCache that delays the onHeaders/onBody/onTrailers callbacks from +// getHeaders/getBody/getTrailers; for verifying that CacheFilter works correctly whether the +// callbacks happen immediately, or after getHeaders/getBody/getTrailers return +// Used to test the synchronization made using get_headers_state_ & encode_cached_response_state_ +class DelayedCache : public SimpleHttpCache { +public: + // HttpCache + LookupContextPtr makeLookupContext(LookupRequest&& request) override { + return std::make_unique( + SimpleHttpCache::makeLookupContext(std::move(request)), + DelayedCallbacks{delayed_headers_cb_, delayed_body_cb_, delayed_trailers_cb_}); + } + InsertContextPtr makeInsertContext(LookupContextPtr&& lookup_context) override { + return SimpleHttpCache::makeInsertContext( + std::move(dynamic_cast(*lookup_context).context_)); + } + + std::function delayed_headers_cb_, delayed_body_cb_, delayed_trailers_cb_; + +private: + struct DelayedCallbacks { + std::function&headers_cb_, &body_cb_, &trailers_cb_; + }; + class DelayedLookupContext : public LookupContext { + public: + DelayedLookupContext(LookupContextPtr&& context, DelayedCallbacks delayed_callbacks) + : context_(std::move(context)), delayed_callbacks_(delayed_callbacks) {} + void getHeaders(LookupHeadersCallback&& cb) override { + delayed_callbacks_.headers_cb_ = [this, cb]() mutable { + context_->getHeaders(std::move(cb)); + }; + } + void getBody(const AdjustedByteRange& range, LookupBodyCallback&& cb) override { + delayed_callbacks_.body_cb_ = [this, cb, range]() mutable { + context_->getBody(range, std::move(cb)); + }; + } + void getTrailers(LookupTrailersCallback&& cb) override { + delayed_callbacks_.trailers_cb_ = [this, cb]() mutable { + context_->getTrailers(std::move(cb)); + }; + } + + LookupContextPtr context_; + DelayedCallbacks delayed_callbacks_; + }; +}; + +std::ostream& operator<<(std::ostream& os, const RequestCacheControl& request_cache_control) { + std::string s = "{"; + s += request_cache_control.must_validate_ ? "must_validate, " : ""; + s += request_cache_control.no_store_ ? "no_store, " : ""; + s += request_cache_control.no_transform_ ? "no_transform, " : ""; + s += request_cache_control.only_if_cached_ ? "only_if_cached, " : ""; + + s += request_cache_control.max_age_.has_value() + ? "max-age=" + std::to_string(request_cache_control.max_age_.value().count()) + ", " + : ""; + s += request_cache_control.min_fresh_.has_value() + ? "min-fresh=" + std::to_string(request_cache_control.min_fresh_.value().count()) + ", " + : ""; + s += request_cache_control.max_stale_.has_value() + ? "max-stale=" + std::to_string(request_cache_control.max_stale_.value().count()) + ", " + : ""; + + // Remove any extra ", " at the end + if (s.size() > 1) { + s.pop_back(); + s.pop_back(); + } + + s += "}"; + return os << s; +} + +std::ostream& operator<<(std::ostream& os, const ResponseCacheControl& response_cache_control) { + std::string s = "{"; + s += response_cache_control.must_validate_ ? "must_validate, " : ""; + s += response_cache_control.no_store_ ? "no_store, " : ""; + s += response_cache_control.no_transform_ ? "no_transform, " : ""; + s += response_cache_control.no_stale_ ? "no_stale, " : ""; + s += response_cache_control.is_public_ ? "public, " : ""; + + s += response_cache_control.max_age_.has_value() + ? "max-age=" + std::to_string(response_cache_control.max_age_.value().count()) + ", " + : ""; + + // Remove any extra ", " at the end + if (s.size() > 1) { + s.pop_back(); + s.pop_back(); + } + + s += "}"; + return os << s; +} + +std::ostream& operator<<(std::ostream& os, CacheEntryStatus status) { + switch (status) { + case CacheEntryStatus::Ok: + return os << "Ok"; + case CacheEntryStatus::Unusable: + return os << "Unusable"; + case CacheEntryStatus::RequiresValidation: + return os << "RequiresValidation"; + case CacheEntryStatus::FoundNotModified: + return os << "FoundNotModified"; + case CacheEntryStatus::SatisfiableRange: + return os << "SatisfiableRange"; + case CacheEntryStatus::NotSatisfiableRange: + return os << "NotSatisfiableRange"; + } + NOT_REACHED_GCOVR_EXCL_LINE; +} + +std::ostream& operator<<(std::ostream& os, const AdjustedByteRange& range) { + return os << "[" << range.begin() << "," << range.end() << ")"; +} + +} // namespace Cache +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy \ No newline at end of file diff --git a/test/extensions/filters/http/cache/config_test.cc b/test/extensions/filters/http/cache/config_test.cc index 2b05de007256..83459d8328ef 100644 --- a/test/extensions/filters/http/cache/config_test.cc +++ b/test/extensions/filters/http/cache/config_test.cc @@ -42,6 +42,7 @@ TEST_F(CacheFilterFactoryTest, UnregisteredTypedConfig) { envoy::extensions::filters::http::cache::v3alpha::CacheConfig()); EXPECT_THROW(factory_.createFilterFactoryFromProto(config_, "stats", context_), EnvoyException); } + } // namespace } // namespace Cache } // namespace HttpFilters diff --git a/test/extensions/filters/http/cache/http_cache_test.cc b/test/extensions/filters/http/cache/http_cache_test.cc index fac7c099f73d..f4e0361156e6 100644 --- a/test/extensions/filters/http/cache/http_cache_test.cc +++ b/test/extensions/filters/http/cache/http_cache_test.cc @@ -1,6 +1,9 @@ +#include + #include "extensions/filters/http/cache/cache_headers_utils.h" #include "extensions/filters/http/cache/http_cache.h" +#include "test/extensions/filters/http/cache/common.h" #include "test/mocks/http/mocks.h" #include "test/test_common/simulated_time_system.h" #include "test/test_common/utility.h" @@ -15,60 +18,135 @@ namespace Envoy { namespace Extensions { namespace HttpFilters { namespace Cache { -TEST(RawByteRangeTest, IsSuffix) { - auto r = RawByteRange(UINT64_MAX, 4); - ASSERT_TRUE(r.isSuffix()); -} - -TEST(RawByteRangeTest, IsNotSuffix) { - auto r = RawByteRange(3, 4); - ASSERT_FALSE(r.isSuffix()); -} - -TEST(RawByteRangeTest, FirstBytePos) { - auto r = RawByteRange(3, 4); - ASSERT_EQ(3, r.firstBytePos()); -} - -TEST(RawByteRangeTest, LastBytePos) { - auto r = RawByteRange(3, 4); - ASSERT_EQ(4, r.lastBytePos()); -} - -TEST(RawByteRangeTest, SuffixLength) { - auto r = RawByteRange(UINT64_MAX, 4); - ASSERT_EQ(4, r.suffixLength()); -} - -TEST(AdjustedByteRangeTest, Length) { - auto a = AdjustedByteRange(3, 6); - ASSERT_EQ(3, a.length()); -} - -TEST(AdjustedByteRangeTest, TrimFront) { - auto a = AdjustedByteRange(3, 6); - a.trimFront(2); - ASSERT_EQ(5, a.begin()); -} +namespace { -TEST(AdjustedByteRangeTest, MaxLength) { - auto a = AdjustedByteRange(0, UINT64_MAX); - ASSERT_EQ(UINT64_MAX, a.length()); -} +struct LookupRequestTestCase { + std::string test_name, request_cache_control, response_cache_control; + SystemTime request_time, response_time; + CacheEntryStatus expected_cache_entry_status; +}; -TEST(AdjustedByteRangeTest, MaxTrim) { - auto a = AdjustedByteRange(0, UINT64_MAX); - a.trimFront(UINT64_MAX); - ASSERT_EQ(0, a.length()); -} +using Seconds = std::chrono::seconds; -class LookupRequestTest : public testing::Test { -protected: - Event::SimulatedTimeSystem time_source_; - SystemTime current_time_ = time_source_.systemTime(); +class LookupRequestTest : public testing::TestWithParam { +public: DateFormatter formatter_{"%a, %d %b %Y %H:%M:%S GMT"}; - Http::TestRequestHeaderMapImpl request_headers_{ - {":path", "/"}, {"x-forwarded-proto", "https"}, {":authority", "example.com"}}; + Http::TestRequestHeaderMapImpl request_headers_{{":path", "/"}, + {":method", "GET"}, + {"x-forwarded-proto", "https"}, + {":authority", "example.com"}}; + + static const SystemTime& currentTime() { + CONSTRUCT_ON_FIRST_USE(SystemTime, Event::SimulatedTimeSystem().systemTime()); + } + + static const std::vector& getTestCases() { + CONSTRUCT_ON_FIRST_USE(std::vector, + {"request_requires_revalidation", + /*request_cache_control=*/"no-cache", + /*response_cache_control=*/"public, max-age=3600", + /*request_time=*/currentTime(), + /*response_time=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation}, + {"response_requires_revalidation", + /*request_cache_control=*/"", + /*response_cache_control=*/"no-cache", + /*request_time=*/currentTime(), + /*response_time=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation}, + {"future_response_time", + /*request_cache_control=*/"", + /*response_cache_control=*/"public, max-age=3600", + /*request_time=*/currentTime(), + /*response_time=*/currentTime() + Seconds(1), + /*expected_result=*/CacheEntryStatus::RequiresValidation}, + {"request_max_age_satisfied", + /*request_cache_control=*/"max-age=10", + /*response_cache_control=*/"public, max-age=3600", + /*request_time=*/currentTime() + Seconds(9), + /*response_time=*/currentTime(), + /*expected_result=*/CacheEntryStatus::Ok}, + {"request_max_age_unsatisfied", + /*request_cache_control=*/"max-age=10", + /*response_cache_control=*/"public, max-age=3600", + /*request_time=*/currentTime() + Seconds(11), + /*response_time=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation}, + {"request_min_fresh_satisfied", + /*request_cache_control=*/"min-fresh=1000", + /*response_cache_control=*/"public, max-age=2000", + /*request_time=*/currentTime() + Seconds(999), + /*response_time=*/currentTime(), + /*expected_result=*/CacheEntryStatus::Ok}, + {"request_min_fresh_unsatisfied", + /*request_cache_control=*/"min-fresh=1000", + /*response_cache_control=*/"public, max-age=2000", + /*request_time=*/currentTime() + Seconds(1001), + /*response_time=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation}, + {"request_max_age_satisfied_but_min_fresh_unsatisfied", + /*request_cache_control=*/"max-age=1500, min-fresh=1000", + /*response_cache_control=*/"public, max-age=2000", + /*request_time=*/currentTime() + Seconds(1001), + /*response_time=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation}, + {"request_max_age_satisfied_but_max_stale_unsatisfied", + /*request_cache_control=*/"max-age=1500, max-stale=400", + /*response_cache_control=*/"public, max-age=1000", + /*request_time=*/currentTime() + Seconds(1401), + /*response_time=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation}, + {"request_max_stale_satisfied_but_min_fresh_unsatisfied", + /*request_cache_control=*/"min-fresh=1000, max-stale=500", + /*response_cache_control=*/"public, max-age=2000", + /*request_time=*/currentTime() + Seconds(1001), + /*response_time=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation}, + {"request_max_stale_satisfied_but_max_age_unsatisfied", + /*request_cache_control=*/"max-age=1200, max-stale=500", + /*response_cache_control=*/"public, max-age=1000", + /*request_time=*/currentTime() + Seconds(1201), + /*response_time=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation}, + {"request_min_fresh_satisfied_but_max_age_unsatisfied", + /*request_cache_control=*/"max-age=500, min-fresh=400", + /*response_cache_control=*/"public, max-age=1000", + /*request_time=*/currentTime() + Seconds(501), + /*response_time=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation}, + {"expired", + /*request_cache_control=*/"", + /*response_cache_control=*/"public, max-age=1000", + /*request_time=*/currentTime() + Seconds(1001), + /*response_time=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation}, + {"expired_but_max_stale_satisfied", + /*request_cache_control=*/"max-stale=500", + /*response_cache_control=*/"public, max-age=1000", + /*request_time=*/currentTime() + Seconds(1499), + /*response_time=*/currentTime(), + /*expected_result=*/CacheEntryStatus::Ok}, + {"expired_max_stale_unsatisfied", + /*request_cache_control=*/"max-stale=500", + /*response_cache_control=*/"public, max-age=1000", + /*request_time=*/currentTime() + Seconds(1501), + /*response_time=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation}, + {"expired_max_stale_satisfied_but_response_must_revalidate", + /*request_cache_control=*/"max-stale=500", + /*response_cache_control=*/"public, max-age=1000, must-revalidate", + /*request_time=*/currentTime() + Seconds(1499), + /*response_time=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation}, + {"fresh_and_response_must_revalidate", + /*request_cache_control=*/"", + /*response_cache_control=*/"public, max-age=1000, must-revalidate", + /*request_time=*/currentTime() + Seconds(999), + /*response_time=*/currentTime(), + /*expected_result=*/CacheEntryStatus::Ok}, + + ); + } }; LookupResult makeLookupResult(const LookupRequest& lookup_request, @@ -78,12 +156,21 @@ LookupResult makeLookupResult(const LookupRequest& lookup_request, std::make_unique(response_headers), content_length); } -TEST_F(LookupRequestTest, MakeLookupResultNoBody) { - const LookupRequest lookup_request(request_headers_, current_time_); +INSTANTIATE_TEST_SUITE_P(ResultMatchesExpectation, LookupRequestTest, + testing::ValuesIn(LookupRequestTest::getTestCases()), + [](const auto& info) { return info.param.test_name; }); + +TEST_P(LookupRequestTest, ResultWithoutBodyMatchesExpectation) { + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, + GetParam().request_cache_control); + const SystemTime request_time = GetParam().request_time, response_time = GetParam().response_time; + const LookupRequest lookup_request(request_headers_, request_time); const Http::TestResponseHeaderMapImpl response_headers( - {{"date", formatter_.fromTime(current_time_)}, {"cache-control", "public, max-age=3600"}}); + {{"cache-control", GetParam().response_cache_control}, + {"date", formatter_.fromTime(response_time)}}); const LookupResult lookup_response = makeLookupResult(lookup_request, response_headers); - ASSERT_EQ(CacheEntryStatus::Ok, lookup_response.cache_entry_status_); + + EXPECT_EQ(GetParam().expected_cache_entry_status, lookup_response.cache_entry_status_); ASSERT_TRUE(lookup_response.headers_); EXPECT_THAT(*lookup_response.headers_, Http::IsSupersetOfHeaders(response_headers)); EXPECT_EQ(lookup_response.content_length_, 0); @@ -91,69 +178,41 @@ TEST_F(LookupRequestTest, MakeLookupResultNoBody) { EXPECT_FALSE(lookup_response.has_trailers_); } -TEST_F(LookupRequestTest, MakeLookupResultBody) { - const LookupRequest lookup_request(request_headers_, current_time_); +TEST_P(LookupRequestTest, ResultWithBodyMatchesExpectation) { + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, + GetParam().request_cache_control); + const SystemTime request_time = GetParam().request_time, response_time = GetParam().response_time; + const LookupRequest lookup_request(request_headers_, request_time); const Http::TestResponseHeaderMapImpl response_headers( - {{"date", formatter_.fromTime(current_time_)}, {"cache-control", "public, max-age=3600"}}); + {{"cache-control", GetParam().response_cache_control}, + {"date", formatter_.fromTime(response_time)}}); const uint64_t content_length = 5; const LookupResult lookup_response = makeLookupResult(lookup_request, response_headers, content_length); - ASSERT_EQ(CacheEntryStatus::Ok, lookup_response.cache_entry_status_); - ASSERT_TRUE(lookup_response.headers_); - EXPECT_THAT(*lookup_response.headers_, Http::IsSupersetOfHeaders(response_headers)); - EXPECT_EQ(lookup_response.content_length_, content_length); - EXPECT_TRUE(lookup_response.response_ranges_.empty()); - EXPECT_FALSE(lookup_response.has_trailers_); -} - -TEST_F(LookupRequestTest, PrivateResponse) { - const LookupRequest lookup_request(request_headers_, current_time_); - const Http::TestResponseHeaderMapImpl response_headers( - {{"age", "2"}, - {"cache-control", "private, max-age=3600"}, - {"date", formatter_.fromTime(current_time_)}}); - const LookupResult lookup_response = makeLookupResult(lookup_request, response_headers); - - // We must make sure at cache insertion time, private responses must not be - // inserted. However, if the insertion did happen, it would be served at the - // time of lookup. (Nothing should rely on this.) - ASSERT_EQ(CacheEntryStatus::Ok, lookup_response.cache_entry_status_); - ASSERT_TRUE(lookup_response.headers_); - EXPECT_THAT(*lookup_response.headers_, Http::IsSupersetOfHeaders(response_headers)); - EXPECT_EQ(lookup_response.content_length_, 0); - EXPECT_TRUE(lookup_response.response_ranges_.empty()); - EXPECT_FALSE(lookup_response.has_trailers_); -} -TEST_F(LookupRequestTest, Expired) { - const LookupRequest lookup_request(request_headers_, current_time_); - const Http::TestResponseHeaderMapImpl response_headers( - {{"cache-control", "public, max-age=3600"}, {"date", "Thu, 01 Jan 2019 00:00:00 GMT"}}); - const LookupResult lookup_response = makeLookupResult(lookup_request, response_headers); - - EXPECT_EQ(CacheEntryStatus::RequiresValidation, lookup_response.cache_entry_status_); + EXPECT_EQ(GetParam().expected_cache_entry_status, lookup_response.cache_entry_status_); ASSERT_TRUE(lookup_response.headers_); EXPECT_THAT(*lookup_response.headers_, Http::IsSupersetOfHeaders(response_headers)); - EXPECT_EQ(lookup_response.content_length_, 0); + EXPECT_EQ(lookup_response.content_length_, content_length); EXPECT_TRUE(lookup_response.response_ranges_.empty()); EXPECT_FALSE(lookup_response.has_trailers_); } TEST_F(LookupRequestTest, ExpiredViaFallbackheader) { - const LookupRequest lookup_request(request_headers_, current_time_); + const LookupRequest lookup_request(request_headers_, currentTime()); const Http::TestResponseHeaderMapImpl response_headers( - {{"expires", formatter_.fromTime(current_time_ - std::chrono::seconds(5))}, - {"date", formatter_.fromTime(current_time_)}}); + {{"expires", formatter_.fromTime(currentTime() - Seconds(5))}, + {"date", formatter_.fromTime(currentTime())}}); const LookupResult lookup_response = makeLookupResult(lookup_request, response_headers); EXPECT_EQ(CacheEntryStatus::RequiresValidation, lookup_response.cache_entry_status_); } TEST_F(LookupRequestTest, NotExpiredViaFallbackheader) { - const LookupRequest lookup_request(request_headers_, current_time_); + const LookupRequest lookup_request(request_headers_, currentTime()); const Http::TestResponseHeaderMapImpl response_headers( - {{"expires", formatter_.fromTime(current_time_ + std::chrono::seconds(5))}, - {"date", formatter_.fromTime(current_time_)}}); + {{"expires", formatter_.fromTime(currentTime() + Seconds(5))}, + {"date", formatter_.fromTime(currentTime())}}); const LookupResult lookup_response = makeLookupResult(lookup_request, response_headers); EXPECT_EQ(CacheEntryStatus::Ok, lookup_response.cache_entry_status_); } @@ -162,54 +221,54 @@ TEST_F(LookupRequestTest, NotExpiredViaFallbackheader) { // "Pragma:no-cache" is equivalent to "Cache-Control:no-cache". // https://httpwg.org/specs/rfc7234.html#header.pragma TEST_F(LookupRequestTest, PragmaNoCacheFallback) { - request_headers_.addCopy("pragma", "no-cache"); - const LookupRequest lookup_request(request_headers_, current_time_); + request_headers_.setReferenceKey(Http::CustomHeaders::get().Pragma, "no-cache"); + const LookupRequest lookup_request(request_headers_, currentTime()); const Http::TestResponseHeaderMapImpl response_headers( - {{"date", formatter_.fromTime(current_time_)}, {"cache-control", "public, max-age=3600"}}); + {{"date", formatter_.fromTime(currentTime())}, {"cache-control", "public, max-age=3600"}}); const LookupResult lookup_response = makeLookupResult(lookup_request, response_headers); // Response is not expired but the request requires revalidation through Pragma: no-cache. EXPECT_EQ(CacheEntryStatus::RequiresValidation, lookup_response.cache_entry_status_); } TEST_F(LookupRequestTest, PragmaNoCacheFallbackExtraDirectivesIgnored) { - request_headers_.addCopy("pragma", "no-cache, custom-directive=custom-value"); - const LookupRequest lookup_request(request_headers_, current_time_); + request_headers_.setReferenceKey(Http::CustomHeaders::get().Pragma, + "no-cache, custom-directive=custom-value"); + const LookupRequest lookup_request(request_headers_, currentTime()); const Http::TestResponseHeaderMapImpl response_headers( - {{"date", formatter_.fromTime(current_time_)}, {"cache-control", "public, max-age=3600"}}); + {{"date", formatter_.fromTime(currentTime())}, {"cache-control", "public, max-age=3600"}}); const LookupResult lookup_response = makeLookupResult(lookup_request, response_headers); // Response is not expired but the request requires revalidation through Pragma: no-cache. EXPECT_EQ(CacheEntryStatus::RequiresValidation, lookup_response.cache_entry_status_); } TEST_F(LookupRequestTest, PragmaFallbackOtherValuesIgnored) { - request_headers_.addCopy("pragma", "max-age=0"); - const LookupRequest lookup_request(request_headers_, current_time_ + std::chrono::seconds(5)); + request_headers_.setReferenceKey(Http::CustomHeaders::get().Pragma, "max-age=0"); + const LookupRequest lookup_request(request_headers_, currentTime() + std::chrono::seconds(5)); const Http::TestResponseHeaderMapImpl response_headers( - {{"date", formatter_.fromTime(current_time_)}, {"cache-control", "public, max-age=3600"}}); + {{"date", formatter_.fromTime(currentTime())}, {"cache-control", "public, max-age=3600"}}); const LookupResult lookup_response = makeLookupResult(lookup_request, response_headers); // Response is fresh, Pragma header with values other than "no-cache" is ignored. EXPECT_EQ(CacheEntryStatus::Ok, lookup_response.cache_entry_status_); } TEST_F(LookupRequestTest, PragmaNoFallback) { - request_headers_.addCopy("pragma", "no-cache"); - request_headers_.addCopy("cache-control", "max-age=10"); - const LookupRequest lookup_request(request_headers_, current_time_ + std::chrono::seconds(5)); + request_headers_.setReferenceKey(Http::CustomHeaders::get().Pragma, "no-cache"); + request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "max-age=10"); + const LookupRequest lookup_request(request_headers_, currentTime() + std::chrono::seconds(5)); const Http::TestResponseHeaderMapImpl response_headers( - {{"date", formatter_.fromTime(current_time_)}, {"cache-control", "public, max-age=3600"}}); + {{"date", formatter_.fromTime(currentTime())}, {"cache-control", "public, max-age=3600"}}); const LookupResult lookup_response = makeLookupResult(lookup_request, response_headers); // Pragma header is ignored when Cache-Control header is present. EXPECT_EQ(CacheEntryStatus::Ok, lookup_response.cache_entry_status_); } TEST_F(LookupRequestTest, SatisfiableRange) { - // add method (GET) and range to headers - request_headers_.addReference(Http::Headers::get().Method, Http::Headers::get().MethodValues.Get); + // add range to headers request_headers_.addReference(Http::Headers::get().Range, "bytes=1-99,3-,-2"); - const LookupRequest lookup_request(request_headers_, current_time_); + const LookupRequest lookup_request(request_headers_, currentTime()); const Http::TestResponseHeaderMapImpl response_headers( - {{"date", formatter_.fromTime(current_time_)}, + {{"date", formatter_.fromTime(currentTime())}, {"cache-control", "public, max-age=3600"}, {"content-length", "4"}}); const uint64_t content_length = 4; @@ -240,14 +299,13 @@ TEST_F(LookupRequestTest, SatisfiableRange) { } TEST_F(LookupRequestTest, NotSatisfiableRange) { - // add method (GET) and range headers - request_headers_.addReference(Http::Headers::get().Method, Http::Headers::get().MethodValues.Get); + // add range headers request_headers_.addReference(Http::Headers::get().Range, "bytes=5-99,100-"); - const LookupRequest lookup_request(request_headers_, current_time_); + const LookupRequest lookup_request(request_headers_, currentTime()); const Http::TestResponseHeaderMapImpl response_headers( - {{"date", formatter_.fromTime(current_time_)}, + {{"date", formatter_.fromTime(currentTime())}, {"cache-control", "public, max-age=3600"}, {"content-length", "4"}}); const uint64_t content_length = 4; @@ -262,6 +320,53 @@ TEST_F(LookupRequestTest, NotSatisfiableRange) { EXPECT_FALSE(lookup_response.has_trailers_); } +TEST(RawByteRangeTest, IsSuffix) { + auto r = RawByteRange(UINT64_MAX, 4); + ASSERT_TRUE(r.isSuffix()); +} + +TEST(RawByteRangeTest, IsNotSuffix) { + auto r = RawByteRange(3, 4); + ASSERT_FALSE(r.isSuffix()); +} + +TEST(RawByteRangeTest, FirstBytePos) { + auto r = RawByteRange(3, 4); + ASSERT_EQ(3, r.firstBytePos()); +} + +TEST(RawByteRangeTest, LastBytePos) { + auto r = RawByteRange(3, 4); + ASSERT_EQ(4, r.lastBytePos()); +} + +TEST(RawByteRangeTest, SuffixLength) { + auto r = RawByteRange(UINT64_MAX, 4); + ASSERT_EQ(4, r.suffixLength()); +} + +TEST(AdjustedByteRangeTest, Length) { + auto a = AdjustedByteRange(3, 6); + ASSERT_EQ(3, a.length()); +} + +TEST(AdjustedByteRangeTest, TrimFront) { + auto a = AdjustedByteRange(3, 6); + a.trimFront(2); + ASSERT_EQ(5, a.begin()); +} + +TEST(AdjustedByteRangeTest, MaxLength) { + auto a = AdjustedByteRange(0, UINT64_MAX); + ASSERT_EQ(UINT64_MAX, a.length()); +} + +TEST(AdjustedByteRangeTest, MaxTrim) { + auto a = AdjustedByteRange(0, UINT64_MAX); + a.trimFront(UINT64_MAX); + ASSERT_EQ(0, a.length()); +} + struct AdjustByteRangeParams { std::vector request; std::vector result; @@ -469,6 +574,7 @@ TEST_P(ParseInvalidRangeHeaderTest, InvalidRangeReturnsEmpty) { ASSERT_EQ(0, result_vector.size()); } +} // namespace } // namespace Cache } // namespace HttpFilters } // namespace Extensions diff --git a/test/extensions/filters/http/cache/simple_http_cache/BUILD b/test/extensions/filters/http/cache/simple_http_cache/BUILD index 3030d84eeae9..c5f39562f6e2 100644 --- a/test/extensions/filters/http/cache/simple_http_cache/BUILD +++ b/test/extensions/filters/http/cache/simple_http_cache/BUILD @@ -14,6 +14,7 @@ envoy_extension_cc_test( extension_name = "envoy.filters.http.cache.simple_http_cache", deps = [ "//source/extensions/filters/http/cache/simple_http_cache:simple_http_cache_lib", + "//test/extensions/filters/http/cache:common", "//test/test_common:simulated_time_system_lib", "//test/test_common:utility_lib", ], 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 301009223163..acafa5037f55 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 @@ -6,6 +6,7 @@ #include "extensions/filters/http/cache/cache_headers_utils.h" #include "extensions/filters/http/cache/simple_http_cache/simple_http_cache.h" +#include "test/extensions/filters/http/cache/common.h" #include "test/test_common/simulated_time_system.h" #include "test/test_common/utility.h" diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index 4ffedaa0d467..1d74fa10a296 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -972,6 +972,7 @@ restarter resync retriable retriggers +revalidated revalidation rmdir rocketmq