Issue #3410022 by heddn, larowlan, catch: Regression from #3295790...

Issue #3410022 by heddn, larowlan, catch: Regression from #3295790 content-length header set earlier than expected

(cherry picked from commit 8f087227)

core/core.services.yml

@@ -891,6 +891,11 @@ services:
   argument_resolver.default:
     class: Symfony\Component\HttpKernel\Controller\ArgumentResolver\DefaultValueResolver
     public: false
+  http_middleware.content_length:
+    class: Drupal\Core\StackMiddleware\ContentLength
+    tags:
+      # Must run before the page_cache and big_pipe middleware.
+      - { name: http_middleware, priority: 140 }
   http_middleware.ajax_page_state:
     class: Drupal\Core\StackMiddleware\AjaxPageState
     tags:

core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php

@@ -300,40 +300,6 @@ protected function setExpiresNoCache(Response $response) {
     $response->setExpires(\DateTime::createFromFormat('j-M-Y H:i:s T', '19-Nov-1978 05:00:00 UTC'));
   }
 
-  /**
-   * Sets the Content-Length header on the response.
-   *
-   * @param \Symfony\Component\HttpKernel\Event\ResponseEvent $event
-   *   The event to process.
-   *
-   * @see \Symfony\Component\HttpFoundation\Response::prepare()
-   * @see https://www.rfc-editor.org/rfc/rfc9110.html#name-content-length
-   */
-  public function setContentLengthHeader(ResponseEvent $event): void {
-    $response = $event->getResponse();
-
-    if ($response->isInformational() || $response->isEmpty()) {
-      return;
-    }
-
-    if ($response->headers->has('Transfer-Encoding')) {
-      return;
-    }
-
-    // Drupal cannot set the correct content length header when there is a
-    // server error.
-    if ($response->isServerError()) {
-      return;
-    }
-
-    $content = $response->getContent();
-    if ($content === FALSE) {
-      return;
-    }
-
-    $response->headers->set('Content-Length', strlen($content), TRUE);
-  }
-
   /**
    * Registers the methods in this class that should be listeners.
    *
@@ -345,10 +311,6 @@ public static function getSubscribedEvents(): array {
     // There is no specific reason for choosing 16 beside it should be executed
     // before ::onRespond().
     $events[KernelEvents::RESPONSE][] = ['onAllResponds', 16];
-    // Run very late, after all other response subscribers have run. However,
-    // any response subscribers that convert a response to a streamed response
-    // must run after this and undo what this does.
-    $events[KernelEvents::RESPONSE][] = ['setContentLengthHeader', -1024];
     return $events;
   }
 

core/lib/Drupal/Core/StackMiddleware/ContentLength.php

+<?php
+
+namespace Drupal\Core\StackMiddleware;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+
+/**
+ * Adds a Content-Length HTTP header to responses.
+ */
+class ContentLength implements HttpKernelInterface {
+
+  /**
+   * Constructs a new ContentLength instance.
+   *
+   * @param \Symfony\Component\HttpKernel\HttpKernelInterface $httpKernel
+   *   The wrapped HTTP kernel.
+   */
+  public function __construct(
+    protected readonly HttpKernelInterface $httpKernel,
+  ) {}
+
+  /**
+   * {@inheritdoc}
+   */
+  public function handle(Request $request, $type = self::MAIN_REQUEST, $catch = TRUE): Response {
+    $response = $this->httpKernel->handle($request, $type, $catch);
+    if ($response->isInformational() || $response->isEmpty()) {
+      return $response;
+    }
+
+    if ($response->headers->has('Transfer-Encoding')) {
+      return $response;
+    }
+
+    // Drupal cannot set the correct content length header when there is a
+    // server error.
+    if ($response->isServerError()) {
+      return $response;
+    }
+
+    $content = $response->getContent();
+    if ($content === FALSE) {
+      return $response;
+    }
+
+    $response->headers->set('Content-Length', strlen($content), TRUE);
+    return $response;
+  }
+
+}

core/modules/big_pipe/big_pipe.services.yml

@@ -23,3 +23,8 @@ services:
     class: Drupal\big_pipe\EventSubscriber\NoBigPipeRouteAlterSubscriber
     tags:
       - { name: event_subscriber }
+  http_middleware.big_pipe:
+    class: \Drupal\big_pipe\StackMiddleware\ContentLength
+    tags:
+      # Must run after the content_length middleware.
+      - { name: http_middleware, priority: 150 }

core/modules/big_pipe/src/EventSubscriber/HtmlResponseBigPipeSubscriber.php

@@ -82,8 +82,6 @@ public function onRespond(ResponseEvent $event) {
       $content = $response->getContent();
       $content = str_replace('<drupal-big-pipe-scripts-bottom-marker>', '', $content);
       $response->setContent($content);
-      // FinishResponseSubscriber::setContentLengthHeader() already ran.
-      $response->headers->set('Content-Length', strlen($content), TRUE);
     }
 
     // If there are neither BigPipe placeholders nor no-JS BigPipe placeholders,
