caching: Validate stale cache entries and update cached headers

source/common/http/headers.h

@@ -64,6 +64,12 @@ class CustomHeaderValues {
   const LowerCaseString ContentEncoding{"content-encoding"};
   const LowerCaseString Etag{"etag"};
   const LowerCaseString GrpcAcceptEncoding{"grpc-accept-encoding"};
+  const LowerCaseString IfMatch{"if-match"};
+  const LowerCaseString IfNonMatch{"if-none-match"};
+  const LowerCaseString IfModifiedSince{"if-modified-since"};
+  const LowerCaseString IfUnmodifiedSince{"if-unmodified-since"};
+  const LowerCaseString IfRange{"if-range"};
+  const LowerCaseString LastModified{"last-modified"};
   const LowerCaseString Origin{"origin"};
   const LowerCaseString OtSpanContext{"x-ot-span-context"};
   const LowerCaseString Referer{"referer"};

source/extensions/filters/http/cache/BUILD

@@ -20,10 +20,12 @@ envoy_cc_library(
         ":cache_headers_utils_lib",
         ":cacheability_utils_lib",
         ":http_cache_lib",
+        "//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",
         "@envoy_api//envoy/extensions/filters/http/cache/v3alpha:pkg_cc_proto",
     ],

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

@@ -6,6 +6,7 @@
 #include <vector>
 
 #include "envoy/extensions/filters/http/cache/v3alpha/cache.pb.h"
+#include "envoy/http/header_map.h"
 
 #include "common/common/logger.h"
 
@@ -38,19 +39,55 @@ class CacheFilter : public Http::PassThroughFilter,
   Http::FilterDataStatus encodeData(Buffer::Instance& buffer, bool end_stream) override;
 
 private:
-  void getBody();
+  // Utility functions; make any necessary checks and call the corresponding lookup_ functions
+  void inline getHeaders();
+  void inline getBody();
+  void inline getTrailers();
+
+  // Callbacks for HttpCache to call when headers/body/trailers are ready
   void onHeaders(LookupResult&& result);
   void onBody(Buffer::InstancePtr&& body);
   void onTrailers(Http::ResponseTrailerMapPtr&& trailers);
 
+  // Precondition: lookup_result_ points to a cache lookup result that requires validation
+  //               validating_cache_entry_ is true (a stale cache entry was being validated)
+  // Serves a validated cached response after updating it with a 304 response
+  Http::FilterHeadersStatus processSuccessfulValidation(Http::ResponseHeaderMap& response_headers);
+
+  // Precondition: lookup_result_ points to a cache lookup result that requires validation
+  // Checks if a cached entry should be updated with a 304 response
+  bool shouldUpdateCachedEntry(const Http::ResponseHeaderMap& response_headers) const;
+
+  // Precondition: request_headers_ points to the RequestHeadersMap of the current request
+  //               lookup_result_ points to a cache lookup result that requires validation
+  // Should only be called during onHeaders as it modifies RequestHeaderMap
+  // Adds required conditional headers for cache validation to the request headers
+  // according to the present cache lookup result headers
+  void injectValidationHeaders();
+
+  // Precondition: lookup_result_ points to a fresh or validated cache look up result
+  // Adds a cache lookup result to the response encoding stream
+  // Can be called during decoding if a valid cache hit is found
+  // or during encoding if a cache entry was validated successfully
+  void encodeCachedResponse();
+
+  // Precondition: finished adding a response from cache to the response encoding stream
+  // Updates the encode_cached_response_state_ and continue encoding filter iteration if necessary
+  void finishedEncodingCachedResponse();
+
   TimeSource& time_source_;
   HttpCache& cache_;
   LookupContextPtr lookup_;
   InsertContextPtr insert_;
+  LookupResultPtr lookup_result_;
 
-  // Tracks what body bytes still need to be read from the cache. This is
-  // currently only one Range, but will expand when full range support is added. Initialized by
-  // onOkHeaders.
+  // Used exclusively to store a reference to the request header map passed to decodeHeaders to be
+  // used in onHeaders afterwards the pointer must not be used and is set back to null
+  // This Http::RequestHeaderMap is NOT owned by the CacheFilter
+  Http::RequestHeaderMap* request_headers_ = nullptr;
+
+  // Tracks what body bytes still need to be read from the cache. This is currently only one Range,
+  // but will expand when full range support is added. Initialized by encodeCachedResponse.
   std::vector<AdjustedByteRange> remaining_body_;
 
   // True if the response has trailers.
@@ -61,9 +98,22 @@ class CacheFilter : public Http::PassThroughFilter,
   // https://httpwg.org/specs/rfc7234.html#response.cacheability
   bool request_allows_inserts_ = false;
 
+  // True if the CacheFilter is injected validation headers and should check for 304 responses
+  bool validating_cache_entry_ = false;
+
+  // True if CacheFilter::encodeHeaders & CacheFilter::encodeData should be skipped
+  bool skip_encoding_ = false;
+
   // Used for coordinating between decodeHeaders and onHeaders.
   enum class GetHeadersState { Initial, FinishedGetHeadersCall, GetHeadersResultUnusable };
-  GetHeadersState state_ = GetHeadersState::Initial;
+  GetHeadersState get_headers_state_ = GetHeadersState::Initial;
+
+  // Used for coordinating between encodeHeaders/encodeData and onBody/onTrailers
+  enum class EncodeCachedResponseState { Initial, FinishedEncoding, IterationStopped };
+  EncodeCachedResponseState encode_cached_response_state_ = EncodeCachedResponseState::Initial;
+
+  enum class FilterState { Initial, Decoding, Encoding };
+  FilterState filter_state_ = FilterState::Initial;
 };
 
 } // namespace Cache

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

