Merge pull request #8245 from dhensby/pulls/4.2/http-cache-fixes

This is a cherry-picked PR from 4.2 -> 4.2.0

src/Control/Middleware/HTTPCacheControlMiddleware.php

@@ -26,8 +26,6 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable
 
     const STATE_DISABLED = 'disabled';
 
-    const STATE_DEFAULT = 'default';
-
     /**
      * Generate response for the given request
      *
@@ -90,11 +88,9 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable
             'must-revalidate' => true,
         ],
         self::STATE_ENABLED => [
-            'must-revalidate' => true,
-        ],
-        self::STATE_DEFAULT => [
             'no-cache' => true,
-        ],
+            'must-revalidate' => true,
+        ]
     ];
 
     /**
@@ -103,7 +99,7 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable
      * @config
      * @var string
      */
-    private static $defaultState = self::STATE_DEFAULT;
+    private static $defaultState = self::STATE_ENABLED;
 
     /**
      * Current state
@@ -498,7 +494,13 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable
     {
         // Affect all non-disabled states
         $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC];
-        $this->setStateDirective($applyTo, 'no-cache', $noCache);
+        if ($noCache) {
+            $this->setStateDirective($applyTo, 'no-cache');
+            $this->removeStateDirective($applyTo, 'max-age');
+            $this->removeStateDirective($applyTo, 's-maxage');
+        } else {
+            $this->removeStateDirective($applyTo, 'no-cache');
+        }
         return $this;
     }
 
@@ -515,6 +517,10 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable
         // Affect all non-disabled states
         $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC];
         $this->setStateDirective($applyTo, 'max-age', $age);
+        if ($age) {
+            $this->removeStateDirective($applyTo, 'no-cache');
+            $this->removeStateDirective($applyTo, 'no-store');
+        }
         return $this;
     }
 
@@ -531,6 +537,10 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable
         // Affect all non-disabled states
         $applyTo = [self::STATE_ENABLED, self::STATE_PRIVATE, self::STATE_PUBLIC];
         $this->setStateDirective($applyTo, 's-maxage', $age);
+        if ($age) {
+            $this->removeStateDirective($applyTo, 'no-cache');
+            $this->removeStateDirective($applyTo, 'no-store');
+        }
         return $this;
     }
 
@@ -763,17 +773,16 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable
      */
     protected function augmentState(HTTPRequest $request, HTTPResponse $response)
     {
-        // If sessions exist we assume that the responses should not be cached by CDNs / proxies as we are
-        // likely to be supplying information relevant to the current user only
-        if ($request->getSession()->getAll()) {
-            // Don't force in case user code chooses to opt in to public caching
-            $this->privateCache();
-        }
-
         // Errors disable cache (unless some errors are cached intentionally by usercode)
         if ($response->isError() || $response->isRedirect()) {
             // Even if publicCache(true) is specified, errors will be uncacheable
             $this->disableCache(true);
+        } elseif ($request->getSession()->getAll()) {
+            // If sessions exist we assume that the responses should not be cached by CDNs / proxies as we are
+            // likely to be supplying information relevant to the current user only
+
+            // Don't force in case user code chooses to opt in to public caching
+            $this->privateCache();
         }
     }
 }

tests/php/Control/Middleware/HTTPCacheControlMiddlewareTest.php

@@ -2,6 +2,7 @@
 
 namespace SilverStripe\Control\Tests\Middleware;
 
+use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\Middleware\HTTPCacheControlMiddleware;
 use SilverStripe\Dev\SapphireTest;
 
@@ -12,15 +13,177 @@ class HTTPCacheControlMiddlewareTest extends SapphireTest
         parent::setUp();
         // Set to disabled at null forcing level
         HTTPCacheControlMiddleware::config()
-            ->set('defaultState', 'disabled')
+            ->set('defaultState', HTTPCacheControlMiddleware::STATE_ENABLED)
             ->set('defaultForcingLevel', 0);
         HTTPCacheControlMiddleware::reset();
     }
 
