[cache filter] Add option to ignore cache-control directives from req…

…uest (#34090)

Signed-off-by: Raven Black <ravenblack@dropbox.com>

api/envoy/extensions/filters/http/cache/v3/cache.proto

@@ -20,7 +20,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // [#protodoc-title: HTTP Cache Filter]
 
 // [#extension: envoy.filters.http.cache]
-// [#next-free-field: 6]
+// [#next-free-field: 7]
 message CacheConfig {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.config.filter.http.cache.v2alpha.CacheConfig";
@@ -88,4 +88,9 @@ message CacheConfig {
   // Max body size the cache filter will insert into a cache. 0 means unlimited (though the cache
   // storage implementation may have its own limit beyond which it will reject insertions).
   uint32 max_body_bytes = 4;
+
+  // By default, a ``cache-control: no-cache`` or ``pragma: no-cache`` header in the request
+  // causes the cache to validate with its upstream even if the lookup is a hit. Setting this
+  // to true will ignore these headers.
+  bool ignore_request_cache_control_header = 6;
 }

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

@@ -41,12 +41,15 @@ struct CacheResponseCodeDetailValues {
 
 using CacheResponseCodeDetails = ConstSingleton<CacheResponseCodeDetailValues>;
 
-CacheFilter::CacheFilter(const envoy::extensions::filters::http::cache::v3::CacheConfig& config,
-                         const std::string&, Stats::Scope&,
-                         Server::Configuration::CommonFactoryContext& context,
+CacheFilterConfig::CacheFilterConfig(
+    const envoy::extensions::filters::http::cache::v3::CacheConfig& config,
+    Server::Configuration::CommonFactoryContext& context)
+    : vary_allow_list_(config.allowed_vary_headers(), context), time_source_(context.timeSource()),
+      ignore_request_cache_control_header_(config.ignore_request_cache_control_header()) {}
+
+CacheFilter::CacheFilter(std::shared_ptr<const CacheFilterConfig> config,
                          std::shared_ptr<HttpCache> http_cache)
-    : time_source_(context.timeSource()), cache_(http_cache),
-      vary_allow_list_(config.allowed_vary_headers(), context) {}
+    : cache_(http_cache), config_(config) {}
 
 void CacheFilter::onDestroy() {
   filter_state_ = FilterState::Destroyed;
@@ -99,7 +102,9 @@ Http::FilterHeadersStatus CacheFilter::decodeHeaders(Http::RequestHeaderMap& hea
   }
   ASSERT(decoder_callbacks_);
 
-  LookupRequest lookup_request(headers, time_source_.systemTime(), vary_allow_list_);
+  LookupRequest lookup_request(headers, config_->timeSource().systemTime(),
+                               config_->varyAllowList(),
+                               config_->ignoreRequestCacheControlHeader());
   request_allows_inserts_ = !lookup_request.requestCacheControl().no_store_;
   is_head_request_ = headers.getMethodValue() == Http::Headers::get().MethodValues.Head;
   lookup_ = cache_->makeLookupContext(std::move(lookup_request), *decoder_callbacks_);
@@ -151,7 +156,7 @@ Http::FilterHeadersStatus CacheFilter::encodeHeaders(Http::ResponseHeaderMap& he
   // Either a cache miss or a cache entry that is no longer valid.
   // Check if the new response can be cached.
   if (request_allows_inserts_ && !is_head_request_ &&
-      CacheabilityUtils::isCacheableResponse(headers, vary_allow_list_)) {
+      CacheabilityUtils::isCacheableResponse(headers, config_->varyAllowList())) {
     ENVOY_STREAM_LOG(debug, "CacheFilter::encodeHeaders inserting headers", *encoder_callbacks_);
     auto insert_context = cache_->makeInsertContext(std::move(lookup_), *encoder_callbacks_);
     if (insert_context != nullptr) {
@@ -166,7 +171,7 @@ Http::FilterHeadersStatus CacheFilter::encodeHeaders(Http::ResponseHeaderMap& he
                                                insert_status_ = InsertStatus::InsertAbortedByCache;
                                              });
       // Add metadata associated with the cached response. Right now this is only response_time;
-      const ResponseMetadata metadata = {time_source_.systemTime()};
+      const ResponseMetadata metadata = {config_->timeSource().systemTime()};
       insert_queue_->insertHeaders(headers, metadata, end_stream);
     }
     if (end_stream) {
@@ -589,7 +594,7 @@ void CacheFilter::processSuccessfulValidation(Http::ResponseHeaderMap& response_
   if (should_update_cached_entry) {
     // TODO(yosrym93): else the cached entry should be deleted.
     // Update metadata associated with the cached response. Right now this is only response_time;
-    const ResponseMetadata metadata = {time_source_.systemTime()};
+    const ResponseMetadata metadata = {config_->timeSource().systemTime()};
     cache_->updateHeaders(*lookup_, response_headers, metadata,
                           [](bool updated ABSL_ATTRIBUTE_UNUSED) {});
     insert_status_ = InsertStatus::HeaderUpdate;

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

@@ -46,16 +46,30 @@ enum class FilterState {
   Destroyed
 };
 
+class CacheFilterConfig {
+public:
+  CacheFilterConfig(const envoy::extensions::filters::http::cache::v3::CacheConfig& config,
+                    Server::Configuration::CommonFactoryContext& context);
+
+  // The allow list rules that decide if a header can be varied upon.
+  const VaryAllowList& varyAllowList() const { return vary_allow_list_; }
+  TimeSource& timeSource() const { return time_source_; }
+  bool ignoreRequestCacheControlHeader() const { return ignore_request_cache_control_header_; }
+
+private:
+  const VaryAllowList vary_allow_list_;
+  TimeSource& time_source_;
+  const bool ignore_request_cache_control_header_;
+};
+
 /**
  * A filter that caches responses and attempts to satisfy requests from cache.
  */
 class CacheFilter : public Http::PassThroughFilter,
                     public Logger::Loggable<Logger::Id::cache_filter>,
                     public std::enable_shared_from_this<CacheFilter> {
 public:
-  CacheFilter(const envoy::extensions::filters::http::cache::v3::CacheConfig& config,
-              const std::string& stats_prefix, Stats::Scope& scope,
-              Server::Configuration::CommonFactoryContext& context,
+  CacheFilter(std::shared_ptr<const CacheFilterConfig> config,
               std::shared_ptr<HttpCache> http_cache);
   // Http::StreamFilterBase
   void onDestroy() override;
@@ -132,7 +146,6 @@ class CacheFilter : public Http::PassThroughFilter,
   // CacheFilter::onDestroy, allowing the insert queue to outlive the filter
   // while the necessary cache write operations complete.
   std::unique_ptr<CacheInsertQueue> insert_queue_;
-  TimeSource& time_source_;
   std::shared_ptr<HttpCache> cache_;
   LookupContextPtr lookup_;
   LookupResultPtr lookup_result_;
@@ -142,11 +155,7 @@ class CacheFilter : public Http::PassThroughFilter,
   // onHeaders for Range Responses, otherwise initialized by encodeCachedResponse.
   std::vector<AdjustedByteRange> remaining_ranges_;
 
-  // TODO(#12901): The allow list could be constructed only once directly from the config, instead
-  // of doing it per-request. A good example of such config is found in the gzip filter:
-  // source/extensions/filters/http/gzip/gzip_filter.h.
-  // Stores the allow list rules that decide if a header can be varied upon.
-  VaryAllowList vary_allow_list_;
+  const std::shared_ptr<const CacheFilterConfig> config_;
 
   // True if the response has trailers.
   // TODO(toddmgreer): cache trailers.

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

@@ -9,7 +9,7 @@ namespace Cache {
 
 Http::FilterFactoryCb CacheFilterFactory::createFilterFactoryFromProtoTyped(
     const envoy::extensions::filters::http::cache::v3::CacheConfig& config,
-    const std::string& stats_prefix, Server::Configuration::FactoryContext& context) {
+    const std::string& /*stats_prefix*/, Server::Configuration::FactoryContext& context) {
   std::shared_ptr<HttpCache> cache;
   if (!config.disabled().value()) {
     if (!config.has_typed_config()) {
@@ -26,10 +26,9 @@ Http::FilterFactoryCb CacheFilterFactory::createFilterFactoryFromProtoTyped(
     cache = http_cache_factory->getCache(config, context);
   }
 
-  return [config, stats_prefix, &context,
+  return [config = std::make_shared<CacheFilterConfig>(config, context.serverFactoryContext()),
           cache](Http::FilterChainFactoryCallbacks& callbacks) -> void {
-    callbacks.addStreamFilter(std::make_shared<CacheFilter>(config, stats_prefix, context.scope(),
-                                                            context.serverFactoryContext(), cache));
+    callbacks.addStreamFilter(std::make_shared<CacheFilter>(config, cache));
   };
 }
 

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

@@ -24,7 +24,8 @@ namespace HttpFilters {
 namespace Cache {
 
 LookupRequest::LookupRequest(const Http::RequestHeaderMap& request_headers, SystemTime timestamp,
-                             const VaryAllowList& vary_allow_list)
+                             const VaryAllowList& vary_allow_list,
+                             bool ignore_request_cache_control_header)
     : request_headers_(Http::createHeaderMap<Http::RequestHeaderMapImpl>(request_headers)),
       vary_allow_list_(vary_allow_list), timestamp_(timestamp) {
   // These ASSERTs check prerequisites. A request without these headers can't be looked up in cache;
@@ -36,7 +37,9 @@ LookupRequest::LookupRequest(const Http::RequestHeaderMap& request_headers, Syst
   absl::string_view scheme = request_headers.getSchemeValue();
   ASSERT(Http::Utility::schemeIsValid(request_headers.getSchemeValue()));
 
-  initializeRequestCacheControl(request_headers);
+  if (!ignore_request_cache_control_header) {
+    initializeRequestCacheControl(request_headers);
+  }
   // TODO(toddmgreer): Let config determine whether to include scheme, host, and
   // query params.
 

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

@@ -78,7 +78,8 @@ class LookupRequest {
 public:
   // Prereq: request_headers's Path(), Scheme(), and Host() are non-null.
   LookupRequest(const Http::RequestHeaderMap& request_headers, SystemTime timestamp,
-                const VaryAllowList& vary_allow_list);
+                const VaryAllowList& vary_allow_list,
+                bool ignore_request_cache_control_header = false);
 
   const RequestCacheControl& requestCacheControl() const { return request_cache_control_; }
 

test/extensions/filters/http/cache/cache_filter_test.cc

@@ -30,9 +30,8 @@ class CacheFilterTest : public ::testing::Test {
   // The filter has to be created as a shared_ptr to enable shared_from_this() which is used in the
   // cache callbacks.
   CacheFilterSharedPtr makeFilter(std::shared_ptr<HttpCache> cache, bool auto_destroy = true) {
-    std::shared_ptr<CacheFilter> filter(new CacheFilter(config_, /*stats_prefix=*/"",
-                                                        context_.scope(),
-                                                        context_.server_factory_context_, cache),
+    auto config = std::make_shared<CacheFilterConfig>(config_, context_.server_factory_context_);
+    std::shared_ptr<CacheFilter> filter(new CacheFilter(config, cache),
                                         [auto_destroy](CacheFilter* f) {
                                           if (auto_destroy) {
                                             f->onDestroy();

test/extensions/filters/http/cache/http_cache_test.cc

@@ -259,6 +259,30 @@ TEST_F(LookupRequestTest, PragmaNoCacheFallback) {
   EXPECT_EQ(CacheEntryStatus::RequiresValidation, lookup_response.cache_entry_status_);
 }
 
+// "pragma:no-cache" is ignored if ignoreRequestCacheControlHeader is true.
+TEST_F(LookupRequestTest, IgnoreRequestCacheControlHeaderIgnoresPragma) {
+  request_headers_.setReferenceKey(Http::CustomHeaders::get().Pragma, "no-cache");
+  const LookupRequest lookup_request(request_headers_, currentTime(), vary_allow_list_,
+                                     /*ignore_request_cache_control_header=*/true);
+  const Http::TestResponseHeaderMapImpl response_headers(
+      {{"date", formatter_.fromTime(currentTime())}, {"cache-control", "public, max-age=3600"}});
+  const LookupResult lookup_response = makeLookupResult(lookup_request, response_headers);
+  // Response is not expired and no-cache is ignored.
+  EXPECT_EQ(CacheEntryStatus::Ok, lookup_response.cache_entry_status_);
+}
+
+// "cache-control:no-cache" is ignored if ignoreRequestCacheControlHeader is true.
+TEST_F(LookupRequestTest, IgnoreRequestCacheControlHeaderIgnoresCacheControl) {
+  request_headers_.setReferenceKey(Http::CustomHeaders::get().CacheControl, "no-cache");
+  const LookupRequest lookup_request(request_headers_, currentTime(), vary_allow_list_,
+                                     /*ignore_request_cache_control_header=*/true);
+  const Http::TestResponseHeaderMapImpl response_headers(
+      {{"date", formatter_.fromTime(currentTime())}, {"cache-control", "public, max-age=3600"}});
+  const LookupResult lookup_response = makeLookupResult(lookup_request, response_headers);
+  // Response is not expired and no-cache is ignored.
+  EXPECT_EQ(CacheEntryStatus::Ok, lookup_response.cache_entry_status_);
+}
+
 TEST_F(LookupRequestTest, PragmaNoCacheFallbackExtraDirectivesIgnored) {
   request_headers_.setReferenceKey(Http::CustomHeaders::get().Pragma,
                                    "no-cache, custom-directive=custom-value");