diff --git a/src/EventSubscriber/CustomElementsFormatSubscriber.php b/src/EventSubscriber/CustomElementsFormatSubscriber.php index ef49c50d14ab1a3b8a2815130540938aab3c6106..74fd2e24a33babd36adabd3c1e18e9a15537d1f6 100644 --- a/src/EventSubscriber/CustomElementsFormatSubscriber.php +++ b/src/EventSubscriber/CustomElementsFormatSubscriber.php @@ -2,6 +2,7 @@ namespace Drupal\lupus_ce_renderer\EventSubscriber; +use Drupal\Core\Render\HtmlResponse; use Drupal\Core\Site\Settings; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpFoundation\RedirectResponse; @@ -29,15 +30,12 @@ class CustomElementsFormatSubscriber implements EventSubscriberInterface { $response = $event->getResponse(); $request = $event->getRequest(); - // Dis-allow html responses when custom elements rendering is enabled. + // Dis-allow HTML responses when custom elements rendering is enabled. // Other formats like 'json' are kept allowed for backwards compatibility. - // @todo Make this alterable somehow. - // Block responses that would result in HTML output. The format check is - // sufficient since CustomElementsRequestFormatRouteFilter sets the correct - // format. Checking Content-Type is avoided as REST ResourceResponse objects - // have no Content-Type set yet when this subscriber runs (priority 500), - // before ResourceResponseSubscriber (priority 0) prepares them. - $is_html = $request->getRequestFormat('html') === 'html'; + // Also block HtmlResponse objects directly, since some controllers return + // them regardless of the request format. We cannot check for the + // 'Content-Type' header since it gets set later. + $is_html = $request->getRequestFormat('html') === 'html' || $response instanceof HtmlResponse; if ($request->attributes->get('lupus_ce_renderer') && $is_html && !$response instanceof RedirectResponse) { // Throw status 406 http exception, but take care to not run in an // end-less loop here. diff --git a/tests/src/Kernel/LupusCeRendererRequestsTest.php b/tests/src/Kernel/LupusCeRendererRequestsTest.php index 26c585048de44ac3e7147f9c70e1de227c103c14..ff7ed5ce6780106adcae4fbf3a97b4ac4c8124d7 100644 --- a/tests/src/Kernel/LupusCeRendererRequestsTest.php +++ b/tests/src/Kernel/LupusCeRendererRequestsTest.php @@ -172,23 +172,22 @@ class LupusCeRendererRequestsTest extends LupusCeRendererKernelTestBase { } /** - * Tests REST responses with lupus_ce_renderer active are not rejected. + * Tests that REST responses with lupus_ce_renderer active are not rejected. * - * Regression test: lupus_ce_renderer 2.x briefly added - * || !$response->headers->has('Content-Type') to the $is_html check in - * CustomElementsFormatSubscriber. This caused REST ResourceResponse objects - * to receive a 406 when lupus_ce_renderer was active, because - * ResourceResponseSubscriber (priority 0) sets Content-Type after this - * subscriber runs (priority 500). + * REST ResourceResponse objects (e.g. menu endpoints) have no Content-Type + * set yet when this subscriber runs at priority 500 — + * ResourceResponseSubscriber (priority 0) sets it later. We must not reject + * them based on a missing + * Content-Type. Instead we rely on the request format and instanceof + * HtmlResponse. */ public function testRestResponseWithLupusCeRendererNotRejected() { $request = Request::create('/api/menu_items/main', 'GET'); $request->setRequestFormat('json'); $request->attributes->set('lupus_ce_renderer', TRUE); - // Simulate a ResourceResponse without Content-Type, as it appears when - // CustomElementsFormatSubscriber runs at priority 500, before - // ResourceResponseSubscriber sets it at priority 0. + // Simulate a response with no Content-Type, as REST ResourceResponse + // looks when this subscriber runs before ResourceResponseSubscriber. $response = new Response('{"items":[]}', 200, []); $response->headers->remove('Content-Type'); @@ -196,7 +195,7 @@ class LupusCeRendererRequestsTest extends LupusCeRendererKernelTestBase { $event = new ResponseEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST, $response); $subscriber = $this->container->get('lupus_ce_renderer.custom_elements_event_subscriber'); - // Must not throw 406 — format is 'json', not 'html'. + // Must not throw 406 — format is 'json' and response is not HtmlResponse. $subscriber->onKernelResponse($event); $this->addToAssertionCount(1); }