caching: Correctly calculate cached responses age

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<std::chrono::system_clock>;
 using MonotonicTime = std::chrono::time_point<std::chrono::steady_clock>;
 

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",
     ],
 )
 

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 =

source/extensions/filters/http/cache/cache_headers_utils.cc

@@ -1,9 +1,15 @@
 #include "extensions/filters/http/cache/cache_headers_utils.h"
 
 #include <array>
+#include <chrono>
 #include <string>
 
-#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<Seconds>(current_age);
+}
+
 absl::optional<uint64_t> CacheHeadersUtils::readAndRemoveLeadingDigits(absl::string_view& str) {
   uint64_t val = 0;
   uint32_t bytes_consumed = 0;

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

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 &&

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);

source/extensions/filters/http/cache/http_cache.h

@@ -170,6 +170,17 @@ using LookupResultPtr = std::unique_ptr<LookupResult>;
 // 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<RawByteRange> request_range_spec_;
@@ -233,7 +245,8 @@ using InsertCallback = std::function<void(bool success_ready_for_more)>;
 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;

source/extensions/filters/http/cache/inline_headers_handles.h

@@ -1,12 +1,15 @@
 #pragma once
 
+#include "envoy/http/header_map.h"
+
 #include "common/http/headers.h"
 
 namespace Envoy {
 namespace Extensions {
 namespace HttpFilters {
 namespace Cache {
 
+// Request headers inline handles
 inline Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::RequestHeaders>
     authorization_handle(Http::CustomHeaders::get().Authorization);
 
@@ -41,6 +44,12 @@ inline Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::
 inline Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::ResponseHeaders>
     etag_handle(Http::CustomHeaders::get().Etag);
 
+inline Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::ResponseHeaders>
+    age_handle(Http::Headers::get().Age);
+
+inline Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::ResponseHeaders>
+    expires_handle(Http::Headers::get().Expires);
+
 } // namespace Cache
 } // namespace HttpFilters
 } // namespace Extensions