diff --git a/src/EventListener/MergeHttpHeadersListener.php b/src/EventListener/MergeHttpHeadersListener.php index d0ab7ce5c3..8858740faf 100644 --- a/src/EventListener/MergeHttpHeadersListener.php +++ b/src/EventListener/MergeHttpHeadersListener.php @@ -11,6 +11,8 @@ namespace Contao\CoreBundle\EventListener; use Contao\CoreBundle\Framework\ContaoFrameworkInterface; +use Contao\CoreBundle\HttpKernel\Header\HeaderStorageInterface; +use Contao\CoreBundle\HttpKernel\Header\NativeHeaderStorage; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\FilterResponseEvent; @@ -18,6 +20,7 @@ * Adds HTTP headers sent by Contao to the Symfony response. * * @author Yanick Witschi + * @author Andreas Schempp */ class MergeHttpHeadersListener { @@ -27,9 +30,14 @@ class MergeHttpHeadersListener private $framework; /** - * @var array|null + * @var HeaderStorageInterface */ - private $headers; + private $headerStorage; + + /** + * @var array + */ + private $headers = []; /** * @var array @@ -45,13 +53,13 @@ class MergeHttpHeadersListener /** * Constructor. * - * @param ContaoFrameworkInterface $framework - * @param array|null $headers Meant for unit testing only! + * @param ContaoFrameworkInterface $framework + * @param HeaderStorageInterface|null $headerStorage */ - public function __construct(ContaoFrameworkInterface $framework, array $headers = null) + public function __construct(ContaoFrameworkInterface $framework, HeaderStorageInterface $headerStorage = null) { $this->framework = $framework; - $this->headers = $headers; + $this->headerStorage = $headerStorage ?: new NativeHeaderStorage(); } /** @@ -111,45 +119,41 @@ public function onKernelResponse(FilterResponseEvent $event) return; } - $event->setResponse($this->mergeHttpHeaders($event->getResponse())); + // Fetch remaining headers and add them to the response + $this->fetchHttpHeaders(); + $this->setResponseHeaders($event->getResponse()); } /** - * Merges the HTTP headers. + * Fetches and stores HTTP headers from PHP. + */ + private function fetchHttpHeaders() + { + $this->headers = array_merge($this->headers, $this->headerStorage->all()); + $this->headerStorage->clear(); + } + + /** + * Sets the response headers. * * @param Response $response - * - * @return Response */ - private function mergeHttpHeaders(Response $response) + private function setResponseHeaders(Response $response) { - foreach ($this->getHeaders() as $header) { - list($name, $content) = explode(':', $header, 2); + $allowOverrides = []; - if ('cli' !== PHP_SAPI && !headers_sent()) { - header_remove($name); - } + foreach ($this->headers as $header) { + list($name, $content) = explode(':', $header, 2); $uniqueKey = $this->getUniqueKey($name); if (in_array($uniqueKey, $this->multiHeaders, true)) { $response->headers->set($uniqueKey, trim($content), false); - } elseif (!$response->headers->has($uniqueKey)) { + } elseif (isset($allowOverrides[$uniqueKey]) || !$response->headers->has($uniqueKey)) { + $allowOverrides[$uniqueKey] = true; $response->headers->set($uniqueKey, trim($content)); } } - - return $response; - } - - /** - * Returns the headers. - * - * @return array - */ - private function getHeaders() - { - return $this->headers ?: headers_list(); } /** diff --git a/src/HttpKernel/Header/HeaderStorageInterface.php b/src/HttpKernel/Header/HeaderStorageInterface.php new file mode 100644 index 0000000000..a2bc183099 --- /dev/null +++ b/src/HttpKernel/Header/HeaderStorageInterface.php @@ -0,0 +1,38 @@ + + */ +interface HeaderStorageInterface +{ + /** + * Returns all headers. + * + * @return array + */ + public function all(); + + /** + * Adds a header to the storage. + * + * @param string $header + */ + public function add($header); + + /** + * Clears the storage. + */ + public function clear(); +} diff --git a/src/HttpKernel/Header/MemoryHeaderStorage.php b/src/HttpKernel/Header/MemoryHeaderStorage.php new file mode 100644 index 0000000000..6a8e8ebef9 --- /dev/null +++ b/src/HttpKernel/Header/MemoryHeaderStorage.php @@ -0,0 +1,58 @@ + + */ +class MemoryHeaderStorage implements HeaderStorageInterface +{ + /** + * @var array + */ + private $headers = []; + + /** + * Constructor. + * + * @param array $headers + */ + public function __construct(array $headers = []) + { + $this->headers = $headers; + } + + /** + * {@inheritdoc} + */ + public function all() + { + return $this->headers; + } + + /** + * {@inheritdoc} + */ + public function add($header) + { + $this->headers[] = $header; + } + + /** + * {@inheritdoc} + */ + public function clear() + { + $this->headers = []; + } +} diff --git a/src/HttpKernel/Header/NativeHeaderStorage.php b/src/HttpKernel/Header/NativeHeaderStorage.php new file mode 100644 index 0000000000..b7e930e29c --- /dev/null +++ b/src/HttpKernel/Header/NativeHeaderStorage.php @@ -0,0 +1,45 @@ + + */ +class NativeHeaderStorage implements HeaderStorageInterface +{ + /** + * {@inheritdoc} + */ + public function all() + { + return headers_list(); + } + + /** + * {@inheritdoc} + */ + public function add($header) + { + header($header); + } + + /** + * {@inheritdoc} + */ + public function clear() + { + if ('cli' !== PHP_SAPI && !headers_sent()) { + header_remove(); + } + } +} diff --git a/tests/EventListener/MergeHttpHeadersListenerTest.php b/tests/EventListener/MergeHttpHeadersListenerTest.php index 5bfe59c193..645315d06e 100644 --- a/tests/EventListener/MergeHttpHeadersListenerTest.php +++ b/tests/EventListener/MergeHttpHeadersListenerTest.php @@ -12,6 +12,7 @@ use Contao\CoreBundle\EventListener\MergeHttpHeadersListener; use Contao\CoreBundle\Framework\ContaoFrameworkInterface; +use Contao\CoreBundle\HttpKernel\Header\MemoryHeaderStorage; use Contao\CoreBundle\Tests\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -37,7 +38,7 @@ public function testCanBeInstantiated() } /** - * Tests that the headers sent using header() are merged into the response object. + * Tests that the headers are merged into the response object. */ public function testMergesTheHeadersSent() { @@ -56,7 +57,7 @@ public function testMergesTheHeadersSent() ->willReturn(true) ; - $listener = new MergeHttpHeadersListener($framework, ['Content-Type: text/html']); + $listener = new MergeHttpHeadersListener($framework, new MemoryHeaderStorage(['Content-Type: text/html'])); $listener->onKernelResponse($responseEvent); $response = $responseEvent->getResponse(); @@ -85,14 +86,14 @@ public function testDoesNotMergeTheHeadersSentIfTheContaoFrameworkIsNotInitializ ->willReturn(false) ; - $listener = new MergeHttpHeadersListener($framework, ['Content-Type: text/html']); + $listener = new MergeHttpHeadersListener($framework, new MemoryHeaderStorage(['Content-Type: text/html'])); $listener->onKernelResponse($responseEvent); $this->assertFalse($responseEvent->getResponse()->headers->has('Content-Type')); } /** - * Tests that multi-value headers are not overriden. + * Tests that multi-value headers are not overridden. */ public function testDoesNotOverrideMultiValueHeaders() { @@ -114,7 +115,9 @@ public function testDoesNotOverrideMultiValueHeaders() ->willReturn(true) ; - $listener = new MergeHttpHeadersListener($framework, ['set-cookie: new-content=foobar']); // lower-case key + $headers = new MemoryHeaderStorage(['set-cookie: new-content=foobar']); // lower-case key + + $listener = new MergeHttpHeadersListener($framework, $headers); $listener->onKernelResponse($responseEvent); $response = $responseEvent->getResponse(); @@ -183,4 +186,92 @@ public function testAddsAndRemovesMultiValueHeaders() ] ); } + + /** + * Tests that headers are inherited from a subrequest. + */ + public function testInheritsHeadersFromSubrequest() + { + $responseEvent = new FilterResponseEvent( + $this->mockKernel(), + new Request(), + HttpKernelInterface::MASTER_REQUEST, + new Response() + ); + + $framework = $this->createMock(ContaoFrameworkInterface::class); + + $framework + ->expects($this->atLeastOnce()) + ->method('isInitialized') + ->willReturn(true) + ; + + $headerStorage = new MemoryHeaderStorage(['Content-Type: text/html']); + + $listener = new MergeHttpHeadersListener($framework, $headerStorage); + $listener->onKernelResponse($responseEvent); + + $response = $responseEvent->getResponse(); + + $this->assertTrue($response->headers->has('Content-Type')); + $this->assertSame('text/html', $response->headers->get('Content-Type')); + + $headerStorage->add('Content-Type: application/json'); + + $responseEvent->setResponse(new Response()); + $listener->onKernelResponse($responseEvent); + + $response = $responseEvent->getResponse(); + + $this->assertTrue($response->headers->has('Content-Type')); + $this->assertSame('application/json', $response->headers->get('Content-Type')); + } + + /** + * Tests that multi headers are inherited from a subrequest. + */ + public function testInheritsMultiHeadersFromSubrequest() + { + $responseEvent = new FilterResponseEvent( + $this->mockKernel(), + new Request(), + HttpKernelInterface::MASTER_REQUEST, + new Response() + ); + + $framework = $this->createMock(ContaoFrameworkInterface::class); + + $framework + ->expects($this->atLeastOnce()) + ->method('isInitialized') + ->willReturn(true) + ; + + $headerStorage = new MemoryHeaderStorage(['Set-Cookie: content=foobar']); + + $listener = new MergeHttpHeadersListener($framework, $headerStorage); + $listener->onKernelResponse($responseEvent); + + $response = $responseEvent->getResponse(); + $allHeaders = $response->headers->get('Set-Cookie', null, false); + + $this->assertTrue($response->headers->has('Set-Cookie')); + $this->assertCount(1, $allHeaders); + $this->assertSame('content=foobar; path=/', $allHeaders[0]); + + $headerStorage->add('Set-Cookie: new-content=foobar'); + + $responseEvent->setResponse(new Response()); + $listener->onKernelResponse($responseEvent); + + $response = $responseEvent->getResponse(); + + $allHeaders = $response->headers->get('Set-Cookie', null, false); + + $this->assertTrue($response->headers->has('Set-Cookie')); + $this->assertCount(2, $allHeaders); + $this->assertSame('content=foobar; path=/', $allHeaders[0]); + $this->assertSame('new-content=foobar; path=/', $allHeaders[1]); + } } diff --git a/tests/HttpKernel/Header/MemoryHeaderStorageTest.php b/tests/HttpKernel/Header/MemoryHeaderStorageTest.php new file mode 100644 index 0000000000..b2634f36e8 --- /dev/null +++ b/tests/HttpKernel/Header/MemoryHeaderStorageTest.php @@ -0,0 +1,59 @@ + + */ +class MemoryHeaderStorageTest extends TestCase +{ + /** + * Tests the object instantiation. + */ + public function testCanBeInstantiated() + { + $storage = new MemoryHeaderStorage(); + + $this->assertInstanceOf('Contao\CoreBundle\HttpKernel\Header\MemoryHeaderStorage', $storage); + $this->assertInstanceOf('Contao\CoreBundle\HttpKernel\Header\HeaderStorageInterface', $storage); + } + + /** + * Tests that all headers are returned. + */ + public function testReturnsAllHeaders() + { + $storage = new MemoryHeaderStorage(['Foo: Bar']); + + $this->assertSame(['Foo: Bar'], $storage->all()); + + $storage->add('Bar: Baz'); + + $this->assertSame(['Foo: Bar', 'Bar: Baz'], $storage->all()); + } + + /** + * Tests that existing headers are cleared. + */ + public function testClearsExistingHeaders() + { + $storage = new MemoryHeaderStorage(['Foo: Bar']); + + $this->assertSame(['Foo: Bar'], $storage->all()); + + $storage->clear(); + + $this->assertSame([], $storage->all()); + } +} diff --git a/tests/HttpKernel/Header/NativeHeaderStorageTest.php b/tests/HttpKernel/Header/NativeHeaderStorageTest.php new file mode 100644 index 0000000000..ff58eae6ed --- /dev/null +++ b/tests/HttpKernel/Header/NativeHeaderStorageTest.php @@ -0,0 +1,31 @@ + + */ +class NativeHeaderStorageTest extends TestCase +{ + /** + * Tests the object instantiation. + */ + public function testCanBeInstantiated() + { + $storage = new NativeHeaderStorage(); + + $this->assertInstanceOf('Contao\CoreBundle\HttpKernel\Header\NativeHeaderStorage', $storage); + $this->assertInstanceOf('Contao\CoreBundle\HttpKernel\Header\HeaderStorageInterface', $storage); + } +}