Public responses without lifetime should not remove lifetime for the resulting response

Author: mpdude

HttpCache/ResponseCacheStrategy.php

@@ -81,14 +81,15 @@ public function add(Response $response)
             return;
         }
 
+        $isHeuristicallyCacheable = $response->headers->hasCacheControlDirective('public');
         $maxAge = $response->headers->hasCacheControlDirective('max-age') ? (int) $response->headers->getCacheControlDirective('max-age') : null;
-        $this->storeRelativeAgeDirective('max-age', $maxAge, $age);
+        $this->storeRelativeAgeDirective('max-age', $maxAge, $age, $isHeuristicallyCacheable);
         $sharedMaxAge = $response->headers->hasCacheControlDirective('s-maxage') ? (int) $response->headers->getCacheControlDirective('s-maxage') : $maxAge;
-        $this->storeRelativeAgeDirective('s-maxage', $sharedMaxAge, $age);
+        $this->storeRelativeAgeDirective('s-maxage', $sharedMaxAge, $age, $isHeuristicallyCacheable);
 
         $expires = $response->getExpires();
         $expires = null !== $expires ? (int) $expires->format('U') - (int) $response->getDate()->format('U') : null;
-        $this->storeRelativeAgeDirective('expires', $expires >= 0 ? $expires : null, 0);
+        $this->storeRelativeAgeDirective('expires', $expires >= 0 ? $expires : null, 0, $isHeuristicallyCacheable);
     }
 
     /**
@@ -199,11 +200,29 @@ private function willMakeFinalResponseUncacheable(Response $response): bool
      * we have to subtract the age so that the value is normalized for an age of 0.
      *
      * If the value is lower than the currently stored value, we update the value, to keep a rolling
-     * minimal value of each instruction. If the value is NULL, the directive will not be set on the final response.
+     * minimal value of each instruction.
+     *
+     * If the value is NULL and the isHeuristicallyCacheable parameter is false, the directive will
+     * not be set on the final response. In this case, not all responses had the directive set and no
+     * value can be found that satisfies the requirements of all responses. The directive will be dropped
+     * from the final response.
+     *
+     * If the isHeuristicallyCacheable parameter is true, however, the current response has been marked
+     * as cacheable in a public (shared) cache, but did not provide an explicit lifetime that would serve
+     * as an upper bound. In this case, we can proceed and possibly keep the directive on the final response.
      */
-    private function storeRelativeAgeDirective(string $directive, ?int $value, int $age)
+    private function storeRelativeAgeDirective(string $directive, ?int $value, int $age, bool $isHeuristicallyCacheable)
     {
         if (null === $value) {
+            if ($isHeuristicallyCacheable) {
+                /*
+                 * See https://datatracker.ietf.org/doc/html/rfc7234#section-4.2.2
+                 * This particular response does not require maximum lifetime; heuristics might be applied.
+                 * Other responses, however, might have more stringent requirements on maximum lifetime.
+                 * So, return early here so that the final response can have the more limiting value set.
+                 */
+                return;
+            }
             $this->ageDirectives[$directive] = false;
         }
 

Tests/HttpCache/ResponseCacheStrategyTest.php

@@ -370,7 +370,7 @@ public function cacheControlMergingProvider()
         ];
 
         yield 'merge max-age and s-maxage' => [
-            ['public' => true, 's-maxage' => '60', 'max-age' => null],
+            ['public' => true, 'max-age' => '60'],
             ['public' => true, 's-maxage' => 3600],
             [
                 ['public' => true, 'max-age' => 60],
@@ -393,6 +393,30 @@ public function cacheControlMergingProvider()
             ],
         ];
 
+        yield 'public subresponse without lifetime does not remove lifetime for main response' => [
+            ['public' => true, 's-maxage' => '30', 'max-age' => null],
+            ['public' => true, 's-maxage' => '30'],
+            [
+                ['public' => true],
+            ],
+        ];
+
+        yield 'lifetime for subresponse is kept when main response has no lifetime' => [
+            ['public' => true, 'max-age' => '30'],
+            ['public' => true],
+            [
+                ['public' => true, 'max-age' => '30'],
+            ],
+        ];
+
+        yield 's-maxage on the subresponse implies public, so the result is public as well' => [
+            ['public' => true, 'max-age' => '10', 's-maxage' => null],
+            ['public' => true, 'max-age' => '10'],
+            [
+                ['max-age' => '30', 's-maxage' => '20'],
+            ],
+        ];
+
         yield 'result is private when combining private responses' => [
             ['no-cache' => false, 'must-revalidate' => false, 'private' => true],
             ['s-maxage' => 60, 'private' => true],