Commit c4690891 authored by catch's avatar catch
Browse files

Issue #3295790 by bradjones1, mstrelan, rpayanm, catch, alexpott, acbramley,... 

Issue #3295790 by bradjones1, mstrelan, rpayanm, catch, alexpott, acbramley, longwave, kim.pepper, mfb: Post-response task running (destructable services) are actually blocking; add test coverage and warn for common misconfiguration
parent 5c126061

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

+20 −0
Original line number Diff line number Diff line
@@ -12,6 +12,7 @@ 
use Drupal\Core\Site\Settings;

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\KernelEvents;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;
@@ -298,6 +299,21 @@ 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.

   */

  public function setContentLengthHeader(ResponseEvent $event): void {

    $response = $event->getResponse();

    if ($response instanceof StreamedResponse) {

      return;

    }



    $response->headers->set('Content-Length', strlen($response->getContent()), TRUE);

  }



  /**

   * Registers the methods in this class that should be listeners.

   *
@@ -309,6 +325,10 @@ 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/misc/cspell/dictionary.txt

+1 −0
Original line number Diff line number Diff line
@@ -625,6 +625,7 @@ linkset 
linktext

lisu

litererally

litespeed

llamaids

llamasarelame

llamma

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

+6 −0
Original line number Diff line number Diff line
@@ -82,6 +82,8 @@ 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,
@@ -93,6 +95,10 @@ 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/jsonapi/tests/src/Functional/JsonApiRegressionTest.php

+11 −7
Original line number Diff line number Diff line
@@ -1300,10 +1300,13 @@ public function testNonCacheableMethods() { 
    $methods = [

      'HEAD',

      'GET',

      'PATCH',

      'DELETE',

    ];

    $non_post_request_options = $base_request_options + [

    foreach ($methods as $method) {

      $response = $this->request($method, Url::fromUri('internal:/jsonapi/node/article/' . $node->uuid()), $base_request_options);

      $this->assertSame(200, $response->getStatusCode());

    }



    $patch_request_options = $base_request_options + [

      RequestOptions::JSON => [

        'data' => [

          'type' => 'node--article',
@@ -1311,10 +1314,11 @@ public function testNonCacheableMethods() { 
        ],

      ],

    ];

    foreach ($methods as $method) {

      $response = $this->request($method, Url::fromUri('internal:/jsonapi/node/article/' . $node->uuid()), $non_post_request_options);

      $this->assertSame($method === 'DELETE' ? 204 : 200, $response->getStatusCode());

    }

    $response = $this->request('PATCH', Url::fromUri('internal:/jsonapi/node/article/' . $node->uuid()), $patch_request_options);

    $this->assertSame(200, $response->getStatusCode());



    $response = $this->request('DELETE', Url::fromUri('internal:/jsonapi/node/article/' . $node->uuid()), $base_request_options);

    $this->assertSame(204, $response->getStatusCode());



    $post_request_options = $base_request_options + [

      RequestOptions::JSON => [

core/modules/jsonapi/tests/src/Functional/NodeTest.php

+4 −0
Original line number Diff line number Diff line
@@ -393,11 +393,15 @@ protected function assertCacheableNormalizations(): void { 
    $request_options = $this->getAuthenticationRequestOptions();

    $request_options[RequestOptions::QUERY] = ['fields' => ['node--camelids' => 'title']];

    $this->request('GET', $url, $request_options);

    // Cacheable normalizations are written after the response is flushed to

    // the client; give the server a chance to complete this work.

    sleep(1);

    // Ensure the normalization cache is being incrementally built. After

    // requesting the title, only the title is in the cache.

    $this->assertNormalizedFieldsAreCached(['title']);

    $request_options[RequestOptions::QUERY] = ['fields' => ['node--camelids' => 'field_rest_test']];

    $this->request('GET', $url, $request_options);

    sleep(1);

    // After requesting an additional field, then that field is in the cache and

    // the old one is still there.

    $this->assertNormalizedFieldsAreCached(['title', 'field_rest_test']);