diff --git a/include/envoy/common/time.h b/include/envoy/common/time.h index 4c5694fabef6b..768016dec3f21 100644 --- a/include/envoy/common/time.h +++ b/include/envoy/common/time.h @@ -12,6 +12,7 @@ namespace Envoy { * SystemTime should be used when getting a time to present to the user, e.g. for logging. * MonotonicTime should be used when tracking time for computing an interval. */ +using Seconds = std::chrono::seconds; using SystemTime = std::chrono::time_point; using MonotonicTime = std::chrono::time_point; diff --git a/source/extensions/filters/http/cache/BUILD b/source/extensions/filters/http/cache/BUILD index e1f634c7fe8a5..1f24a71d332a7 100644 --- a/source/extensions/filters/http/cache/BUILD +++ b/source/extensions/filters/http/cache/BUILD @@ -24,6 +24,7 @@ envoy_cc_library( "//source/common/common:enum_to_int", "//source/common/common:logger_lib", "//source/common/common:macros", + "//source/common/http:header_map_lib", "//source/common/http:headers_lib", "//source/common/http:utility_lib", "//source/extensions/filters/http/common:pass_through_filter_lib", @@ -75,11 +76,11 @@ envoy_cc_library( hdrs = ["cache_headers_utils.h"], external_deps = ["abseil_optional"], deps = [ + ":inline_headers_handles", "//include/envoy/common:time_interface", "//include/envoy/http:header_map_interface", "//source/common/http:header_map_lib", "//source/common/http:header_utility_lib", - "//source/common/http:headers_lib", ], ) diff --git a/source/extensions/filters/http/cache/cache_filter.cc b/source/extensions/filters/http/cache/cache_filter.cc index d163160977d21..3ee022cf3eebc 100644 --- a/source/extensions/filters/http/cache/cache_filter.cc +++ b/source/extensions/filters/http/cache/cache_filter.cc @@ -1,5 +1,7 @@ #include "extensions/filters/http/cache/cache_filter.h" +#include "envoy/http/header_map.h" + #include "common/common/enum_to_int.h" #include "common/http/headers.h" #include "common/http/utility.h" @@ -96,10 +98,11 @@ Http::FilterHeadersStatus CacheFilter::encodeHeaders(Http::ResponseHeaderMap& he // Check if the new response can be cached. if (request_allows_inserts_ && CacheabilityUtils::isCacheableResponse(headers, allowed_vary_headers_)) { - // TODO(#12140): Add date internal header or metadata to cached responses. ENVOY_STREAM_LOG(debug, "CacheFilter::encodeHeaders inserting headers", *encoder_callbacks_); insert_ = cache_.makeInsertContext(std::move(lookup_)); - insert_->insertHeaders(headers, end_stream); + // Add metadata associated with the cached response. Right now this is only response_time; + const ResponseMetadata metadata = {time_source_.systemTime()}; + insert_->insertHeaders(headers, metadata, end_stream); } return Http::FilterHeadersStatus::Continue; } @@ -359,10 +362,11 @@ void CacheFilter::processSuccessfulValidation(Http::ResponseHeaderMap& response_ response_headers.setStatus(lookup_result_->headers_->getStatusValue()); response_headers.setContentLength(lookup_result_->headers_->getContentLengthValue()); - // A cache entry was successfully validated -> encode cached body and trailers. - // encodeCachedResponse also adds the age header to lookup_result_ - // so it should be called before headers are merged. - encodeCachedResponse(); + // A response that has been validated should not contain an Age header as it is equivalent to a + // freshly served response from the origin, unless the 304 response has an Age header, which + // means it was served by an upstream cache. + // Remove any existing Age header in the cached response. + lookup_result_->headers_->removeInline(age_handle.handle()); // Add any missing headers from the cached response to the 304 response. lookup_result_->headers_->iterate([&response_headers](const Http::HeaderEntry& cached_header) { @@ -377,8 +381,13 @@ void CacheFilter::processSuccessfulValidation(Http::ResponseHeaderMap& response_ if (should_update_cached_entry) { // TODO(yosrym93): else the cached entry should be deleted. - cache_.updateHeaders(*lookup_, response_headers); + // Update metadata associated with the cached response. Right now this is only response_time; + const ResponseMetadata metadata = {time_source_.systemTime()}; + cache_.updateHeaders(*lookup_, response_headers, metadata); } + + // A cache entry was successfully validated -> encode cached body and trailers. + encodeCachedResponse(); } // TODO(yosrym93): Write a test that exercises this when SimpleHttpCache implements updateHeaders @@ -416,7 +425,7 @@ void CacheFilter::injectValidationHeaders(Http::RequestHeaderMap& request_header absl::string_view etag = etag_header->value().getStringView(); request_headers.setInline(if_none_match_handle.handle(), etag); } - if (CacheHeadersUtils::httpTime(last_modified_header) != SystemTime()) { + if (DateUtil::timePointValid(CacheHeadersUtils::httpTime(last_modified_header))) { // Valid Last-Modified header exists. absl::string_view last_modified = last_modified_header->value().getStringView(); request_headers.setInline(if_modified_since_handle.handle(), last_modified); @@ -435,8 +444,6 @@ void CacheFilter::encodeCachedResponse() { response_has_trailers_ = lookup_result_->has_trailers_; const bool end_stream = (lookup_result_->content_length_ == 0 && !response_has_trailers_); - // TODO(toddmgreer): Calculate age per https://httpwg.org/specs/rfc7234.html#age.calculations - lookup_result_->headers_->addReferenceKey(Http::Headers::get().Age, 0); // Set appropriate response flags and codes. Http::StreamFilterCallbacks* callbacks = diff --git a/source/extensions/filters/http/cache/cache_headers_utils.cc b/source/extensions/filters/http/cache/cache_headers_utils.cc index 11d45b6db97c9..7762c3091a5fb 100644 --- a/source/extensions/filters/http/cache/cache_headers_utils.cc +++ b/source/extensions/filters/http/cache/cache_headers_utils.cc @@ -1,9 +1,15 @@ #include "extensions/filters/http/cache/cache_headers_utils.h" #include +#include #include -#include "envoy/common/time.h" +#include "envoy/http/header_map.h" + +#include "common/http/header_map_impl.h" +#include "common/http/header_utility.h" + +#include "extensions/filters/http/cache/inline_headers_handles.h" #include "absl/algorithm/container.h" #include "absl/strings/ascii.h" @@ -28,7 +34,7 @@ OptionalDuration parseDuration(absl::string_view s) { long num; if (absl::SimpleAtoi(s, &num) && num >= 0) { // s is a valid string of digits representing a positive number. - duration = std::chrono::seconds(num); + duration = Seconds(num); } return duration; } @@ -143,6 +149,31 @@ SystemTime CacheHeadersUtils::httpTime(const Http::HeaderEntry* header_entry) { return {}; } +Seconds CacheHeadersUtils::calculateAge(const Http::ResponseHeaderMap& response_headers, + const SystemTime response_time, const SystemTime now) { + // Age headers calculations follow: https://httpwg.org/specs/rfc7234.html#age.calculations + const SystemTime date_value = CacheHeadersUtils::httpTime(response_headers.Date()); + + long age_value; + const absl::string_view age_header = response_headers.getInlineValue(age_handle.handle()); + if (!absl::SimpleAtoi(age_header, &age_value)) { + age_value = 0; + } + + const SystemTime::duration apparent_age = + std::max(SystemTime::duration(0), response_time - date_value); + + // Assumption: response_delay is negligible -> corrected_age_value = age_value. + const SystemTime::duration corrected_age_value = Seconds(age_value); + const SystemTime::duration corrected_initial_age = std::max(apparent_age, corrected_age_value); + + // Calculate current_age: + const SystemTime::duration resident_time = now - response_time; + const SystemTime::duration current_age = corrected_initial_age + resident_time; + + return std::chrono::duration_cast(current_age); +} + absl::optional CacheHeadersUtils::readAndRemoveLeadingDigits(absl::string_view& str) { uint64_t val = 0; uint32_t bytes_consumed = 0; diff --git a/source/extensions/filters/http/cache/cache_headers_utils.h b/source/extensions/filters/http/cache/cache_headers_utils.h index 1957858224bb5..e64752d846e45 100644 --- a/source/extensions/filters/http/cache/cache_headers_utils.h +++ b/source/extensions/filters/http/cache/cache_headers_utils.h @@ -1,10 +1,7 @@ #pragma once #include "envoy/common/time.h" -#include "envoy/http/header_map.h" -#include "common/http/header_map_impl.h" -#include "common/http/header_utility.h" #include "common/http/headers.h" #include "absl/strings/str_join.h" @@ -96,6 +93,10 @@ class CacheHeadersUtils { // header_entry is null or malformed. static SystemTime httpTime(const Http::HeaderEntry* header_entry); + // Calculates the age of a cached response + static Seconds calculateAge(const Http::ResponseHeaderMap& response_headers, + SystemTime response_time, SystemTime now); + /** * Read a leading positive decimal integer value and advance "*str" past the * digits read. If overflow occurs, or no digits exist, return diff --git a/source/extensions/filters/http/cache/cacheability_utils.cc b/source/extensions/filters/http/cache/cacheability_utils.cc index 7b9ae2eb0783c..48e02a04d20eb 100644 --- a/source/extensions/filters/http/cache/cacheability_utils.cc +++ b/source/extensions/filters/http/cache/cacheability_utils.cc @@ -62,14 +62,15 @@ bool CacheabilityUtils::isCacheableResponse( absl::string_view cache_control = headers.getInlineValue(response_cache_control_handle.handle()); ResponseCacheControl response_cache_control(cache_control); - // Only cache responses with explicit validation data, either: - // "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); + // Only cache responses with enough data to calculate freshness lifetime as per: + // https://httpwg.org/specs/rfc7234.html#calculating.freshness.lifetime. + // Either: + // "no-cache" cache-control directive (requires revalidation anyway). + // "max-age" or "s-maxage" cache-control directives. + // Both "Expires" and "Date" headers. + const bool has_validation_data = response_cache_control.must_validate_ || + response_cache_control.max_age_.has_value() || + (headers.Date() && headers.getInline(expires_handle.handle())); return !response_cache_control.no_store_ && cacheableStatusCodes().contains((headers.getStatusValue())) && has_validation_data && diff --git a/source/extensions/filters/http/cache/http_cache.cc b/source/extensions/filters/http/cache/http_cache.cc index a01c10b33f276..fb5934491f8c2 100644 --- a/source/extensions/filters/http/cache/http_cache.cc +++ b/source/extensions/filters/http/cache/http_cache.cc @@ -11,6 +11,7 @@ #include "common/http/headers.h" #include "common/protobuf/utility.h" +#include "extensions/filters/http/cache/cache_headers_utils.h" #include "extensions/filters/http/cache/inline_headers_handles.h" #include "absl/strings/str_split.h" @@ -77,21 +78,14 @@ void LookupRequest::initializeRequestCacheControl(const Http::RequestHeaderMap& } } -bool LookupRequest::requiresValidation(const Http::ResponseHeaderMap& response_headers) const { +bool LookupRequest::requiresValidation(const Http::ResponseHeaderMap& response_headers, + SystemTime::duration response_age) const { // TODO(yosrym93): Store parsed response cache-control in cache instead of parsing it on every // lookup. const absl::string_view cache_control = response_headers.getInlineValue(response_cache_control_handle.handle()); const ResponseCacheControl response_cache_control(cache_control); - const SystemTime response_time = CacheHeadersUtils::httpTime(response_headers.Date()); - - if (timestamp_ < response_time) { - // Response time is in the future, validate response. - return true; - } - - const SystemTime::duration response_age = timestamp_ - response_time; const bool request_max_age_exceeded = request_cache_control_.max_age_.has_value() && request_cache_control_.max_age_.value() < response_age; if (response_cache_control.must_validate_ || request_cache_control_.must_validate_ || @@ -102,40 +96,50 @@ bool LookupRequest::requiresValidation(const Http::ResponseHeaderMap& response_h } // CacheabilityUtils::isCacheableResponse(..) guarantees that any cached response satisfies this. - // When date metadata injection for responses with no date - // is implemented, this ASSERT will need to be updated. - ASSERT((response_headers.Date() && response_cache_control.max_age_.has_value()) || - response_headers.get(Http::Headers::get().Expires), + ASSERT(response_cache_control.max_age_.has_value() || + (response_headers.getInline(expires_handle.handle()) && response_headers.Date()), "Cache entry does not have valid expiration data."); - const SystemTime expiration_time = - response_cache_control.max_age_.has_value() - ? response_time + response_cache_control.max_age_.value() - : CacheHeadersUtils::httpTime(response_headers.get(Http::Headers::get().Expires)); + SystemTime::duration freshness_lifetime; + if (response_cache_control.max_age_.has_value()) { + freshness_lifetime = response_cache_control.max_age_.value(); + } else { + const SystemTime expires_value = + CacheHeadersUtils::httpTime(response_headers.getInline(expires_handle.handle())); + const SystemTime date_value = CacheHeadersUtils::httpTime(response_headers.Date()); + freshness_lifetime = expires_value - date_value; + } - if (timestamp_ > expiration_time) { + if (response_age > freshness_lifetime) { // Response is stale, requires validation if // the response does not allow being served stale, // or the request max-stale directive does not allow it. const bool allowed_by_max_stale = request_cache_control_.max_stale_.has_value() && - request_cache_control_.max_stale_.value() > timestamp_ - expiration_time; + request_cache_control_.max_stale_.value() > response_age - freshness_lifetime; return response_cache_control.no_stale_ || !allowed_by_max_stale; } else { // Response is fresh, requires validation only if there is an unsatisfied min-fresh requirement. const bool min_fresh_unsatisfied = request_cache_control_.min_fresh_.has_value() && - request_cache_control_.min_fresh_.value() > expiration_time - timestamp_; + request_cache_control_.min_fresh_.value() > freshness_lifetime - response_age; return min_fresh_unsatisfied; } } LookupResult LookupRequest::makeLookupResult(Http::ResponseHeaderMapPtr&& response_headers, + ResponseMetadata&& metadata, uint64_t content_length) const { // TODO(toddmgreer): Implement all HTTP caching semantics. ASSERT(response_headers); LookupResult result; - result.cache_entry_status_ = requiresValidation(*response_headers) + + // Assumption: Cache lookup time is negligible. Therefore, now == timestamp_ + const Seconds age = + CacheHeadersUtils::calculateAge(*response_headers, metadata.response_time_, timestamp_); + response_headers->setInline(age_handle.handle(), std::to_string(age.count())); + + result.cache_entry_status_ = requiresValidation(*response_headers, age) ? CacheEntryStatus::RequiresValidation : CacheEntryStatus::Ok; result.headers_ = std::move(response_headers); diff --git a/source/extensions/filters/http/cache/http_cache.h b/source/extensions/filters/http/cache/http_cache.h index 4ded95d183e8a..49afbf74bd4b5 100644 --- a/source/extensions/filters/http/cache/http_cache.h +++ b/source/extensions/filters/http/cache/http_cache.h @@ -170,6 +170,17 @@ using LookupResultPtr = std::unique_ptr; // TODO(toddmgreer): Ensure that stability guarantees above are accurate. size_t stableHashKey(const Key& key); +// The metadata associated with a cached response. +// TODO(yosrym93): This could be changed to a proto if a need arises. +// If a cache was created with the current interface, then it was changed to a proto, all the cache +// entries will need to be invalidated. +struct ResponseMetadata { + // The time at which a response was was most recently inserted, updated, or validated in this + // cache. This represents "response_time" in the age header calculations at: + // https://httpwg.org/specs/rfc7234.html#age.calculations + SystemTime response_time_; +}; + // LookupRequest holds everything about a request that's needed to look for a // response in a cache, to evaluate whether an entry from a cache is usable, and // to determine what ranges are needed. @@ -190,19 +201,20 @@ class LookupRequest { // LookupHeadersCallback. Specifically, // - LookupResult::cache_entry_status_ is set according to HTTP cache // validation logic. - // - LookupResult::headers takes ownership of response_headers. - // - LookupResult::content_length == content_length. - // - LookupResult::response_ranges entries are satisfiable (as documented + // - LookupResult::headers_ takes ownership of response_headers. + // - LookupResult::content_length_ == content_length. + // - LookupResult::response_ranges_ entries are satisfiable (as documented // there). LookupResult makeLookupResult(Http::ResponseHeaderMapPtr&& response_headers, - uint64_t content_length) const; + ResponseMetadata&& metadata, uint64_t content_length) const; // Warning: this should not be accessed out-of-thread! const Http::RequestHeaderMap& getVaryHeaders() const { return *vary_headers_; } private: void initializeRequestCacheControl(const Http::RequestHeaderMap& request_headers); - bool requiresValidation(const Http::ResponseHeaderMap& response_headers) const; + bool requiresValidation(const Http::ResponseHeaderMap& response_headers, + SystemTime::duration age) const; Key key_; std::vector request_range_spec_; @@ -233,7 +245,8 @@ using InsertCallback = std::function; class InsertContext { public: // Accepts response_headers for caching. Only called once. - virtual void insertHeaders(const Http::ResponseHeaderMap& response_headers, bool end_stream) PURE; + virtual void insertHeaders(const Http::ResponseHeaderMap& response_headers, + const ResponseMetadata& metadata, bool end_stream) PURE; // The insertion is streamed into the cache in chunks whose size is determined // by the client, but with a pace determined by the cache. To avoid streaming @@ -345,7 +358,8 @@ class HttpCache { // This is called when an expired cache entry is successfully validated, to // update the cache entry. virtual void updateHeaders(const LookupContext& lookup_context, - const Http::ResponseHeaderMap& response_headers) PURE; + const Http::ResponseHeaderMap& response_headers, + const ResponseMetadata& metadata) PURE; // Returns statically known information about a cache. virtual CacheInfo cacheInfo() const PURE; diff --git a/source/extensions/filters/http/cache/inline_headers_handles.h b/source/extensions/filters/http/cache/inline_headers_handles.h index c9ae8353a8e26..3b5318ffee610 100644 --- a/source/extensions/filters/http/cache/inline_headers_handles.h +++ b/source/extensions/filters/http/cache/inline_headers_handles.h @@ -1,5 +1,7 @@ #pragma once +#include "envoy/http/header_map.h" + #include "common/http/headers.h" namespace Envoy { @@ -7,6 +9,7 @@ namespace Extensions { namespace HttpFilters { namespace Cache { +// Request headers inline handles inline Http::RegisterCustomInlineHeader authorization_handle(Http::CustomHeaders::get().Authorization); @@ -41,6 +44,12 @@ inline Http::RegisterCustomInlineHeader etag_handle(Http::CustomHeaders::get().Etag); +inline Http::RegisterCustomInlineHeader + age_handle(Http::Headers::get().Age); + +inline Http::RegisterCustomInlineHeader + expires_handle(Http::Headers::get().Expires); + } // namespace Cache } // namespace HttpFilters } // namespace Extensions diff --git a/source/extensions/filters/http/cache/simple_http_cache/simple_http_cache.cc b/source/extensions/filters/http/cache/simple_http_cache/simple_http_cache.cc index 385c8295b2eed..0fea6205801c4 100644 --- a/source/extensions/filters/http/cache/simple_http_cache/simple_http_cache.cc +++ b/source/extensions/filters/http/cache/simple_http_cache/simple_http_cache.cc @@ -21,9 +21,9 @@ class SimpleLookupContext : public LookupContext { void getHeaders(LookupHeadersCallback&& cb) override { auto entry = cache_.lookup(request_); body_ = std::move(entry.body_); - cb(entry.response_headers_ - ? request_.makeLookupResult(std::move(entry.response_headers_), body_.size()) - : LookupResult{}); + cb(entry.response_headers_ ? request_.makeLookupResult(std::move(entry.response_headers_), + std::move(entry.metadata_), body_.size()) + : LookupResult{}); } void getBody(const AdjustedByteRange& range, LookupBodyCallback&& cb) override { @@ -53,9 +53,11 @@ class SimpleInsertContext : public InsertContext { dynamic_cast(lookup_context).request().getVaryHeaders()), cache_(cache) {} - void insertHeaders(const Http::ResponseHeaderMap& response_headers, bool end_stream) override { + void insertHeaders(const Http::ResponseHeaderMap& response_headers, + const ResponseMetadata& metadata, bool end_stream) override { ASSERT(!committed_); response_headers_ = Http::createHeaderMap(response_headers); + metadata_ = metadata; if (end_stream) { commit(); } @@ -84,14 +86,16 @@ class SimpleInsertContext : public InsertContext { void commit() { committed_ = true; if (VaryHeader::hasVary(*response_headers_)) { - cache_.varyInsert(key_, std::move(response_headers_), body_.toString(), entry_vary_headers_); + cache_.varyInsert(key_, std::move(response_headers_), std::move(metadata_), body_.toString(), + entry_vary_headers_); } else { - cache_.insert(key_, std::move(response_headers_), body_.toString()); + cache_.insert(key_, std::move(response_headers_), std::move(metadata_), body_.toString()); } } Key key_; Http::ResponseHeaderMapPtr response_headers_; + ResponseMetadata metadata_; const Http::RequestHeaderMap& entry_vary_headers_; SimpleHttpCache& cache_; Buffer::OwnedImpl body_; @@ -103,7 +107,8 @@ LookupContextPtr SimpleHttpCache::makeLookupContext(LookupRequest&& request) { return std::make_unique(*this, std::move(request)); } -void SimpleHttpCache::updateHeaders(const LookupContext&, const Http::ResponseHeaderMap&) { +void SimpleHttpCache::updateHeaders(const LookupContext&, const Http::ResponseHeaderMap&, + const ResponseMetadata&) { // TODO(toddmgreer): Support updating headers. // Not implemented yet, however this is called during tests // NOT_IMPLEMENTED_GCOVR_EXCL_LINE; @@ -122,14 +127,15 @@ SimpleHttpCache::Entry SimpleHttpCache::lookup(const LookupRequest& request) { } else { return SimpleHttpCache::Entry{ Http::createHeaderMap(*iter->second.response_headers_), - iter->second.body_}; + iter->second.metadata_, iter->second.body_}; } } void SimpleHttpCache::insert(const Key& key, Http::ResponseHeaderMapPtr&& response_headers, - std::string&& body) { + ResponseMetadata&& metadata, std::string&& body) { absl::WriterMutexLock lock(&mutex_); - map_[key] = SimpleHttpCache::Entry{std::move(response_headers), std::move(body)}; + map_[key] = + SimpleHttpCache::Entry{std::move(response_headers), std::move(metadata), std::move(body)}; } SimpleHttpCache::Entry @@ -153,11 +159,12 @@ SimpleHttpCache::varyLookup(const LookupRequest& request, return SimpleHttpCache::Entry{ Http::createHeaderMap(*iter->second.response_headers_), - iter->second.body_}; + iter->second.metadata_, iter->second.body_}; } void SimpleHttpCache::varyInsert(const Key& request_key, - Http::ResponseHeaderMapPtr&& response_headers, std::string&& body, + Http::ResponseHeaderMapPtr&& response_headers, + ResponseMetadata&& metadata, std::string&& body, const Http::RequestHeaderMap& request_vary_headers) { absl::WriterMutexLock lock(&mutex_); @@ -168,7 +175,8 @@ void SimpleHttpCache::varyInsert(const Key& request_key, Key varied_request_key = request_key; const std::string vary_key = VaryHeader::createVaryKey(vary_header, request_vary_headers); varied_request_key.add_custom_fields(vary_key); - map_[varied_request_key] = SimpleHttpCache::Entry{std::move(response_headers), std::move(body)}; + map_[varied_request_key] = + SimpleHttpCache::Entry{std::move(response_headers), std::move(metadata), std::move(body)}; // Add a special entry to flag that this request generates varied responses. auto iter = map_.find(request_key); @@ -181,7 +189,7 @@ void SimpleHttpCache::varyInsert(const Key& request_key, // have inserted for that resource. For the first entry simply use vary_key as the entry_list, // for future entries append vary_key to existing list. std::string entry_list; - map_[request_key] = SimpleHttpCache::Entry{std::move(vary_only_map), std::move(entry_list)}; + map_[request_key] = SimpleHttpCache::Entry{std::move(vary_only_map), {}, std::move(entry_list)}; } } diff --git a/source/extensions/filters/http/cache/simple_http_cache/simple_http_cache.h b/source/extensions/filters/http/cache/simple_http_cache/simple_http_cache.h index e8c93e1cc06a8..4c2c1a85d0e04 100644 --- a/source/extensions/filters/http/cache/simple_http_cache/simple_http_cache.h +++ b/source/extensions/filters/http/cache/simple_http_cache/simple_http_cache.h @@ -18,6 +18,7 @@ class SimpleHttpCache : public HttpCache { private: struct Entry { Http::ResponseHeaderMapPtr response_headers_; + ResponseMetadata metadata_; std::string body_; }; @@ -30,15 +31,18 @@ class SimpleHttpCache : public HttpCache { LookupContextPtr makeLookupContext(LookupRequest&& request) override; InsertContextPtr makeInsertContext(LookupContextPtr&& lookup_context) override; void updateHeaders(const LookupContext& lookup_context, - const Http::ResponseHeaderMap& response_headers) override; + const Http::ResponseHeaderMap& response_headers, + const ResponseMetadata& metadata) override; CacheInfo cacheInfo() const override; Entry lookup(const LookupRequest& request); - void insert(const Key& key, Http::ResponseHeaderMapPtr&& response_headers, std::string&& body); + void insert(const Key& key, Http::ResponseHeaderMapPtr&& response_headers, + ResponseMetadata&& metadata, std::string&& body); // Inserts a response that has been varied on certain headers. void varyInsert(const Key& request_key, Http::ResponseHeaderMapPtr&& response_headers, - std::string&& body, const Http::RequestHeaderMap& request_vary_headers); + ResponseMetadata&& metadata, std::string&& body, + const Http::RequestHeaderMap& request_vary_headers); absl::Mutex mutex_; absl::flat_hash_map map_ ABSL_GUARDED_BY(mutex_); diff --git a/test/extensions/filters/http/cache/BUILD b/test/extensions/filters/http/cache/BUILD index c17f76a3a2065..dfdfe16f0db18 100644 --- a/test/extensions/filters/http/cache/BUILD +++ b/test/extensions/filters/http/cache/BUILD @@ -27,6 +27,7 @@ envoy_extension_cc_test( "//include/envoy/http:header_map_interface", "//source/common/http:header_map_lib", "//source/extensions/filters/http/cache:cache_headers_utils_lib", + "//test/test_common:simulated_time_system_lib", "//test/test_common:utility_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 fde9cd63fe4c2..7630e0c44fdb7 100644 --- a/test/extensions/filters/http/cache/cache_filter_integration_test.cc +++ b/test/extensions/filters/http/cache/cache_filter_integration_test.cc @@ -78,7 +78,7 @@ TEST_P(CacheIntegrationTest, MissInsertHit) { } // Advance time, to verify the original date header is preserved. - simTime().advanceTimeWait(std::chrono::seconds(10)); + simTime().advanceTimeWait(Seconds(10)); // Send second request, and get response from cache. { @@ -88,9 +88,9 @@ TEST_P(CacheIntegrationTest, MissInsertHit) { 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); + EXPECT_THAT(response_decoder->headers(), HeaderHasValueRef(Http::Headers::get().Age, "10")); // Advance time to force a log flush. - simTime().advanceTimeWait(std::chrono::seconds(1)); + simTime().advanceTimeWait(Seconds(1)); EXPECT_THAT(waitForAccessLog(access_log_name_, 1), testing::HasSubstr("RFCF cache.response_from_cache_filter")); } @@ -136,7 +136,7 @@ TEST_P(CacheIntegrationTest, ExpiredValidated) { // 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)); + simTime().advanceTimeWait(Seconds(11)); // Send second request, the cached response should be validate then served { @@ -164,11 +164,14 @@ TEST_P(CacheIntegrationTest, ExpiredValidated) { 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); + + // A response that has been validated should not contain an Age header as it is equivalent to a + // freshly served response from the origin, unless the 304 response has an Age header, which + // means it was served by an upstream cache. + EXPECT_EQ(response_decoder->headers().get(Http::Headers::get().Age), nullptr); // Advance time to force a log flush. - simTime().advanceTimeWait(std::chrono::seconds(1)); + simTime().advanceTimeWait(Seconds(1)); EXPECT_THAT(waitForAccessLog(access_log_name_, 1), testing::HasSubstr("RFCF cache.response_from_cache_filter")); } @@ -214,7 +217,7 @@ TEST_P(CacheIntegrationTest, ExpiredFetchedNewResponse) { // 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)); + simTime().advanceTimeWait(Seconds(11)); // Send second request, validation of the cached response should be attempted but should fail // The new response should be served @@ -249,7 +252,7 @@ TEST_P(CacheIntegrationTest, ExpiredFetchedNewResponse) { EXPECT_EQ(response_decoder->headers().get(Http::Headers::get().Age), nullptr); // Advance time to force a log flush. - simTime().advanceTimeWait(std::chrono::seconds(1)); + simTime().advanceTimeWait(Seconds(1)); EXPECT_THAT(waitForAccessLog(access_log_name_), testing::HasSubstr("- via_upstream")); } } diff --git a/test/extensions/filters/http/cache/cache_filter_test.cc b/test/extensions/filters/http/cache/cache_filter_test.cc index f6060049a9ea2..e74890235b1ed 100644 --- a/test/extensions/filters/http/cache/cache_filter_test.cc +++ b/test/extensions/filters/http/cache/cache_filter_test.cc @@ -1,5 +1,4 @@ #include "envoy/event/dispatcher.h" -#include "envoy/http/header_map.h" #include "common/http/headers.h" @@ -33,6 +32,10 @@ class CacheFilterTest : public ::testing::Test { void SetUp() override { ON_CALL(decoder_callbacks_, dispatcher()).WillByDefault(::testing::ReturnRef(*dispatcher_)); + // Initialize the time source (otherwise it returns the real time) + time_source_.setSystemTime(std::chrono::hours(1)); + // Use the initialized time source to set the response date header + response_headers_.setDate(formatter_.now(time_source_)); } void testDecodeRequestMiss(CacheFilterSharedPtr filter) { @@ -59,7 +62,7 @@ class CacheFilterTest : public ::testing::Test { // The filter should encode cached headers. EXPECT_CALL(decoder_callbacks_, encodeHeaders_(testing::AllOf(IsSupersetOfHeaders(response_headers_), - HeaderHasValueRef("age", "0")), + HeaderHasValueRef(Http::Headers::get().Age, age)), true)); // The filter should not encode any data as the response has no body. @@ -85,7 +88,7 @@ class CacheFilterTest : public ::testing::Test { // The filter should encode cached headers. EXPECT_CALL(decoder_callbacks_, encodeHeaders_(testing::AllOf(IsSupersetOfHeaders(response_headers_), - HeaderHasValueRef("age", "0")), + HeaderHasValueRef(Http::Headers::get().Age, age)), false)); // The filter should encode cached data. @@ -111,6 +114,8 @@ class CacheFilterTest : public ::testing::Test { ::testing::Mock::VerifyAndClearExpectations(&decoder_callbacks_); } + void waitBeforeSecondRequest() { time_source_.advanceTimeWait(delay_); } + SimpleHttpCache simple_cache_; envoy::extensions::filters::http::cache::v3alpha::CacheConfig config_; NiceMock context_; @@ -119,12 +124,13 @@ class CacheFilterTest : public ::testing::Test { Http::TestRequestHeaderMapImpl request_headers_{ {":path", "/"}, {":method", "GET"}, {"x-forwarded-proto", "https"}}; Http::TestResponseHeaderMapImpl response_headers_{{":status", "200"}, - {"date", formatter_.now(time_source_)}, {"cache-control", "public,max-age=3600"}}; NiceMock decoder_callbacks_; NiceMock encoder_callbacks_; Api::ApiPtr api_ = Api::createApiForTest(); Event::DispatcherPtr dispatcher_ = api_->allocateDispatcher("test_thread"); + const Seconds delay_ = Seconds(10); + const std::string age = std::to_string(delay_.count()); }; TEST_F(CacheFilterTest, UncacheableRequest) { @@ -199,6 +205,7 @@ TEST_F(CacheFilterTest, CacheHitNoBody) { EXPECT_EQ(filter->encodeHeaders(response_headers_, true), Http::FilterHeadersStatus::Continue); filter->onDestroy(); } + waitBeforeSecondRequest(); { // Create filter for request 2. CacheFilterSharedPtr filter = makeFilter(simple_cache_); @@ -227,6 +234,7 @@ TEST_F(CacheFilterTest, CacheHitWithBody) { filter->onDestroy(); } + waitBeforeSecondRequest(); { // Create filter for request 2 CacheFilterSharedPtr filter = makeFilter(simple_cache_); @@ -240,7 +248,8 @@ TEST_F(CacheFilterTest, CacheHitWithBody) { TEST_F(CacheFilterTest, SuccessfulValidation) { request_headers_.setHost("SuccessfulValidation"); const std::string body = "abc"; - + const std::string etag = "abc123"; + const std::string last_modified_date = formatter_.now(time_source_); { // Create filter for request 1 CacheFilterSharedPtr filter = makeFilter(simple_cache_); @@ -249,9 +258,8 @@ TEST_F(CacheFilterTest, SuccessfulValidation) { // 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_)); + response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, etag); + response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, last_modified_date); Buffer::OwnedImpl buffer(body); response_headers_.setContentLength(body.size()); @@ -259,6 +267,7 @@ TEST_F(CacheFilterTest, SuccessfulValidation) { EXPECT_EQ(filter->encodeData(buffer, true), Http::FilterDataStatus::Continue); filter->onDestroy(); } + waitBeforeSecondRequest(); { // Create filter for request 2 CacheFilterSharedPtr filter = makeFilter(simple_cache_); @@ -273,12 +282,11 @@ TEST_F(CacheFilterTest, SuccessfulValidation) { // Make sure validation conditional headers are added const Http::TestRequestHeaderMapImpl injected_headers = { - {"if-none-match", "abc123"}, {"if-modified-since", formatter_.now(time_source_)}}; + {"if-none-match", etag}, {"if-modified-since", last_modified_date}}; EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); // 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}}; @@ -292,9 +300,7 @@ TEST_F(CacheFilterTest, SuccessfulValidation) { // 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"))); + EXPECT_THAT(not_modified_response_headers, IsSupersetOfHeaders(updated_response_headers)); // 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 @@ -322,18 +328,18 @@ TEST_F(CacheFilterTest, SuccessfulValidation) { TEST_F(CacheFilterTest, UnsuccessfulValidation) { request_headers_.setHost("UnsuccessfulValidation"); const std::string body = "abc"; - + const std::string etag = "abc123"; + const std::string last_modified_date = formatter_.now(time_source_); { // Create filter for request 1 CacheFilterSharedPtr filter = makeFilter(simple_cache_); testDecodeRequestMiss(filter); - // Encode response. + // 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_)); + response_headers_.setReferenceKey(Http::CustomHeaders::get().Etag, etag); + response_headers_.setReferenceKey(Http::CustomHeaders::get().LastModified, last_modified_date); Buffer::OwnedImpl buffer(body); response_headers_.setContentLength(body.size()); @@ -341,6 +347,7 @@ TEST_F(CacheFilterTest, UnsuccessfulValidation) { EXPECT_EQ(filter->encodeData(buffer, true), Http::FilterDataStatus::Continue); filter->onDestroy(); } + waitBeforeSecondRequest(); { // Create filter for request 2. CacheFilterSharedPtr filter = makeFilter(simple_cache_); @@ -355,7 +362,7 @@ TEST_F(CacheFilterTest, UnsuccessfulValidation) { // Make sure validation conditional headers are added. const Http::TestRequestHeaderMapImpl injected_headers = { - {"if-none-match", "abc123"}, {"if-modified-since", formatter_.now(time_source_)}}; + {"if-none-match", etag}, {"if-modified-since", last_modified_date}}; EXPECT_THAT(request_headers_, IsSupersetOfHeaders(injected_headers)); // Encode new response. @@ -368,7 +375,7 @@ TEST_F(CacheFilterTest, UnsuccessfulValidation) { EXPECT_EQ(filter->encodeData(new_body, true), Http::FilterDataStatus::Continue); // The response headers should have the new status. - EXPECT_THAT(response_headers_, HeaderHasValueRef(":status", "201")); + EXPECT_THAT(response_headers_, HeaderHasValueRef(Http::Headers::get().Status, "201")); // The filter should not encode any data. EXPECT_CALL(encoder_callbacks_, addEncodedData).Times(0); @@ -400,6 +407,7 @@ TEST_F(CacheFilterTest, SingleSatisfiableRange) { EXPECT_EQ(filter->encodeData(buffer, true), Http::FilterDataStatus::Continue); filter->onDestroy(); } + waitBeforeSecondRequest(); { // Add range info to headers. request_headers_.addReference(Http::Headers::get().Range, "bytes=-2"); @@ -414,7 +422,7 @@ TEST_F(CacheFilterTest, SingleSatisfiableRange) { // Decode request 2 header EXPECT_CALL(decoder_callbacks_, encodeHeaders_(testing::AllOf(IsSupersetOfHeaders(response_headers_), - HeaderHasValueRef("age", "0")), + HeaderHasValueRef(Http::Headers::get().Age, age)), false)); EXPECT_CALL( @@ -451,6 +459,7 @@ TEST_F(CacheFilterTest, MultipleSatisfiableRanges) { EXPECT_EQ(filter->encodeData(buffer, true), Http::FilterDataStatus::Continue); filter->onDestroy(); } + waitBeforeSecondRequest(); { // Add range info to headers // multi-part responses are not supported, 200 expected @@ -462,7 +471,7 @@ TEST_F(CacheFilterTest, MultipleSatisfiableRanges) { // Decode request 2 header EXPECT_CALL(decoder_callbacks_, encodeHeaders_(testing::AllOf(IsSupersetOfHeaders(response_headers_), - HeaderHasValueRef("age", "0")), + HeaderHasValueRef(Http::Headers::get().Age, age)), false)); EXPECT_CALL( @@ -499,6 +508,7 @@ TEST_F(CacheFilterTest, NotSatisfiableRange) { EXPECT_EQ(filter->encodeData(buffer, true), Http::FilterDataStatus::Continue); filter->onDestroy(); } + waitBeforeSecondRequest(); { // Add range info to headers request_headers_.addReference(Http::Headers::get().Range, "bytes=123-"); @@ -513,7 +523,7 @@ TEST_F(CacheFilterTest, NotSatisfiableRange) { // Decode request 2 header EXPECT_CALL(decoder_callbacks_, encodeHeaders_(testing::AllOf(IsSupersetOfHeaders(response_headers_), - HeaderHasValueRef("age", "0")), + HeaderHasValueRef(Http::Headers::get().Age, age)), true)); // 416 response should not have a body, so we don't expect a call to encodeData 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 e96b3362dac6b..8e963fad01eba 100644 --- a/test/extensions/filters/http/cache/cache_headers_utils_test.cc +++ b/test/extensions/filters/http/cache/cache_headers_utils_test.cc @@ -5,11 +5,14 @@ #include "envoy/common/time.h" #include "common/common/macros.h" +#include "common/common/utility.h" #include "common/http/header_map_impl.h" +#include "common/http/header_utility.h" #include "extensions/filters/http/cache/cache_headers_utils.h" #include "test/extensions/filters/http/cache/common.h" +#include "test/test_common/simulated_time_system.h" #include "test/test_common/utility.h" #include "gtest/gtest.h" @@ -34,28 +37,11 @@ struct TestRequestCacheControl : public RequestCacheControl { } }; -struct TestResponseCacheControl : public ResponseCacheControl { - TestResponseCacheControl(bool must_validate, bool no_store, bool no_transform, bool no_stale, - bool is_public, OptionalDuration max_age) { - must_validate_ = must_validate; - no_store_ = no_store; - no_transform_ = no_transform; - no_stale_ = no_stale; - is_public_ = is_public; - max_age_ = max_age; - } -}; - struct RequestCacheControlTestCase { absl::string_view cache_control_header; TestRequestCacheControl request_cache_control; }; -struct ResponseCacheControlTestCase { - absl::string_view cache_control_header; - TestResponseCacheControl response_cache_control; -}; - class RequestCacheControlTest : public testing::TestWithParam { public: static const std::vector& getTestCases() { @@ -71,55 +57,55 @@ class RequestCacheControlTest : public testing::TestWithParam { public: static const std::vector& getTestCases() { @@ -165,17 +177,17 @@ class ResponseCacheControlTest : public testing::TestWithParam { public: static const std::vector& getOkTestCases() { @@ -283,24 +303,6 @@ class HttpTimeTest : public testing::TestWithParam { } }; -INSTANTIATE_TEST_SUITE_P(RequestCacheControlTest, RequestCacheControlTest, - testing::ValuesIn(RequestCacheControlTest::getTestCases())); - -TEST_P(RequestCacheControlTest, RequestCacheControlTest) { - const absl::string_view cache_control_header = GetParam().cache_control_header; - const RequestCacheControl expected_request_cache_control = GetParam().request_cache_control; - EXPECT_EQ(expected_request_cache_control, RequestCacheControl(cache_control_header)); -} - -INSTANTIATE_TEST_SUITE_P(ResponseCacheControlTest, ResponseCacheControlTest, - testing::ValuesIn(ResponseCacheControlTest::getTestCases())); - -TEST_P(ResponseCacheControlTest, ResponseCacheControlTest) { - const absl::string_view cache_control_header = GetParam().cache_control_header; - const ResponseCacheControl expected_response_cache_control = GetParam().response_cache_control; - EXPECT_EQ(expected_response_cache_control, ResponseCacheControl(cache_control_header)); -} - INSTANTIATE_TEST_SUITE_P(Ok, HttpTimeTest, testing::ValuesIn(HttpTimeTest::getOkTestCases())); TEST_P(HttpTimeTest, OkFormats) { @@ -318,6 +320,102 @@ TEST(HttpTime, InvalidFormat) { TEST(HttpTime, Null) { EXPECT_EQ(CacheHeadersUtils::httpTime(nullptr), SystemTime()); } +struct CalculateAgeTestCase { + std::string test_name; + Http::TestResponseHeaderMapImpl response_headers; + SystemTime response_time, now; + Seconds expected_age; +}; + +class CalculateAgeTest : public testing::TestWithParam { +public: + static std::string durationToString(const SystemTime::duration& duration) { + return std::to_string(duration.count()); + } + static std::string formatTime(const SystemTime& time) { return formatter().fromTime(time); } + static const DateFormatter& formatter() { + CONSTRUCT_ON_FIRST_USE(DateFormatter, {"%a, %d %b %Y %H:%M:%S GMT"}); + } + static const SystemTime& currentTime() { + CONSTRUCT_ON_FIRST_USE(SystemTime, Event::SimulatedTimeSystem().systemTime()); + } + static const std::vector& getTestCases() { + // clang-format off + CONSTRUCT_ON_FIRST_USE(std::vector, + { + "no_initial_age_all_times_equal", + /*response_headers=*/{{"date", formatTime(currentTime())}}, + /*response_time=*/currentTime(), + /*now=*/currentTime(), + /*expected_age=*/Seconds(0) + }, + { + "initial_age_zero_all_times_equal", + /*response_headers=*/{{"date", formatTime(currentTime())}, {"age", "0"}}, + /*response_time=*/currentTime(), + /*now=*/currentTime(), + /*expected_age=*/Seconds(0) + }, + { + "initial_age_non_zero_all_times_equal", + /*response_headers=*/{{"date", formatTime(currentTime())}, {"age", "50"}}, + /*response_time=*/currentTime(), + /*now=*/currentTime(), + /*expected_age=*/Seconds(50) + }, + { + "date_after_response_time_no_initial_age", + /*response_headers=*/{{"date", formatTime(currentTime() + Seconds(5))}}, + /*response_time=*/currentTime(), + /*now=*/currentTime() + Seconds(10), + /*expected_age=*/Seconds(10) + }, + { + "date_after_response_time_with_initial_age", + /*response_headers=*/{{"date", formatTime(currentTime() + Seconds(10))}, {"age", "5"}}, + /*response_time=*/currentTime(), + /*now=*/currentTime() + Seconds(10), + /*expected_age=*/Seconds(15) + }, + { + "apparent_age_equals_initial_age", + /*response_headers=*/{{"date", formatTime(currentTime())}, {"age", "1"}}, + /*response_time=*/currentTime() + Seconds(1), + /*now=*/currentTime() + Seconds(5), + /*expected_age=*/Seconds(5) + }, + { + "apparent_age_lower_than_initial_age", + /*response_headers=*/{{"date", formatTime(currentTime())}, {"age", "3"}}, + /*response_time=*/currentTime() + Seconds(1), + /*now=*/currentTime() + Seconds(5), + /*expected_age=*/Seconds(7) + }, + { + "apparent_age_higher_than_initial_age", + /*response_headers=*/{{"date", formatTime(currentTime())}, {"age", "1"}}, + /*response_time=*/currentTime() + Seconds(3), + /*now=*/currentTime() + Seconds(5), + /*expected_age=*/Seconds(5) + }, + ); + // clang-format on + } +}; + +INSTANTIATE_TEST_SUITE_P(CalculateAgeTest, CalculateAgeTest, + testing::ValuesIn(CalculateAgeTest::getTestCases()), + [](const auto& info) { return info.param.test_name; }); + +TEST_P(CalculateAgeTest, CalculateAgeTest) { + const Seconds calculated_age = CacheHeadersUtils::calculateAge( + GetParam().response_headers, GetParam().response_time, GetParam().now); + const Seconds expected_age = GetParam().expected_age; + EXPECT_EQ(calculated_age, expected_age) + << "Expected age: " << durationToString(expected_age) + << ", Calculated age: " << durationToString(calculated_age); +} + void testReadAndRemoveLeadingDigits(absl::string_view input, int64_t expected, absl::string_view remaining) { absl::string_view test_input(input); diff --git a/test/extensions/filters/http/cache/cacheability_utils_test.cc b/test/extensions/filters/http/cache/cacheability_utils_test.cc index 8db55691ead97..44deb3a9cb931 100644 --- a/test/extensions/filters/http/cache/cacheability_utils_test.cc +++ b/test/extensions/filters/http/cache/cacheability_utils_test.cc @@ -120,13 +120,11 @@ TEST_F(IsCacheableResponseTest, ValidationData) { // Max-age data available response_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "s-maxage=1000"); EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers_, allowed_vary_headers_)); - // Max-age data available with no date - response_headers_.removeDate(); - EXPECT_FALSE(CacheabilityUtils::isCacheableResponse(response_headers_, allowed_vary_headers_)); - // No date, but the response requires revalidation anyway + // No max-age data, but the response requires revalidation anyway response_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache"); EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers_, allowed_vary_headers_)); - // No cache control headers or date, but there is an expires header + // No cache control headers, but there is an expires header + response_headers_.remove(Http::CustomHeaders::get().CacheControl); response_headers_.setReferenceKey(Http::Headers::get().Expires, "Sun, 06 Nov 1994 09:49:37 GMT"); EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(response_headers_, allowed_vary_headers_)); } diff --git a/test/extensions/filters/http/cache/http_cache_test.cc b/test/extensions/filters/http/cache/http_cache_test.cc index f427e21881851..88794cce6b9ea 100644 --- a/test/extensions/filters/http/cache/http_cache_test.cc +++ b/test/extensions/filters/http/cache/http_cache_test.cc @@ -1,7 +1,9 @@ #include +#include #include "extensions/filters/http/cache/cache_headers_utils.h" #include "extensions/filters/http/cache/http_cache.h" +#include "extensions/filters/http/cache/inline_headers_handles.h" #include "test/extensions/filters/http/cache/common.h" #include "test/mocks/http/mocks.h" @@ -22,12 +24,11 @@ namespace { struct LookupRequestTestCase { std::string test_name, request_cache_control, response_cache_control; - SystemTime request_time, response_time; + SystemTime request_time, response_date; CacheEntryStatus expected_cache_entry_status; + std::string expected_age; }; -using Seconds = std::chrono::seconds; - class LookupRequestTest : public testing::TestWithParam { public: DateFormatter formatter_{"%a, %d %b %Y %H:%M:%S GMT"}; @@ -49,104 +50,114 @@ class LookupRequestTest : public testing::TestWithParam { /*request_cache_control=*/"no-cache", /*response_cache_control=*/"public, max-age=3600", /*request_time=*/currentTime(), - /*response_time=*/currentTime(), - /*expected_result=*/CacheEntryStatus::RequiresValidation}, + /*response_date=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation, + /*expected_age=*/"0"}, {"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}, + /*response_date=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation, + /*expected_age=*/"0"}, {"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}, + /*response_date=*/currentTime(), + /*expected_result=*/CacheEntryStatus::Ok, + /*expected_age=*/"9"}, {"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}, + /*response_date=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation, + /*expected_age=*/"11"}, {"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}, + /*response_date=*/currentTime(), + /*expected_result=*/CacheEntryStatus::Ok, + /*expected_age=*/"999"}, {"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}, + /*response_date=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation, + /*expected_age=*/"1001"}, {"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}, + /*response_date=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation, + /*expected_age=*/"1001"}, {"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}, + /*response_date=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation, + /*expected_age=*/"1401"}, {"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}, + /*response_date=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation, + /*expected_age=*/"1001"}, {"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}, + /*response_date=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation, + /*expected_age=*/"1201"}, {"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}, + /*response_date=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation, + /*expected_age=*/"501"}, {"expired", /*request_cache_control=*/"", /*response_cache_control=*/"public, max-age=1000", /*request_time=*/currentTime() + Seconds(1001), - /*response_time=*/currentTime(), - /*expected_result=*/CacheEntryStatus::RequiresValidation}, + /*response_date=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation, + /*expected_age=*/"1001"}, {"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}, + /*response_date=*/currentTime(), + /*expected_result=*/CacheEntryStatus::Ok, + /*expected_age=*/"1499"}, {"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}, + /*response_date=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation, + /*expected_age=*/"1501"}, {"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}, + /*response_date=*/currentTime(), + /*expected_result=*/CacheEntryStatus::RequiresValidation, + /*expected_age=*/"1499"}, {"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}, + /*response_date=*/currentTime(), + /*expected_result=*/CacheEntryStatus::Ok, + /*expected_age=*/"999"}, ); } @@ -155,8 +166,11 @@ class LookupRequestTest : public testing::TestWithParam { LookupResult makeLookupResult(const LookupRequest& lookup_request, const Http::TestResponseHeaderMapImpl& response_headers, uint64_t content_length = 0) { + // For the purpose of the test, set the response_time to the date header value. + ResponseMetadata metadata = {CacheHeadersUtils::httpTime(response_headers.Date())}; return lookup_request.makeLookupResult( - std::make_unique(response_headers), content_length); + std::make_unique(response_headers), std::move(metadata), + content_length); } INSTANTIATE_TEST_SUITE_P(ResultMatchesExpectation, LookupRequestTest, @@ -166,16 +180,18 @@ INSTANTIATE_TEST_SUITE_P(ResultMatchesExpectation, LookupRequestTest, 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 SystemTime request_time = GetParam().request_time, response_date = GetParam().response_date; const LookupRequest lookup_request(request_headers_, request_time, allowed_vary_headers_); const Http::TestResponseHeaderMapImpl response_headers( {{"cache-control", GetParam().response_cache_control}, - {"date", formatter_.fromTime(response_time)}}); + {"date", formatter_.fromTime(response_date)}}); const LookupResult lookup_response = makeLookupResult(lookup_request, response_headers); 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_THAT(*lookup_response.headers_, + HeaderHasValueRef(Http::Headers::get().Age, GetParam().expected_age)); EXPECT_EQ(lookup_response.content_length_, 0); EXPECT_TRUE(lookup_response.response_ranges_.empty()); EXPECT_FALSE(lookup_response.has_trailers_); @@ -184,11 +200,11 @@ TEST_P(LookupRequestTest, ResultWithoutBodyMatchesExpectation) { 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 SystemTime request_time = GetParam().request_time, response_date = GetParam().response_date; const LookupRequest lookup_request(request_headers_, request_time, allowed_vary_headers_); const Http::TestResponseHeaderMapImpl response_headers( {{"cache-control", GetParam().response_cache_control}, - {"date", formatter_.fromTime(response_time)}}); + {"date", formatter_.fromTime(response_date)}}); const uint64_t content_length = 5; const LookupResult lookup_response = makeLookupResult(lookup_request, response_headers, content_length); @@ -196,6 +212,8 @@ TEST_P(LookupRequestTest, ResultWithBodyMatchesExpectation) { 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_THAT(*lookup_response.headers_, + HeaderHasValueRef(Http::Headers::get().Age, GetParam().expected_age)); EXPECT_EQ(lookup_response.content_length_, content_length); EXPECT_TRUE(lookup_response.response_ranges_.empty()); EXPECT_FALSE(lookup_response.has_trailers_); @@ -246,7 +264,7 @@ TEST_F(LookupRequestTest, PragmaNoCacheFallbackExtraDirectivesIgnored) { TEST_F(LookupRequestTest, PragmaFallbackOtherValuesIgnored) { request_headers_.setReferenceKey(Http::CustomHeaders::get().Pragma, "max-age=0"); - const LookupRequest lookup_request(request_headers_, currentTime() + std::chrono::seconds(5), + const LookupRequest lookup_request(request_headers_, currentTime() + Seconds(5), allowed_vary_headers_); const Http::TestResponseHeaderMapImpl response_headers( {{"date", formatter_.fromTime(currentTime())}, {"cache-control", "public, max-age=3600"}}); @@ -258,7 +276,7 @@ TEST_F(LookupRequestTest, PragmaFallbackOtherValuesIgnored) { TEST_F(LookupRequestTest, PragmaNoFallback) { 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 LookupRequest lookup_request(request_headers_, currentTime() + Seconds(5), allowed_vary_headers_); const Http::TestResponseHeaderMapImpl response_headers( {{"date", formatter_.fromTime(currentTime())}, {"cache-control", "public, max-age=3600"}}); 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 242fa13a2f9c2..3d2ecacc2ebf8 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 @@ -41,7 +41,8 @@ class SimpleHttpCacheTest : public testing::Test { void insert(LookupContextPtr lookup, const Http::TestResponseHeaderMapImpl& response_headers, const absl::string_view response_body) { InsertContextPtr inserter = cache_.makeInsertContext(move(lookup)); - inserter->insertHeaders(response_headers, false); + const ResponseMetadata metadata = {current_time_}; + inserter->insertHeaders(response_headers, metadata, false); inserter->insertBody(Buffer::OwnedImpl(response_body), nullptr, true); } @@ -148,7 +149,7 @@ TEST_F(SimpleHttpCacheTest, Fresh) { {"date", formatter_.fromTime(current_time_)}, {"cache-control", "public, max-age=3600"}}; // TODO(toddmgreer): Test with various date headers. insert("/", response_headers, ""); - time_source_.advanceTimeWait(std::chrono::seconds(3600)); + time_source_.advanceTimeWait(Seconds(3600)); lookup("/"); EXPECT_EQ(CacheEntryStatus::Ok, lookup_result_.cache_entry_status_); } @@ -158,7 +159,7 @@ TEST_F(SimpleHttpCacheTest, Stale) { {"date", formatter_.fromTime(current_time_)}, {"cache-control", "public, max-age=3600"}}; // TODO(toddmgreer): Test with various date headers. insert("/", response_headers, ""); - time_source_.advanceTimeWait(std::chrono::seconds(3601)); + time_source_.advanceTimeWait(Seconds(3601)); lookup("/"); EXPECT_EQ(CacheEntryStatus::Ok, lookup_result_.cache_entry_status_); } @@ -198,7 +199,8 @@ TEST_F(SimpleHttpCacheTest, StreamingPut) { {"age", "2"}, {"cache-control", "public, max-age=3600"}}; InsertContextPtr inserter = cache_.makeInsertContext(lookup("request_path")); - inserter->insertHeaders(response_headers, false); + const ResponseMetadata metadata = {current_time_}; + inserter->insertHeaders(response_headers, metadata, false); inserter->insertBody( Buffer::OwnedImpl("Hello, "), [](bool ready) { EXPECT_TRUE(ready); }, false); inserter->insertBody(Buffer::OwnedImpl("World!"), nullptr, true);