@@ -18,6 +18,14 @@ const absl::flat_hash_set<absl::string_view>& cacheableStatusCodes() {
   CONSTRUCT_ON_FIRST_USE(absl::flat_hash_set<absl::string_view>, "200", "203", "204", "206", "300",
                          "301", "308", "404", "405", "410", "414", "451", "501");
 }
+
+const std::vector<const Http::LowerCaseString*>& conditionalHeaders() {
+  // As defined by: https://httpwg.org/specs/rfc7232.html#preconditions
+  CONSTRUCT_ON_FIRST_USE(
+      std::vector<const Http::LowerCaseString*>, &Http::CustomHeaders::get().IfMatch,
+      &Http::CustomHeaders::get().IfNonMatch, &Http::CustomHeaders::get().IfModifiedSince,
+      &Http::CustomHeaders::get().IfUnmodifiedSince, &Http::CustomHeaders::get().IfRange);
+}
 } // namespace
 
 Http::RegisterCustomInlineHeader<Http::CustomInlineHeaderRegistry::Type::RequestHeaders>
@@ -29,10 +37,24 @@ bool CacheabilityUtils::isCacheableRequest(const Http::RequestHeaderMap& headers
   const absl::string_view method = headers.getMethodValue();
   const absl::string_view forwarded_proto = headers.getForwardedProtoValue();
   const Http::HeaderValues& header_values = Http::Headers::get();
+
+  // Check if the request contains any conditional headers
+  // For now, requests with conditional headers bypass the CacheFilter
+  // This behavior does not cause any incorrect results but may reduce the cache effectiveness
+  // If needed to be handled properly refer to:
+  // https://httpwg.org/specs/rfc7234.html#validation.received
+  bool contains_conditional_headers = false;
+  for (auto conditional_header : conditionalHeaders()) {
+    if (headers.get(*conditional_header)) {
+      contains_conditional_headers = true;
+      break;
+    }
+  }
+
   // TODO(toddmgreer): Also serve HEAD requests from cache.
-  // TODO(toddmgreer): Check all the other cache-related headers.
+  // Cache-related headers are checked in HttpCache::LookupRequest
   return headers.Path() && headers.Host() && !headers.getInline(authorization_handle.handle()) &&
-         (method == header_values.MethodValues.Get) &&
+         !contains_conditional_headers && (method == header_values.MethodValues.Get) &&
          (forwarded_proto == header_values.SchemeValues.Http ||
           forwarded_proto == header_values.SchemeValues.Https);
 }

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