+    public function provideCacheStates()
+    {
+        return [
+            ['enableCache', false],
+            ['publicCache', false],
+            ['privateCache', false],
+            ['disableCache', true],
+        ];
+    }
+
+    /**
+     * @dataProvider provideCacheStates
+     */
+    public function testCheckDefaultStates($state, $immutable)
+    {
+        $cc = HTTPCacheControlMiddleware::singleton();
+        $cc->{$state}();
+
+        $response = new HTTPResponse();
+        $cc->applyToResponse($response);
+
+        $this->assertContains('must-revalidate', $response->getHeader('cache-control'));
+    }
+
+    /**
+     * @dataProvider provideCacheStates
+     */
+    public function testSetMaxAge($state, $immutable)
+    {
+        $cc = HTTPCacheControlMiddleware::singleton();
+        $cc->{$state}();
+
+        $originalResponse = new HTTPResponse();
+        $cc->applyToResponse($originalResponse);
+
+        $cc->setMaxAge('300');
+
+        $response = new HTTPResponse();
+
+        $cc->applyToResponse($response);
+
+        if ($immutable) {
+            $this->assertEquals($originalResponse->getHeader('cache-control'), $response->getHeader('cache-control'));
+        } else {
+            $this->assertContains('max-age=300', $response->getHeader('cache-control'));
+            $this->assertNotContains('no-cache', $response->getHeader('cache-control'));
+            $this->assertNotContains('no-store', $response->getHeader('cache-control'));
+        }
+    }
+
+    /**
+     * @dataProvider provideCacheStates
+     */
+    public function testSetNoStore($state, $immutable)
+    {
+        $cc = HTTPCacheControlMiddleware::singleton();
+        $cc->setMaxAge('300');
+        $cc->setSharedMaxAge('300');
+
+        $cc->{$state}();
+
+        $originalResponse = new HTTPResponse();
+        $cc->applyToResponse($originalResponse);
+
+        $cc->setNoStore();
+
+        $response = new HTTPResponse();
+
+        $cc->applyToResponse($response);
+
+        if ($immutable) {
+            $this->assertEquals($originalResponse->getHeader('cache-control'), $response->getHeader('cache-control'));
+        } else {
+            $this->assertContains('no-store', $response->getHeader('cache-control'));
+            $this->assertNotContains('max-age', $response->getHeader('cache-control'));
+            $this->assertNotContains('s-maxage', $response->getHeader('cache-control'));
+        }
+    }
+
+    /**
+     * @dataProvider provideCacheStates
+     */
+    public function testSetNoCache($state, $immutable)
+    {
+        $cc = HTTPCacheControlMiddleware::singleton();
+        $cc->setMaxAge('300');
+        $cc->setSharedMaxAge('300');
+
+        $cc->{$state}();
+
+        $originalResponse = new HTTPResponse();
+        $cc->applyToResponse($originalResponse);
+
+        $cc->setNoCache();
+
+        $response = new HTTPResponse();
+
+        $cc->applyToResponse($response);
+
+        if ($immutable) {
+            $this->assertEquals($originalResponse->getHeader('cache-control'), $response->getHeader('cache-control'));
+        } else {
+            $this->assertContains('no-cache', $response->getHeader('cache-control'));
+            $this->assertNotContains('max-age', $response->getHeader('cache-control'));
+            $this->assertNotContains('s-maxage', $response->getHeader('cache-control'));
+        }
+    }
+
+    /**
+     * @dataProvider provideCacheStates
+     */
+    public function testSetSharedMaxAge($state, $immutable)
+    {
+        $cc = HTTPCacheControlMiddleware::singleton();
+
+        $cc->{$state}();
+
+        $originalResponse = new HTTPResponse();
+        $cc->applyToResponse($originalResponse);
+
+        $cc->setSharedMaxAge('300');
+
+        $response = new HTTPResponse();
+
+        $cc->applyToResponse($response);
+
+        if ($immutable) {
+            $this->assertEquals($originalResponse->getHeader('cache-control'), $response->getHeader('cache-control'));
+        } else {
+            $this->assertContains('s-maxage=300', $response->getHeader('cache-control'));
+            $this->assertNotContains('no-cache', $response->getHeader('cache-control'));
+            $this->assertNotContains('no-store', $response->getHeader('cache-control'));
+        }
+    }
+
+    /**
+     * @dataProvider provideCacheStates
+     */
+    public function testSetMustRevalidate($state, $immutable)
+    {
+        $cc = HTTPCacheControlMiddleware::singleton();
+
+        $cc->{$state}();
+
+        $originalResponse = new HTTPResponse();
+        $cc->applyToResponse($originalResponse);
+
+        $cc->setMustRevalidate();
+
+        $response = new HTTPResponse();
+
+        $cc->applyToResponse($response);
+
+        if ($immutable) {
+            $this->assertEquals($originalResponse->getHeader('cache-control'), $response->getHeader('cache-control'));
+        } else {
+            $this->assertContains('must-revalidate', $response->getHeader('cache-control'));
+            $this->assertNotContains('max-age', $response->getHeader('cache-control'));
+            $this->assertNotContains('s-maxage', $response->getHeader('cache-control'));
+        }
+    }
+
     public function testCachingPriorities()
     {
         $hcc = new HTTPCacheControlMiddleware();
-        $this->assertTrue($this->isDisabled($hcc), 'caching starts as disabled');
+        $this->assertFalse($this->isDisabled($hcc), 'caching starts as disabled');
 
         $hcc->enableCache();
         $this->assertFalse($this->isDisabled($hcc));
@@ -74,6 +237,6 @@ class HTTPCacheControlMiddlewareTest extends SapphireTest
 
     protected function isDisabled(HTTPCacheControlMiddleware $hcc)
     {
-        return $hcc->hasDirective('no-cache') && !$hcc->hasDirective('private') && !$hcc->hasDirective('public');
+        return $hcc->hasDirective('no-store') && !$hcc->hasDirective('private') && !$hcc->hasDirective('public');
     }
 }