diff -u b/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php --- b/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php @@ -141,35 +141,25 @@ // for that is stored in the current render context. If the current route is // not cacheable we simply discard it. if (!$context->isEmpty() && !$this->routeMatch->getRouteObject()->getOption('no_cache')) { - /** @var \Drupal\Core\Render\BubbleableMetadata $early_rendering_bubbleable_metadata */ - $early_rendering_bubbleable_metadata = $context->pop(); - // If a render array is returned by the controller, merge the "lost" // bubbleable metadata. if (is_array($response)) { + $early_rendering_bubbleable_metadata = $context->pop(); BubbleableMetadata::createFromRenderArray($response) ->merge($early_rendering_bubbleable_metadata) ->applyTo($response); } - - // If a Response object is returned, and it cares about cacheability, then - // add the dependency. - if ($response instanceof CacheableResponseInterface) { - $response->addCacheableDependency($early_rendering_bubbleable_metadata); - } - // If a domain object is returned, and it cares about cacheability, then - // throw an exception: early rendering is not permitted in that case. It - // is the developer's responsibility to not use early rendering. - elseif ($response instanceof CacheableDependencyInterface) { + // If a Response or domain object is returned, and it cares about + // attachments or cacheability, then throw an exception: early rendering + // is not permitted in that case. It is the developer's responsibility + // to not use early rendering. + elseif ($response instanceof AttachmentsInterface || $response instanceof CacheableResponseInterface || $response instanceof CacheableDependencyInterface) { throw new \LogicException(sprintf('The controller result claims to be providing relevant cache metadata, but leaked metadata was detected. Please ensure you are not rendering content too early. Returned object class: %s.', get_class($response))); } - - // If a Response or domain object is returned, and it cares about - // attachments, and early rendering required additional attachments then - // throw an exception: early rendering is not permitted in that case. It - // is the developer's responsibility to not use early rendering. - elseif ($response instanceof AttachmentsInterface && $early_rendering_bubbleable_metadata->getAttachments()) { - throw new \LogicException(sprintf('The controller result claims to be providing relevant attachments, but leaked attachments were detected. Please ensure you are not rendering content too early. Returned object class: %s.', get_class($response))); + else { + // A Response or domain object is returned that does not care about + // attachments nor cacheability. E.g. a RedirectResponse. It is safe to + // discard any early rendering metadata. } } diff -u b/core/lib/Drupal/Core/Render/CacheableMetadataAwareUrlGenerator.php b/core/lib/Drupal/Core/Render/CacheableMetadataAwareUrlGenerator.php --- b/core/lib/Drupal/Core/Render/CacheableMetadataAwareUrlGenerator.php +++ b/core/lib/Drupal/Core/Render/CacheableMetadataAwareUrlGenerator.php @@ -8,8 +8,8 @@ namespace Drupal\Core\Render; use Drupal\Core\GeneratedUrl; +use Drupal\Core\PageCache\RequestPolicyInterface; use Drupal\Core\Routing\UrlGeneratorInterface; -use Drupal\Core\UncacheableUrl; use Symfony\Component\Routing\RequestContext as SymfonyRequestContext; /** @@ -74,18 +74,22 @@ * * @param \Drupal\Core\GeneratedUrl $generated_url * The generated URL whose bubbleable metadata to bubble. - * @param array $options - * (optional) The URL options. Defaults to none. */ - protected function bubble(GeneratedUrl $generated_url, array $options = []) { - // Bubbling metadata makes sense only if the code is executed inside a - // render context. All code running outside controllers has no render - // context by default, so URLs used there are not supposed to affect the - // response cacheability. - if (empty($options[UncacheableUrl::UNCACHEABLE_OPTION]) && $this->renderer->hasRenderContext()) { - $build = []; - $generated_url->applyTo($build); - $this->renderer->render($build); + protected function bubble(GeneratedUrl $generated_url) { + if ($generated_url->affectsCacheability()) { + if ($this->renderer->hasRenderContext()) { + $build = []; + $generated_url->applyTo($build); + $this->renderer->render($build); + } + else { + $request_policy = \Drupal::service('page_cache_request_policy'); + $request = \Drupal::requestStack()->getCurrentRequest(); + $request_is_cacheable = $request_policy->check($request) === RequestPolicyInterface::ALLOW; + if ($request_is_cacheable) { + throw new \LogicException('A generated URL affects cacheability, but has no render context into which to add that information.'); + } + } } } @@ -105,7 +109,7 @@ public function generateFromRoute($name, $parameters = array(), $options = array(), $collect_cacheability_metadata = FALSE) { $generated_url = $this->urlGenerator->generateFromRoute($name, $parameters, $options, TRUE); if (!$collect_cacheability_metadata) { - $this->bubble($generated_url, $options); + $this->bubble($generated_url); } return $collect_cacheability_metadata ? $generated_url : $generated_url->getGeneratedUrl(); } @@ -116,7 +120,7 @@ public function generateFromPath($path = NULL, $options = array(), $collect_cacheability_metadata = FALSE) { $generated_url = $this->urlGenerator->generateFromPath($path, $options, TRUE); if (!$collect_cacheability_metadata) { - $this->bubble($generated_url, $options); + $this->bubble($generated_url); } return $collect_cacheability_metadata ? $generated_url : $generated_url->getGeneratedUrl(); } diff -u b/core/modules/simpletest/src/WebTestBase.php b/core/modules/simpletest/src/WebTestBase.php --- b/core/modules/simpletest/src/WebTestBase.php +++ b/core/modules/simpletest/src/WebTestBase.php @@ -2811,20 +2811,20 @@ * Options to be passed to Url::fromUri(). * * @return string - * An absolute URL stsring. + * An absolute URL string. */ protected function buildUrl($path, array $options = array()) { if ($path instanceof Url) { $url_options = $path->getOptions(); $options = $url_options + $options; $path->setOptions($options); - return $path->setAbsolute()->toString(); + return $path->setAbsolute()->toString(TRUE)->getGeneratedUrl(); } // The URL generator service is not necessarily available yet; e.g., in // interactive installer tests. else if ($this->container->has('url_generator')) { $options['absolute'] = TRUE; - return $this->container->get('url_generator')->generateFromPath($path, $options); + return $this->container->get('url_generator')->generateFromPath($path, $options, TRUE)->getGeneratedUrl(); } else { return $this->getAbsoluteUrl($path); diff -u b/core/modules/system/src/Tests/Common/EarlyRenderingControllerTest.php b/core/modules/system/src/Tests/Common/EarlyRenderingControllerTest.php --- b/core/modules/system/src/Tests/Common/EarlyRenderingControllerTest.php +++ b/core/modules/system/src/Tests/Common/EarlyRenderingControllerTest.php @@ -59,7 +59,7 @@ $this->assertNoCacheTag('foo'); $this->drupalGet(Url::fromRoute('early_rendering_controller_test.response-with-attachments.early')); $this->assertResponse(500); - $this->assertRaw('The controller result claims to be providing relevant attachments, but leaked attachments were detected. Please ensure you are not rendering content too early. Returned object class: Drupal\early_rendering_controller_test\AttachmentsTestResponse.'); + $this->assertRaw('The controller result claims to be providing relevant cache metadata, but leaked metadata was detected. Please ensure you are not rendering content too early. Returned object class: Drupal\early_rendering_controller_test\AttachmentsTestResponse.'); $this->drupalGet(Url::fromRoute('early_rendering_controller_test.response-with-attachments.early_no_cache')); $this->assertResponse(200); $this->assertRaw('Hello world!'); @@ -95,7 +95,7 @@ $this->assertNoCacheTag('foo'); $this->drupalGet(Url::fromRoute('early_rendering_controller_test.domain-object-with-attachments.early')); $this->assertResponse(500); - $this->assertRaw('The controller result claims to be providing relevant attachments, but leaked attachments were detected. Please ensure you are not rendering content too early. Returned object class: Drupal\early_rendering_controller_test\AttachmentsTestDomainObject.'); + $this->assertRaw('The controller result claims to be providing relevant cache metadata, but leaked metadata was detected. Please ensure you are not rendering content too early. Returned object class: Drupal\early_rendering_controller_test\AttachmentsTestDomainObject.'); $this->drupalGet(Url::fromRoute('early_rendering_controller_test.domain-object-with-attachments.early_no_cache')); $this->assertResponse(200); $this->assertRaw('AttachmentsTestDomainObject'); only in patch2: unchanged: --- a/core/lib/Drupal/Core/Cache/CacheableMetadata.php +++ b/core/lib/Drupal/Core/Cache/CacheableMetadata.php @@ -179,6 +179,15 @@ public function merge(CacheableMetadata $other) { } /** + * Checks if the current values affect cacheability. + * + * @return bool + */ + public function affectsCacheability() { + return !empty($this->contexts) || !empty($this->tags) || ($this->maxAge !== Cache::PERMANENT); + } + + /** * Applies the values of this CacheableMetadata object to a render array. * * @param array &$build only in patch2: unchanged: --- a/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php @@ -82,7 +82,7 @@ public function checkRedirectUrl(FilterResponseEvent $event) { $options['fragment'] = $destination['fragment']; // The 'Location' HTTP header must always be absolute. $options['absolute'] = TRUE; - $destination = $this->urlGenerator->generateFromPath($path, $options); + $destination = $this->urlGenerator->generateFromPath($path, $options, TRUE)->getGeneratedUrl(); } $response->setTargetUrl($destination); }