@@ -96,9 +94,6 @@ public function onRespond(ResponseEvent $event) {
     $big_pipe_response = new BigPipeResponse($response);
     $big_pipe_response->setBigPipeService($this->getBigPipeService($event));
 
-    // A BigPipe response's length is impossible to predict.
-    $big_pipe_response->headers->remove('Content-Length');
-
     $event->setResponse($big_pipe_response);
   }
 

core/modules/big_pipe/src/StackMiddleware/ContentLength.php

+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\big_pipe\StackMiddleware;
+
+use Drupal\big_pipe\Render\BigPipeResponse;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+
+/**
+ * Defines a big pipe middleware that removes Content-Length headers.
+ */
+final class ContentLength implements HttpKernelInterface {
+
+  /**
+   * Constructs a new ContentLength instance.
+   *
+   * @param \Symfony\Component\HttpKernel\HttpKernelInterface $httpKernel
+   *   The wrapped HTTP kernel.
+   */
+  public function __construct(
+    protected readonly HttpKernelInterface $httpKernel,
+  ) {
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function handle(Request $request, $type = self::MAIN_REQUEST, $catch = TRUE): Response {
+    $response = $this->httpKernel->handle($request, $type, $catch);
+    if (!$response instanceof BigPipeResponse) {
+      return $response;
+    }
+    $response->headers->remove('Content-Length');
+    return $response;
+  }
+
+}

core/modules/big_pipe/tests/src/Unit/StackMiddleware/ContentLengthTest.php

+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\Tests\big_pipe\Unit\StackMiddleware;
+
+use Drupal\big_pipe\Render\BigPipeResponse;
+use Drupal\big_pipe\StackMiddleware\ContentLength;
+use Drupal\Core\Render\HtmlResponse;
+use Drupal\Tests\UnitTestCase;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+
+/**
+ * Defines a test for ContentLength middleware.
+ *
+ * @group big_pipe
+ * @coversDefaultClass \Drupal\big_pipe\StackMiddleware\ContentLength
+ */
+final class ContentLengthTest extends UnitTestCase {
+
+  /**
+   * @covers ::handle
+   * @dataProvider providerTestSetContentLengthHeader
+   */
+  public function testHandle(false|int $expected_header, Response $response) {
+    $kernel = $this->prophesize(HttpKernelInterface::class);
+    $request = Request::create('/');
+    $kernel->handle($request, HttpKernelInterface::MAIN_REQUEST, TRUE)->willReturn($response);
+    $middleware = new ContentLength($kernel->reveal());
+    $response = $middleware->handle($request);
+    if ($expected_header === FALSE) {
+      $this->assertFalse($response->headers->has('Content-Length'));
+      return;
+    }
+    $this->assertSame((string) $expected_header, $response->headers->get('Content-Length'));
+  }
+
+  public function providerTestSetContentLengthHeader() {
+    $response = new Response('Test content', 200);
+    $response->headers->set('Content-Length', (string) strlen('Test content'));
+    return [
+      '200 ok' => [
+        12,
+        $response,
+      ],
+      'Big pipe' => [
+        FALSE,
+        new BigPipeResponse(new HtmlResponse('Test content')),
+      ],
+    ];
+  }
+
+}

core/modules/system/tests/modules/http_middleware_test/http_middleware_test.info.yml

+name: 'Test HTTP Middleware'
+type: module
+description: 'Provides a test http middleware for automated tests.'
+package: Testing
+version: VERSION

core/modules/system/tests/modules/http_middleware_test/http_middleware_test.routing.yml

+http_middleware_test.test_response:
+  path: '/test-response'
+  defaults:
+    _title: 'Test response'
+    _controller: '\Drupal\http_middleware_test\Controller\TestResponseController::testResponse'
+  requirements:
+    _access: 'TRUE'

core/modules/system/tests/modules/http_middleware_test/http_middleware_test.services.yml

+services:
+  http_middleware.alter_content_middleware:
+    class: Drupal\http_middleware_test\StackMiddleware\AlterContentMiddleware
+    tags:
+      - { name: http_middleware, priority: 100, responder: true }

core/modules/system/tests/modules/http_middleware_test/src/Controller/TestResponseController.php

+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\http_middleware_test\Controller;
+
+use Symfony\Component\HttpFoundation\Response;
+
+/**
+ * Controller routines for http_middleware_test routes.
+ */
+final class TestResponseController {
+
+  /**
+   * Returns a test response.
+   */
+  public function testResponse(): Response {
+    return new Response('<html><body><p>Mangoes</p></body></html>');
+  }
+
+}

core/modules/system/tests/modules/http_middleware_test/src/StackMiddleware/AlterContentMiddleware.php

+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\http_middleware_test\StackMiddleware;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+
+/**
+ * Alters the response before content length is calculated.
+ */
+final class AlterContentMiddleware implements HttpKernelInterface {
+
+  public function __construct(
+    private readonly HttpKernelInterface $httpKernel,
+  ) {}
+
+  /**
+   * {@inheritdoc}
+   */
+  public function handle(Request $request, int $type = self::MAIN_REQUEST, bool $catch = TRUE): Response {
+    $response = $this->httpKernel->handle($request, $type, $catch);
+    if (\Drupal::getContainer()->hasParameter('no-alter-content-length') && \Drupal::getContainer()->getParameter('no-alter-content-length')) {
+      $response->setContent('<html><body><p>Avocados</p></body></html>');
+    }
+    return $response;
+  }
+
+}

core/tests/Drupal/FunctionalTests/HttpKernel/ContentLengthTest.php

+<?php
+
+namespace Drupal\FunctionalTests\HttpKernel;
+
+use Drupal\Core\Url;
+use Drupal\Tests\BrowserTestBase;
+
+/**
+ * Tests Content-Length set by Drupal.
+ *
+ * @group Http
+ */
+class ContentLengthTest extends BrowserTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = ['system', 'http_middleware_test'];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $defaultTheme = 'stark';
+
+  public function testContentLength(): void {
+    // Fire off a request.
+    $this->drupalGet(Url::fromRoute('http_middleware_test.test_response'));
+    $this->assertSession()->statusCodeEquals(200);
+    $this->assertSession()->responseHeaderEquals('Content-Length', '40');
+
+    $this->setContainerParameter('no-alter-content-length', TRUE);
+    $this->rebuildContainer();
+
+    // Fire the same exact request but this time length is different.
+    $this->drupalGet(Url::fromRoute('http_middleware_test.test_response'));
+    $this->assertSession()->statusCodeEquals(200);
+    $this->assertSession()->responseHeaderEquals('Content-Length', '41');
+  }
+
+}

