diff --git a/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php index 40070036a98a6842ed0f043a06832714d0264e3f..f7d513b0915dde9b3ccf516ccec5a1049b6b8e03 100644 --- a/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php @@ -115,13 +115,30 @@ public function onRespond(FilterResponseEvent $event) { $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 2cc65855bd9e93bd2d7b4e0ee7fb94b1b255a516..14072981d1a69d248c4bd056b9ab639a39ac2e16 100644 --- a/core/modules/image/src/Controller/ImageStyleDownloadController.php +++ b/core/modules/image/src/Controller/ImageStyleDownloadController.php @@ -178,7 +178,11 @@ public function deliver(Request $request, $scheme, ImageStyleInterface $image_st '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 fa0c2d414669a73b0ae38ef76b94f99761fbbbed..1de06071581c9f9a4dafd20f4c97ab1b428f4081 100644 --- a/core/modules/image/src/Tests/ImageStylesPathAndUrlTest.php +++ b/core/modules/image/src/Tests/ImageStylesPathAndUrlTest.php @@ -196,10 +196,15 @@ function doImageStyleUrlAndPathTests($scheme, $clean_url = TRUE, $extra_slash = $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 18a6dfdfb5e9c0572610dc6538e1ee36d5dc969b..a2278e75fbb7c0077e839420da27044ed1545790 100644 --- a/core/modules/page_cache/src/Tests/PageCacheTest.php +++ b/core/modules/page_cache/src/Tests/PageCacheTest.php @@ -406,4 +406,50 @@ public function testFormImmutability() { $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 a65342df6645e28090668c149fa31792b61d90b2..f8354ca29c8ab30b126eab2fc50c94e0e8a508ff 100644 --- a/core/modules/system/src/FileDownloadController.php +++ b/core/modules/system/src/FileDownloadController.php @@ -59,7 +59,11 @@ public function download(Request $request, $scheme = 'private') { } 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 c24d7c51429b4970a3f804a0a745c12158cce090..a77c472d3e47d83a0ee551f3ee4319ce935767d9 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("

oh hai this is html.

"); + return new CacheableResponse("

oh hai this is html.

"); } } } 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 055123b0610d3a94d6599846c0f11ad770994b3c..abd90229b68b8bc540aff8c79c91c573335a7d0b 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,13 +260,28 @@ public function authorizeInit($page_title) { */ 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. */ 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 ecb9921f4433c6f969bac394b4e0f0cb2a9f1a00..94a1bef41636615eb414530505e1098c828366d9 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'