.../EarlyRenderingControllerWrapperSubscriber.php | 30 +++++++------ core/lib/Drupal/Core/Render/RendererInterface.php | 4 +- .../Tests/Common/EarlyRenderingControllerTest.php | 28 ++++++++++-- .../early_rendering_controller_test.routing.yml | 42 ++++++++++++++++++ .../early_rendering_controller_test.services.yml | 5 +++ .../src/AttachmentsTestDomainObject.php | 17 ++++++++ .../src/CacheableTestDomainObject.php | 35 +++++++++++++++ .../src/EarlyRenderingTestController.php | 30 +++++++++++++ .../src/TestDomainObject.php | 10 +++++ .../src/TestDomainObjectViewSubscriber.php | 51 ++++++++++++++++++++++ 10 files changed, 232 insertions(+), 20 deletions(-) diff --git a/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php index 5f8948f..09ea886 100644 --- a/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php @@ -7,6 +7,7 @@ namespace Drupal\Core\EventSubscriber; +use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\Cache\CacheableResponseInterface; use Drupal\Core\Controller\ControllerResolverInterface; use Drupal\Core\Render\AttachmentsInterface; @@ -20,11 +21,11 @@ /** * Subscriber that wraps controllers, to handle early rendering. * - * When controllers call drupal_render() (RendererInterface::render()), we call - * that "early rendering". Controllers should return only render arrays, but we - * cannot prevent controllers from doing early rendering. The problem with early - * rendering is that the bubbleable metadata (cacheability & attachments) are - * lost. + * When controllers call drupal_render() (RendererInterface::render()) outside + * of a render context, we call that "early rendering". Controllers should + * return only render arrays, but we cannot prevent controllers from doing early + * rendering. The problem with early rendering is that the bubbleable metadata + * (cacheability & attachments) are lost. * * This can lead to broken pages (missing assets), stale pages (missing cache * tags causing a page not to be invalidated) or even security problems (missing @@ -111,7 +112,8 @@ public function onController(FilterControllerEvent $event) { * The return value of the controller. * * @throws \LogicException - * When early rendering occurs + * When early rendering has occurred in a controller that returned a + * Response or domain object that cares about attachments or cacheability. * * @see \Symfony\Component\HttpKernel\HttpKernel::handleRaw() */ @@ -124,8 +126,8 @@ protected function wrapControllerExecutionInRenderContext($controller, array $ar }); // If early rendering happened, i.e. if code in the controller called - // drupal_render(), then the bubbleable metadata for that is stored in - // the current render context. + // drupal_render() outside of a render context, then the bubbleable metadata + // for that is stored in the current render context. if (!$context->isEmpty()) { // If a render array is returned by the controller, merge the "lost" // bubbleable metadata. @@ -135,17 +137,17 @@ protected function wrapControllerExecutionInRenderContext($controller, array $ar ->merge($early_rendering_bubbleable_metadata) ->applyTo($response); } - // If a response object is returned, and the response object cares about + // 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) { - throw new \LogicException(sprintf('Early rendering is not permitted for controllers returning anything else than render arrays. Response class: %s.', get_class($response))); + elseif (is_object($response) && ($response instanceof AttachmentsInterface || $response instanceof CacheableResponseInterface || $response instanceof CacheableDependencyInterface)) { + throw new \LogicException(sprintf('Early rendering is not permitted for controllers returning anything else than render arrays. Returned object class: %s.', get_class($response))); } else { - // A response object is returned that does not care about attachments - // nor cacheability. E.g. a RedirectResponse. It is safe to discard any - // early rendering metadata. + // 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 --git a/core/lib/Drupal/Core/Render/RendererInterface.php b/core/lib/Drupal/Core/Render/RendererInterface.php index 394c355..b55966f 100644 --- a/core/lib/Drupal/Core/Render/RendererInterface.php +++ b/core/lib/Drupal/Core/Render/RendererInterface.php @@ -327,8 +327,8 @@ public function render(&$elements, $is_root_call = FALSE); * Only for very advanced use cases. Prefer using ::renderRoot() and * ::renderPlain() instead. * - * All rendering must happen within a render context: within a render context, - * all bubbleable metadata is bubbled and hence tracked, without a render + * All rendering must happen within a render context. Within a render context, + * all bubbleable metadata is bubbled and hence tracked. Outside of a render * context, it would be lost. This could lead to missing assets, incorrect * cache variations (and thus security issues), insufficient cache * invalidations, and so on. diff --git a/core/modules/system/src/Tests/Common/EarlyRenderingControllerTest.php b/core/modules/system/src/Tests/Common/EarlyRenderingControllerTest.php index d9d11d7..27bc664 100644 --- a/core/modules/system/src/Tests/Common/EarlyRenderingControllerTest.php +++ b/core/modules/system/src/Tests/Common/EarlyRenderingControllerTest.php @@ -52,17 +52,37 @@ function testEarlyRendering() { $this->drupalGet(Url::fromRoute('early_rendering_controller_test.response-with-attachments')); $this->assertResponse(200, 'Hello world!'); $this->assertNoCacheTag('foo'); - $this->config('system.logging')->set('error_level', ERROR_REPORTING_DISPLAY_VERBOSE)->save(); $this->drupalGet(Url::fromRoute('early_rendering_controller_test.response-with-attachments.early')); - $this->assertResponse(500, 'Early rendering is not permitted for controllers returning anything else than render arrays. Response class: Drupal\early_rendering_controller_test\AttachmentsTestResponse.'); + $this->assertResponse(500, 'Early rendering is not permitted for controllers returning anything else than render arrays. Returned object class: Drupal\early_rendering_controller_test\AttachmentsTestResponse.'); // Cacheable Response object: non-early & early. $this->drupalGet(Url::fromRoute('early_rendering_controller_test.cacheable-response')); $this->assertResponse(200, 'Hello world!'); $this->assertNoCacheTag('foo'); - $this->config('system.logging')->set('error_level', ERROR_REPORTING_DISPLAY_VERBOSE)->save(); $this->drupalGet(Url::fromRoute('early_rendering_controller_test.cacheable-response.early')); - $this->assertResponse(500, 'Early rendering is not permitted for controllers returning anything else than render arrays. Response class: Drupal\early_rendering_controller_test\AttachmentsTestResponse.'); + $this->assertResponse(500, 'Early rendering is not permitted for controllers returning anything else than render arrays. Returned object class: Drupal\early_rendering_controller_test\AttachmentsTestResponse.'); + + // Basic domain object: non-early & early. + $this->drupalGet(Url::fromRoute('early_rendering_controller_test.domain-object')); + $this->assertResponse(200, 'TestDomainObject'); + $this->assertNoCacheTag('foo'); + $this->drupalGet(Url::fromRoute('early_rendering_controller_test.domain-object.early')); + $this->assertResponse(200, 'TestDomainObject'); + $this->assertNoCacheTag('foo'); + + // Basic domain object with attachments: non-early & early. + $this->drupalGet(Url::fromRoute('early_rendering_controller_test.domain-object-with-attachments')); + $this->assertResponse(200, 'AttachmentsTestDomainObject'); + $this->assertNoCacheTag('foo'); + $this->drupalGet(Url::fromRoute('early_rendering_controller_test.domain-object-with-attachments.early')); + $this->assertResponse(500, 'Early rendering is not permitted for controllers returning anything else than render arrays. Returned object class: Drupal\early_rendering_controller_test\AttachmentsTestDomainObject.'); + + // Cacheable Response object: non-early & early. + $this->drupalGet(Url::fromRoute('early_rendering_controller_test.cacheable-domain-object')); + $this->assertResponse(200, 'CacheableTestDomainObject'); + $this->assertNoCacheTag('foo'); + $this->drupalGet(Url::fromRoute('early_rendering_controller_test.cacheable-domain-object.early')); + $this->assertResponse(500, 'Early rendering is not permitted for controllers returning anything else than render arrays. Returned object class: Drupal\early_rendering_controller_test\CacheableTestDomainObject.'); // The exceptions are expected. Do not interpret them as a test failure. // Not using File API; a potential error must trigger a PHP warning. diff --git a/core/modules/system/tests/modules/early_rendering_controller_test/early_rendering_controller_test.routing.yml b/core/modules/system/tests/modules/early_rendering_controller_test/early_rendering_controller_test.routing.yml index 98d70c3..4e050e4 100644 --- a/core/modules/system/tests/modules/early_rendering_controller_test/early_rendering_controller_test.routing.yml +++ b/core/modules/system/tests/modules/early_rendering_controller_test/early_rendering_controller_test.routing.yml @@ -53,3 +53,45 @@ early_rendering_controller_test.cacheable-response.early: _controller: '\Drupal\early_rendering_controller_test\EarlyRenderingTestController::cacheableResponseEarly' requirements: _access: 'TRUE' + +# Controller returning a basic domain object. +early_rendering_controller_test.domain-object: + path: '/early-rendering-controller-test/domain-object' + defaults: + _controller: '\Drupal\early_rendering_controller_test\EarlyRenderingTestController::domainObject' + requirements: + _access: 'TRUE' +early_rendering_controller_test.domain-object.early: + path: '/early-rendering-controller-test/domain-object/early' + defaults: + _controller: '\Drupal\early_rendering_controller_test\EarlyRenderingTestController::domainObjectEarly' + requirements: + _access: 'TRUE' + +# Controller returning a domain object with attachments. +early_rendering_controller_test.domain-object-with-attachments: + path: '/early-rendering-controller-test/domain-object-with-attachments' + defaults: + _controller: '\Drupal\early_rendering_controller_test\EarlyRenderingTestController::domainObjectWithAttachments' + requirements: + _access: 'TRUE' +early_rendering_controller_test.domain-object-with-attachments.early: + path: '/early-rendering-controller-test/domain-object-with-attachments/early' + defaults: + _controller: '\Drupal\early_rendering_controller_test\EarlyRenderingTestController::domainObjectWithAttachmentsEarly' + requirements: + _access: 'TRUE' + +# Controller returning a cacheable domain object. +early_rendering_controller_test.cacheable-domain-object: + path: '/early-rendering-controller-test/cacheable-domain-object' + defaults: + _controller: '\Drupal\early_rendering_controller_test\EarlyRenderingTestController::cacheableDomainObject' + requirements: + _access: 'TRUE' +early_rendering_controller_test.cacheable-domain-object.early: + path: '/early-rendering-controller-test/cacheable-domain-object/early' + defaults: + _controller: '\Drupal\early_rendering_controller_test\EarlyRenderingTestController::cacheableDomainObjectEarly' + requirements: + _access: 'TRUE' diff --git a/core/modules/system/tests/modules/early_rendering_controller_test/early_rendering_controller_test.services.yml b/core/modules/system/tests/modules/early_rendering_controller_test/early_rendering_controller_test.services.yml new file mode 100644 index 0000000..8c62f1a --- /dev/null +++ b/core/modules/system/tests/modules/early_rendering_controller_test/early_rendering_controller_test.services.yml @@ -0,0 +1,5 @@ +services: + test_domain_object.view_subscriber: + class: Drupal\early_rendering_controller_test\TestDomainObjectViewSubscriber + tags: + - { name: event_subscriber } diff --git a/core/modules/system/tests/modules/early_rendering_controller_test/src/AttachmentsTestDomainObject.php b/core/modules/system/tests/modules/early_rendering_controller_test/src/AttachmentsTestDomainObject.php new file mode 100644 index 0000000..e6b6a4f --- /dev/null +++ b/core/modules/system/tests/modules/early_rendering_controller_test/src/AttachmentsTestDomainObject.php @@ -0,0 +1,17 @@ +renderer->render($render_array)); } + public function domainObject() { + return new TestDomainObject(); + } + + public function domainObjectEarly() { + $render_array = $this->earlyRenderContent(); + $this->renderer->render($render_array); + return new TestDomainObject(); + } + + public function domainObjectWithAttachments() { + return new AttachmentsTestDomainObject(); + } + + public function domainObjectWithAttachmentsEarly() { + $render_array = $this->earlyRenderContent(); + $this->renderer->render($render_array); + return new AttachmentsTestDomainObject(); + } + + public function cacheableDomainObject() { + return new CacheableTestDomainObject(); + } + + public function cacheableDomainObjectEarly() { + $render_array = $this->earlyRenderContent(); + $this->renderer->render($render_array); + return new CacheableTestDomainObject(); + } + } diff --git a/core/modules/system/tests/modules/early_rendering_controller_test/src/TestDomainObject.php b/core/modules/system/tests/modules/early_rendering_controller_test/src/TestDomainObject.php new file mode 100644 index 0000000..b2f8f8b --- /dev/null +++ b/core/modules/system/tests/modules/early_rendering_controller_test/src/TestDomainObject.php @@ -0,0 +1,10 @@ +getControllerResult(); + + if ($result instanceof TestDomainObject) { + if ($result instanceof AttachmentsTestDomainObject) { + $event->setResponse(new AttachmentsTestResponse('AttachmentsTestDomainObject')); + } + elseif ($result instanceof CacheableTestDomainObject) { + $event->setResponse(new CacheableTestResponse('CacheableTestDomainObject')); + } + else { + $event->setResponse(new Response('TestDomainObject')); + } + } + } + + /** + * {@inheritdoc} + */ + static function getSubscribedEvents() { + $events[KernelEvents::VIEW][] = ['onViewTestDomainObject']; + + return $events; + } + +}