@@ -1,5 +1,7 @@
 #pragma once
 
+#include "envoy/http/header_map.h"
+
 #include "common/common/utility.h"
 #include "common/http/headers.h"
 

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

@@ -138,6 +138,7 @@ struct LookupResult {
   // True if the cached response has trailers.
   bool has_trailers_ = false;
 };
+using LookupResultPtr = std::unique_ptr<LookupResult>;
 
 // Produces a hash of key that is consistent across restarts, architectures,
 // builds, and configurations. Caches that store persistent entries based on a
@@ -291,8 +292,8 @@ class HttpCache {
   //
   // This is called when an expired cache entry is successfully validated, to
   // update the cache entry.
-  virtual void updateHeaders(LookupContextPtr&& lookup_context,
-                             Http::ResponseHeaderMapPtr&& response_headers) PURE;
+  virtual void updateHeaders(const LookupContext& lookup_context,
+                             const Http::ResponseHeaderMap& response_headers) PURE;
 
   // Returns statically known information about a cache.
   virtual CacheInfo cacheInfo() const PURE;

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

@@ -92,12 +92,10 @@ LookupContextPtr SimpleHttpCache::makeLookupContext(LookupRequest&& request) {
   return std::make_unique<SimpleLookupContext>(*this, std::move(request));
 }
 
-void SimpleHttpCache::updateHeaders(LookupContextPtr&& lookup_context,
-                                    Http::ResponseHeaderMapPtr&& response_headers) {
-  ASSERT(lookup_context);
-  ASSERT(response_headers);
+void SimpleHttpCache::updateHeaders(const LookupContext&, const Http::ResponseHeaderMap&) {
   // TODO(toddmgreer): Support updating headers.
-  NOT_IMPLEMENTED_GCOVR_EXCL_LINE;
+  // Not implemented yet, however this is called during tests
+  // NOT_IMPLEMENTED_GCOVR_EXCL_LINE;
 }
 
 SimpleHttpCache::Entry SimpleHttpCache::lookup(const LookupRequest& request) {

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

@@ -25,8 +25,8 @@ class SimpleHttpCache : public HttpCache {
   // HttpCache
   LookupContextPtr makeLookupContext(LookupRequest&& request) override;
   InsertContextPtr makeInsertContext(LookupContextPtr&& lookup_context) override;
-  void updateHeaders(LookupContextPtr&& lookup_context,
-                     Http::ResponseHeaderMapPtr&& response_headers) override;
+  void updateHeaders(const LookupContext& lookup_context,
+                     const Http::ResponseHeaderMap& response_headers) override;
   CacheInfo cacheInfo() const override;
 
   Entry lookup(const LookupRequest& request);

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

@@ -87,6 +87,150 @@ TEST_P(CacheIntegrationTest, MissInsertHit) {
   EXPECT_EQ(waitForAccessLog(access_log_name_, 1), "RFCF cache.response_from_cache_filter\n");
 }
 
+TEST_P(CacheIntegrationTest, SuccessfulValidation) {
+  useAccessLog("%RESPONSE_FLAGS% %RESPONSE_CODE_DETAILS%");
+  // Set system time to cause Envoy's cached formatted time to match time on this thread.
+  simTime().setSystemTime(std::chrono::hours(1));
+  initializeFilter(default_config);
+
+  // Include test name and params in URL to make each test's requests unique.
+  const Http::TestRequestHeaderMapImpl request_headers = {
+      {":method", "GET"},
+      {":path", absl::StrCat("/", protocolTestParamsToString({GetParam(), 0}))},
+      {":scheme", "http"},
+      {":authority", "SuccessfulValidation"}};
+
+  const std::string original_response_date = formatter_.now(simTime());
+  Http::TestResponseHeaderMapImpl response_headers = {{":status", "200"},
+                                                      {"date", original_response_date},
+                                                      {"cache-control", "max-age=0"},
+                                                      {"content-length", "42"},
+                                                      {"etag", "abc123"}};
+
+  // Send first request, and get response from upstream.
+  {
+    IntegrationStreamDecoderPtr response_decoder =
+        codec_client_->makeHeaderOnlyRequest(request_headers);
+    waitForNextUpstreamRequest();
+    upstream_request_->encodeHeaders(response_headers, /*end_stream=*/false);
+    // send 42 'a's
+    upstream_request_->encodeData(42, true);
+    // Wait for the response to be read by the codec client.
+    response_decoder->waitForEndStream();
+    EXPECT_TRUE(response_decoder->complete());
+    EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(response_headers));
+    EXPECT_EQ(response_decoder->headers().get(Http::Headers::get().Age), nullptr);
+    EXPECT_EQ(response_decoder->body(), std::string(42, 'a'));
+    EXPECT_EQ(waitForAccessLog(access_log_name_), "- via_upstream\n");
+  }
+
+  simTime().advanceTimeWait(std::chrono::seconds(10));
+  const std::string not_modified_date = formatter_.now(simTime());
+
+  // Send second request, the cached response should be validated then served
+  IntegrationStreamDecoderPtr response_decoder =
+      codec_client_->makeHeaderOnlyRequest(request_headers);
+  waitForNextUpstreamRequest();
+
+  // Check for injected precondition headers (no Last-Modified header so should fallback to Date)
+  Http::TestRequestHeaderMapImpl injected_headers = {{"if-none-match", "abc123"},
+                                                     {"if-modified-since", original_response_date}};
+  EXPECT_THAT(upstream_request_->headers(), IsSupersetOfHeaders(injected_headers));
+
+  // Create a 304 (not modified) response -> cached response is valid
+  Http::TestResponseHeaderMapImpl not_modified_response_headers = {{":status", "304"},
+                                                                   {"date", not_modified_date}};
+  upstream_request_->encodeHeaders(not_modified_response_headers, /*end_stream=*/true);
+
+  // The original response headers should be updated with 304 response headers
+  response_headers.setDate(not_modified_date);
+
+  // Wait for the response to be read by the codec client.
+  response_decoder->waitForEndStream();
+
+  // Check that the served response is the cached response
+  EXPECT_TRUE(response_decoder->complete());
+  EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(response_headers));
+  EXPECT_EQ(response_decoder->body(), std::string(42, 'a'));
+  // Check that age header exists as this is a cached response
+  EXPECT_NE(response_decoder->headers().get(Http::Headers::get().Age), nullptr);
+
+  // Advance time to force a log flush.
+  simTime().advanceTimeWait(std::chrono::seconds(1));
+  EXPECT_EQ(waitForAccessLog(access_log_name_, 1), "RFCF cache.response_from_cache_filter\n");
+}
+
+TEST_P(CacheIntegrationTest, UnsuccessfulValidation) {
+  useAccessLog("%RESPONSE_FLAGS% %RESPONSE_CODE_DETAILS%");
+  // Set system time to cause Envoy's cached formatted time to match time on this thread.
+  simTime().setSystemTime(std::chrono::hours(1));
+  initializeFilter(default_config);
+
+  // Include test name and params in URL to make each test's requests unique.
+  const Http::TestRequestHeaderMapImpl request_headers = {
+      {":method", "GET"},
+      {":path", absl::StrCat("/", protocolTestParamsToString({GetParam(), 0}))},
+      {":scheme", "http"},
+      {":authority", "UnsuccessfulValidation"}};
+
+  Http::TestResponseHeaderMapImpl original_response_headers = {{":status", "200"},
+                                                               {"date", formatter_.now(simTime())},
+                                                               {"cache-control", "max-age=0"},
+                                                               {"content-length", "10"},
+                                                               {"etag", "a1"}};
+
+  // Send first request, and get response from upstream.
+  {
+    IntegrationStreamDecoderPtr response_decoder =
+        codec_client_->makeHeaderOnlyRequest(request_headers);
+    waitForNextUpstreamRequest();
+    upstream_request_->encodeHeaders(original_response_headers, /*end_stream=*/false);
+    // send 10 'a's
+    upstream_request_->encodeData(10, true);
+    // Wait for the response to be read by the codec client.
+    response_decoder->waitForEndStream();
+    EXPECT_TRUE(response_decoder->complete());
+    EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(original_response_headers));
+    EXPECT_EQ(response_decoder->headers().get(Http::Headers::get().Age), nullptr);
+    EXPECT_EQ(response_decoder->body(), std::string(10, 'a'));
+    EXPECT_EQ(waitForAccessLog(access_log_name_), "- via_upstream\n");
+  }
+
+  simTime().advanceTimeWait(std::chrono::seconds(10));
+  Http::TestResponseHeaderMapImpl updated_response_headers = {{":status", "200"},
+                                                              {"date", formatter_.now(simTime())},
+                                                              {"cache-control", "max-age=0"},
+                                                              {"content-length", "20"},
+                                                              {"etag", "a2"}};
+
+  // Send second request, validation of the cached response should be attempted but should fail
+  IntegrationStreamDecoderPtr response_decoder =
+      codec_client_->makeHeaderOnlyRequest(request_headers);
+  waitForNextUpstreamRequest();
+
+  // Check for injected precondition headers
+  Http::TestRequestHeaderMapImpl injected_headers = {{"if-none-match", "a1"}};
+  EXPECT_THAT(upstream_request_->headers(), IsSupersetOfHeaders(injected_headers));
+
+  // Reply with the updated response -> cached response is invalid
+  upstream_request_->encodeHeaders(updated_response_headers, /*end_stream=*/false);
+  // send 20 'a's
+  upstream_request_->encodeData(20, true);
+
+  // Wait for the response to be read by the codec client.
+  response_decoder->waitForEndStream();
+  // Check that the served response is the updated response
+  EXPECT_TRUE(response_decoder->complete());
+  EXPECT_THAT(response_decoder->headers(), IsSupersetOfHeaders(updated_response_headers));
+  EXPECT_EQ(response_decoder->body(), std::string(20, 'a'));
+  // Check that age header does not exist as this is not a cached response
+  EXPECT_EQ(response_decoder->headers().get(Http::Headers::get().Age), nullptr);
+
+  // Advance time to force a log flush.
+  simTime().advanceTimeWait(std::chrono::seconds(1));
+  EXPECT_EQ(waitForAccessLog(access_log_name_, 1), "- via_upstream\n");
+}
+
 // Send the same GET request twice with body and trailers twice, then check that the response
 // doesn't have an age header, to confirm that it wasn't served from cache.
 TEST_P(CacheIntegrationTest, GetRequestWithBodyAndTrailers) {

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

@@ -1,3 +1,5 @@
+#include "envoy/http/header_map.h"
+
 #include "extensions/filters/http/cache/cacheability_utils.h"
 
 #include "test/test_common/utility.h"
@@ -10,7 +12,7 @@ namespace HttpFilters {
 namespace Cache {
 namespace {
 
-class IsCacheableRequestTest : public testing::Test {
+class IsCacheableRequestTest : public testing::TestWithParam<std::string> {
 protected:
   const Http::TestRequestHeaderMapImpl cacheable_request_headers_ = {{":path", "/"},
                                                                      {":method", "GET"},
@@ -74,6 +76,17 @@ TEST_F(IsCacheableRequestTest, AuthorizationHeader) {
   EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers));
 }
 
+INSTANTIATE_TEST_SUITE_P(ConditionalHeaders, IsCacheableRequestTest,
+                         testing::Values("if-match", "if-none-match", "if-modified-since",
+                                         "if-unmodified-since", "if-range"));
+
+TEST_P(IsCacheableRequestTest, ConditionalHeaders) {
+  Http::TestRequestHeaderMapImpl request_headers = cacheable_request_headers_;
+  EXPECT_TRUE(CacheabilityUtils::isCacheableRequest(request_headers));
+  request_headers.setCopy(Http::LowerCaseString{GetParam()}, "test-value");
+  EXPECT_FALSE(CacheabilityUtils::isCacheableRequest(request_headers));
+}
+
 TEST_F(IsCacheableResponseTest, CacheableResponse) {
   EXPECT_TRUE(CacheabilityUtils::isCacheableResponse(cacheable_response_headers_));
 }