summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authoreffulgentsia2015-10-05 15:09:09 -0700
committereffulgentsia2015-10-05 15:09:09 -0700
commit866be5fb95ab375c10d2e9884d628b44ac228296 (patch)
tree8fba56e0c33d46a5da232ce3bf3d249f506fcc2a
parentd5c827e43c567c771c4d91aa0afda87bdbebf670 (diff)
Issue #2476407 by borisson_, hussainweb, znerol, Fabianx, Wim Leers, dawehner, Crell, Berdir: Use CacheableResponseInterface to determine which responses should be cached
-rw-r--r--core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php27
-rw-r--r--core/modules/image/src/Controller/ImageStyleDownloadController.php6
-rw-r--r--core/modules/image/src/Tests/ImageStylesPathAndUrlTest.php13
-rw-r--r--core/modules/page_cache/src/Tests/PageCacheTest.php46
-rw-r--r--core/modules/system/src/FileDownloadController.php6
-rw-r--r--core/modules/system/tests/modules/system_test/src/Controller/PageCacheAcceptHeaderController.php8
-rw-r--r--core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php18
-rw-r--r--core/modules/system/tests/modules/system_test/system_test.routing.yml14
8 files changed, 122 insertions, 16 deletions
diff --git a/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php
index 4007003..f7d513b 100644
--- a/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php
+++ b/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php
@@ -115,13 +115,30 @@ class FinishResponseSubscriber implements EventSubscriberInterface {
$response->headers->set('X-Content-Type-Options', 'nosniff', FALSE);
$response->headers->set('X-Frame-Options', 'SAMEORIGIN', FALSE);
+ // If the current response isn't an implementation of the
+ // CacheableResponseInterface, we assume that a Response is either
+ // explicitly not cacheable or that caching headers are already set in
+ // another place.
+ if (!$response instanceof CacheableResponseInterface) {
+ if (!$this->isCacheControlCustomized($response)) {
+ $this->setResponseNotCacheable($response, $request);
+ }
+
+ // HTTP/1.0 proxies do not support the Vary header, so prevent any caching
+ // by sending an Expires date in the past. HTTP/1.1 clients ignore the
+ // Expires header if a Cache-Control: max-age directive is specified (see
+ // RFC 2616, section 14.9.3).
+ if (!$response->headers->has('Expires')) {
+ $this->setExpiresNoCache($response);
+ }
+ return;
+ }
+
// Expose the cache contexts and cache tags associated with this page in a
// X-Drupal-Cache-Contexts and X-Drupal-Cache-Tags header respectively.
- if ($response instanceof CacheableResponseInterface) {
- $response_cacheability = $response->getCacheableMetadata();
- $response->headers->set('X-Drupal-Cache-Tags', implode(' ', $response_cacheability->getCacheTags()));
- $response->headers->set('X-Drupal-Cache-Contexts', implode(' ', $this->cacheContextsManager->optimizeTokens($response_cacheability->getCacheContexts())));
- }
+ $response_cacheability = $response->getCacheableMetadata();
+ $response->headers->set('X-Drupal-Cache-Tags', implode(' ', $response_cacheability->getCacheTags()));
+ $response->headers->set('X-Drupal-Cache-Contexts', implode(' ', $this->cacheContextsManager->optimizeTokens($response_cacheability->getCacheContexts())));
$is_cacheable = ($this->requestPolicy->check($request) === RequestPolicyInterface::ALLOW) && ($this->responsePolicy->check($response, $request) !== ResponsePolicyInterface::DENY);
diff --git a/core/modules/image/src/Controller/ImageStyleDownloadController.php b/core/modules/image/src/Controller/ImageStyleDownloadController.php
index 2cc6585..1407298 100644
--- a/core/modules/image/src/Controller/ImageStyleDownloadController.php
+++ b/core/modules/image/src/Controller/ImageStyleDownloadController.php
@@ -178,7 +178,11 @@ class ImageStyleDownloadController extends FileDownloadController {
'Content-Type' => $image->getMimeType(),
'Content-Length' => $image->getFileSize(),
);
- return new BinaryFileResponse($uri, 200, $headers);
+ // \Drupal\Core\EventSubscriber\FinishResponseSubscriber::onRespond()
+ // sets response as not cacheable if the Cache-Control header is not
+ // already modified. We pass in FALSE for non-private schemes for the
+ // $public parameter to make sure we don't change the headers.
+ return new BinaryFileResponse($uri, 200, $headers, $scheme !== 'private');
}
else {
$this->logger->notice('Unable to generate the derived image located at %path.', array('%path' => $derivative_uri));
diff --git a/core/modules/image/src/Tests/ImageStylesPathAndUrlTest.php b/core/modules/image/src/Tests/ImageStylesPathAndUrlTest.php
index fa0c2d4..1de0607 100644
--- a/core/modules/image/src/Tests/ImageStylesPathAndUrlTest.php
+++ b/core/modules/image/src/Tests/ImageStylesPathAndUrlTest.php
@@ -196,10 +196,15 @@ class ImageStylesPathAndUrlTest extends WebTestBase {
$this->assertNoRaw( chr(137) . chr(80) . chr(78) . chr(71) . chr(13) . chr(10) . chr(26) . chr(10), 'No PNG signature found in the response body.');
}
}
- elseif ($clean_url) {
- // Add some extra chars to the token.
- $this->drupalGet(str_replace(IMAGE_DERIVATIVE_TOKEN . '=', IMAGE_DERIVATIVE_TOKEN . '=Zo', $generate_url));
- $this->assertResponse(200, 'Existing image was accessible at the URL with an invalid token.');
+ else {
+ $this->assertEqual($this->drupalGetHeader('Expires'), 'Sun, 19 Nov 1978 05:00:00 GMT', 'Expires header was sent.');
+ $this->assertEqual(strpos($this->drupalGetHeader('Cache-Control'), 'no-cache'), FALSE, 'Cache-Control header contains \'no-cache\' to prevent caching.');
+
+ if ($clean_url) {
+ // Add some extra chars to the token.
+ $this->drupalGet(str_replace(IMAGE_DERIVATIVE_TOKEN . '=', IMAGE_DERIVATIVE_TOKEN . '=Zo', $generate_url));
+ $this->assertResponse(200, 'Existing image was accessible at the URL with an invalid token.');
+ }
}
// Allow insecure image derivatives to be created for the remainder of this
diff --git a/core/modules/page_cache/src/Tests/PageCacheTest.php b/core/modules/page_cache/src/Tests/PageCacheTest.php
index 18a6dfd..a2278e7 100644
--- a/core/modules/page_cache/src/Tests/PageCacheTest.php
+++ b/core/modules/page_cache/src/Tests/PageCacheTest.php
@@ -406,4 +406,50 @@ class PageCacheTest extends WebTestBase {
$this->assertText("Immutable: FALSE", "Form is not immutable,");
}
+ /**
+ * Tests cacheability of a CacheableResponse.
+ *
+ * Tests the difference between having a controller return a plain Symfony
+ * Response object versus returning a Response object that implements the
+ * CacheableResponseInterface.
+ */
+ public function testCacheableResponseResponses() {
+ $config = $this->config('system.performance');
+ $config->set('cache.page.max_age', 300);
+ $config->save();
+
+ // Try to fill the cache.
+ $this->drupalGet('/system-test/respond-reponse');
+ $this->assertFalse(in_array('X-Drupal-Cache', $this->drupalGetHeaders()), 'Drupal page cache header not found');
+ $this->assertEqual($this->drupalGetHeader('Cache-Control'), 'must-revalidate, no-cache, post-check=0, pre-check=0, private', 'Cache-Control header was sent');
+
+ // Still not cached, uncacheable response.
+ $this->drupalGet('/system-test/respond-reponse');
+ $this->assertFalse(in_array('X-Drupal-Cache', $this->drupalGetHeaders()), 'Drupal page cache header not found');
+ $this->assertEqual($this->drupalGetHeader('Cache-Control'), 'must-revalidate, no-cache, post-check=0, pre-check=0, private', 'Cache-Control header was sent');
+
+ // Try to fill the cache.
+ $this->drupalGet('/system-test/respond-cacheable-reponse');
+ $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS', 'Page was not cached.');
+ $this->assertEqual($this->drupalGetHeader('Cache-Control'), 'max-age=300, public', 'Cache-Control header was sent.');
+
+ // Should be cached now.
+ $this->drupalGet('/system-test/respond-cacheable-reponse');
+ $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'HIT', 'Page was cached.');
+ $this->assertEqual($this->drupalGetHeader('Cache-Control'), 'max-age=300, public', 'Cache-Control header was sent.');
+
+ // Uninstall page cache. This should flush all caches so the next call to a
+ // previously cached page should be a miss now.
+ $this->container->get('module_installer')
+ ->uninstall(['page_cache']);
+
+ // Try to fill the cache.
+ $this->drupalGet('/system-test/respond-reponse');
+ $this->assertEqual($this->drupalGetHeader('Cache-Control'), 'must-revalidate, no-cache, post-check=0, pre-check=0, private', 'Cache-Control header was sent');
+
+ // Still not cached, uncacheable response.
+ $this->drupalGet('/system-test/respond-reponse');
+ $this->assertEqual($this->drupalGetHeader('Cache-Control'), 'must-revalidate, no-cache, post-check=0, pre-check=0, private', 'Cache-Control header was sent');
+ }
+
}
diff --git a/core/modules/system/src/FileDownloadController.php b/core/modules/system/src/FileDownloadController.php
index a65342d..f8354ca 100644
--- a/core/modules/system/src/FileDownloadController.php
+++ b/core/modules/system/src/FileDownloadController.php
@@ -59,7 +59,11 @@ class FileDownloadController extends ControllerBase {
}
if (count($headers)) {
- return new BinaryFileResponse($uri, 200, $headers);
+ // \Drupal\Core\EventSubscriber\FinishResponseSubscriber::onRespond()
+ // sets response as not cacheable if the Cache-Control header is not
+ // already modified. We pass in FALSE for non-private schemes for the
+ // $public parameter to make sure we don't change the headers.
+ return new BinaryFileResponse($uri, 200, $headers, $scheme !== 'private');
}
throw new AccessDeniedHttpException();
diff --git a/core/modules/system/tests/modules/system_test/src/Controller/PageCacheAcceptHeaderController.php b/core/modules/system/tests/modules/system_test/src/Controller/PageCacheAcceptHeaderController.php
index c24d7c5..a77c472 100644
--- a/core/modules/system/tests/modules/system_test/src/Controller/PageCacheAcceptHeaderController.php
+++ b/core/modules/system/tests/modules/system_test/src/Controller/PageCacheAcceptHeaderController.php
@@ -7,9 +7,9 @@
namespace Drupal\system_test\Controller;
+use Drupal\Core\Cache\CacheableJsonResponse;
+use Drupal\Core\Cache\CacheableResponse;
use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\HttpFoundation\JsonResponse;
-use Symfony\Component\HttpFoundation\Response;
/**
* Defines a controller to respond the page cache accept header test.
@@ -26,10 +26,10 @@ class PageCacheAcceptHeaderController {
*/
public function content(Request $request) {
if ($request->getRequestFormat() === 'json') {
- return new JsonResponse(array('content' => 'oh hai this is json'));
+ return new CacheableJsonResponse(['content' => 'oh hai this is json']);
}
else {
- return new Response("<p>oh hai this is html.</p>");
+ return new CacheableResponse("<p>oh hai this is html.</p>");
}
}
}
diff --git a/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php
index 055123b..abd9022 100644
--- a/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php
+++ b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php
@@ -8,6 +8,7 @@
namespace Drupal\system_test\Controller;
use Drupal\Core\Access\AccessResult;
+use Drupal\Core\Cache\CacheableResponse;
use Drupal\Core\Controller\ControllerBase;
use Drupal\Core\Render\RendererInterface;
use Drupal\Core\Render\Markup;
@@ -259,14 +260,29 @@ class SystemTestController extends ControllerBase {
*/
public function setHeader(Request $request) {
$query = $request->query->all();
- $response = new Response();
+ $response = new CacheableResponse();
$response->headers->set($query['name'], $query['value']);
+ $response->getCacheableMetadata()->addCacheContexts(['url.query_args:name', 'url.query_args:value']);
$response->setContent($this->t('The following header was set: %name: %value', array('%name' => $query['name'], '%value' => $query['value'])));
return $response;
}
/**
+ * A simple page callback that uses a plain Symfony response object.
+ */
+ public function respondWithReponse(Request $request) {
+ return new Response('test');
+ }
+
+ /**
+ * A simple page callback that uses a CacheableResponse object.
+ */
+ public function respondWithCacheableReponse(Request $request) {
+ return new CacheableResponse('test');
+ }
+
+ /**
* A simple page callback which adds a register shutdown function.
*/
public function shutdownFunctions($arg1, $arg2) {
diff --git a/core/modules/system/tests/modules/system_test/system_test.routing.yml b/core/modules/system/tests/modules/system_test/system_test.routing.yml
index ecb9921..94a1bef 100644
--- a/core/modules/system/tests/modules/system_test/system_test.routing.yml
+++ b/core/modules/system/tests/modules/system_test/system_test.routing.yml
@@ -129,3 +129,17 @@ system_test.permission_dependent_route_access:
_controller: '\Drupal\system_test\Controller\SystemTestController::mainContentFallback'
requirements:
_permission: 'pet llamas'
+
+system_test.respond_response:
+ path: '/system-test/respond-reponse'
+ defaults:
+ _controller: '\Drupal\system_test\Controller\SystemTestController::respondWithReponse'
+ requirements:
+ _access: 'TRUE'
+
+system_test.respond_cacheable_response:
+ path: '/system-test/respond-cacheable-reponse'
+ defaults:
+ _controller: '\Drupal\system_test\Controller\SystemTestController::respondWithCacheableReponse'
+ requirements:
+ _access: 'TRUE'