core/tests/Drupal/Tests/Core/EventSubscriber/FinishResponserSubscriberTest.php → core/tests/Drupal/Tests/Core/StackMiddleware/ContentLengthTest.php

 <?php
 
-namespace Drupal\Tests\Core\EventSubscriber;
+declare(strict_types=1);
 
-use Drupal\Core\Cache\Context\CacheContextsManager;
-use Drupal\Core\EventSubscriber\FinishResponseSubscriber;
-use Drupal\Core\Language\LanguageManagerInterface;
-use Drupal\Core\PageCache\RequestPolicyInterface;
-use Drupal\Core\PageCache\ResponsePolicyInterface;
+namespace Drupal\Tests\Core\StackMiddleware;
+
+use Drupal\Core\StackMiddleware\ContentLength;
 use Drupal\Tests\UnitTestCase;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpFoundation\StreamedResponse;
-use Symfony\Component\HttpKernel\Event\ResponseEvent;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
 
 /**
- * @coversDefaultClass \Drupal\Core\EventSubscriber\FinishResponseSubscriber
- * @group EventSubscriber
+ * @coversDefaultClass \Drupal\Core\StackMiddleware\ContentLength
+ * @group Middleware
  */
-class FinishResponserSubscriberTest extends UnitTestCase {
+class ContentLengthTest extends UnitTestCase {
 
   /**
-   * @covers ::setContentLengthHeader
+   * @covers ::handle
    * @dataProvider providerTestSetContentLengthHeader
    */
-  public function testSetContentLengthHeader(false|int $expected_header, Response $response) {
-    $event_subscriber = new FinishResponseSubscriber(
-      $this->prophesize(LanguageManagerInterface::class)->reveal(),
-      $this->getConfigFactoryStub(),
-      $this->prophesize(RequestPolicyInterface::class)->reveal(),
-      $this->prophesize(ResponsePolicyInterface::class)->reveal(),
-      $this->prophesize(CacheContextsManager::class)->reveal()
-    );
-
-    $event = new ResponseEvent(
-      $this->prophesize(HttpKernelInterface::class)->reveal(),
-      $this->prophesize(Request::class)->reveal(),
-      HttpKernelInterface::MAIN_REQUEST,
-      $response
-    );
-
-    $event_subscriber->setContentLengthHeader($event);
+  public function testHandle(false|int $expected_header, Response $response) {
+    $kernel = $this->prophesize(HttpKernelInterface::class);
+    $request = Request::create('/');
+    $kernel->handle($request, HttpKernelInterface::MAIN_REQUEST, TRUE)->willReturn($response);
+    $middleware = new ContentLength($kernel->reveal());
+    $response = $middleware->handle($request);
     if ($expected_header === FALSE) {
-      $this->assertFalse($event->getResponse()->headers->has('Content-Length'));
-    }
-    else {
-      $this->assertSame((string) $expected_header, $event->getResponse()->headers->get('Content-Length'));
+      $this->assertFalse($response->headers->has('Content-Length'));
+      return;
     }
+    $this->assertSame((string) $expected_header, $response->headers->get('Content-Length'));
   }
 
   public function providerTestSetContentLengthHeader() {