diff --git a/CHANGELOG.md b/CHANGELOG.md index caeeb231..e124a732 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ All notable changes to this project will be documented in this file, in reverse - Nothing. -## 1.6.1 - TBD +## 1.6.1 - 2017-10-12 ### Added @@ -32,7 +32,10 @@ All notable changes to this project will be documented in this file, in reverse ### Changed -- Nothing. +- [#273](https://github.com/zendframework/zend-diactoros/pull/273) updates each + of the SAPI emitter implementations to emit the status line after emitting + other headers; this is done to ensure that the status line is not overridden + by PHP. ### Deprecated @@ -44,7 +47,10 @@ All notable changes to this project will be documented in this file, in reverse ### Fixed -- Nothing. +- [#273](https://github.com/zendframework/zend-diactoros/pull/273) modifies how + the `SapiEmitterTrait` calls `header()` to ensure that a response code is + _always_ passed as the third argument; this is done to prevent PHP from + silently overriding it. ## 1.6.0 - 2017-09-13 diff --git a/src/Response/SapiEmitter.php b/src/Response/SapiEmitter.php index 0b0a7bd6..111e83be 100644 --- a/src/Response/SapiEmitter.php +++ b/src/Response/SapiEmitter.php @@ -26,8 +26,8 @@ public function emit(ResponseInterface $response) { $this->assertNoPreviousOutput(); - $this->emitStatusLine($response); $this->emitHeaders($response); + $this->emitStatusLine($response); $this->emitBody($response); } diff --git a/src/Response/SapiEmitterTrait.php b/src/Response/SapiEmitterTrait.php index 936eb852..ce9612c8 100644 --- a/src/Response/SapiEmitterTrait.php +++ b/src/Response/SapiEmitterTrait.php @@ -38,17 +38,25 @@ private function assertNoPreviousOutput() * Emits the status line using the protocol version and status code from * the response; if a reason phrase is available, it, too, is emitted. * + * It is important to mention that this method should be called after + * `emitHeaders()` in order to prevent PHP from changing the status code of + * the emitted response. + * * @param ResponseInterface $response + * + * @see \Zend\Diactoros\Response\SapiEmitterTrait::emitHeaders() */ private function emitStatusLine(ResponseInterface $response) { $reasonPhrase = $response->getReasonPhrase(); + $statusCode = $response->getStatusCode(); + header(sprintf( 'HTTP/%s %d%s', $response->getProtocolVersion(), - $response->getStatusCode(), + $statusCode, ($reasonPhrase ? ' ' . $reasonPhrase : '') - )); + ), true, $statusCode); } /** @@ -63,6 +71,8 @@ private function emitStatusLine(ResponseInterface $response) */ private function emitHeaders(ResponseInterface $response) { + $statusCode = $response->getStatusCode(); + foreach ($response->getHeaders() as $header => $values) { $name = $this->filterHeader($header); $first = $name === 'Set-Cookie' ? false : true; @@ -71,7 +81,7 @@ private function emitHeaders(ResponseInterface $response) '%s: %s', $name, $value - ), $first); + ), $first, $statusCode); $first = false; } } diff --git a/src/Response/SapiStreamEmitter.php b/src/Response/SapiStreamEmitter.php index 4175cb07..e06c99f2 100644 --- a/src/Response/SapiStreamEmitter.php +++ b/src/Response/SapiStreamEmitter.php @@ -27,8 +27,8 @@ class SapiStreamEmitter implements EmitterInterface public function emit(ResponseInterface $response, $maxBufferLength = 8192) { $this->assertNoPreviousOutput(); - $this->emitStatusLine($response); $this->emitHeaders($response); + $this->emitStatusLine($response); $range = $this->parseContentRange($response->getHeaderLine('Content-Range')); diff --git a/test/Response/AbstractEmitterTest.php b/test/Response/AbstractEmitterTest.php index b73b17dd..48bb95ee 100644 --- a/test/Response/AbstractEmitterTest.php +++ b/test/Response/AbstractEmitterTest.php @@ -40,8 +40,9 @@ public function testEmitsResponseHeaders() ob_start(); $this->emitter->emit($response); ob_end_clean(); - $this->assertContains('HTTP/1.1 200 OK', HeaderStack::stack()); - $this->assertContains('Content-Type: text/plain', HeaderStack::stack()); + + $this->assertTrue(HeaderStack::has('HTTP/1.1 200 OK')); + $this->assertTrue(HeaderStack::has('Content-Type: text/plain')); } public function testEmitsMessageBody() @@ -55,6 +56,42 @@ public function testEmitsMessageBody() $this->emitter->emit($response); } + public function testMultipleSetCookieHeadersAreNotReplaced() + { + $response = (new Response()) + ->withStatus(200) + ->withAddedHeader('Set-Cookie', 'foo=bar') + ->withAddedHeader('Set-Cookie', 'bar=baz'); + + $this->emitter->emit($response); + + $expectedStack = [ + ['header' => 'Set-Cookie: foo=bar', 'replace' => false, 'status_code' => 200], + ['header' => 'Set-Cookie: bar=baz', 'replace' => false, 'status_code' => 200], + ['header' => 'HTTP/1.1 200 OK', 'replace' => true, 'status_code' => 200], + ]; + + $this->assertSame($expectedStack, HeaderStack::stack()); + } + + public function testDoesNotLetResponseCodeBeOverriddenByPHP() + { + $response = (new Response()) + ->withStatus(202) + ->withAddedHeader('Location', 'http://api.my-service.com/12345678') + ->withAddedHeader('Content-Type', 'text/plain'); + + $this->emitter->emit($response); + + $expectedStack = [ + ['header' => 'Location: http://api.my-service.com/12345678', 'replace' => true, 'status_code' => 202], + ['header' => 'Content-Type: text/plain', 'replace' => true, 'status_code' => 202], + ['header' => 'HTTP/1.1 202 Accepted', 'replace' => true, 'status_code' => 202], + ]; + + $this->assertSame($expectedStack, HeaderStack::stack()); + } + public function testDoesNotInjectContentLengthHeaderIfStreamSizeIsUnknown() { $stream = $this->prophesize('Psr\Http\Message\StreamInterface'); @@ -68,7 +105,7 @@ public function testDoesNotInjectContentLengthHeaderIfStreamSizeIsUnknown() $this->emitter->emit($response); ob_end_clean(); foreach (HeaderStack::stack() as $header) { - $this->assertNotContains('Content-Length:', $header); + $this->assertNotContains('Content-Length:', $header['header']); } } } diff --git a/test/Response/SapiStreamEmitterTest.php b/test/Response/SapiStreamEmitterTest.php index 909bf511..7be4bb6d 100644 --- a/test/Response/SapiStreamEmitterTest.php +++ b/test/Response/SapiStreamEmitterTest.php @@ -56,7 +56,7 @@ public function testDoesNotInjectContentLengthHeaderIfStreamSizeIsUnknown() $this->emitter->emit($response); ob_end_clean(); foreach (HeaderStack::stack() as $header) { - $this->assertNotContains('Content-Length:', $header); + $this->assertNotContains('Content-Length:', $header['header']); } } diff --git a/test/ServerTest.php b/test/ServerTest.php index 5d10f58f..5bdac59c 100644 --- a/test/ServerTest.php +++ b/test/ServerTest.php @@ -151,8 +151,8 @@ public function testListenInvokesCallbackAndSendsResponse() $this->expectOutputString('FOOBAR'); $server->listen(); - $this->assertContains('HTTP/1.1 200 OK', HeaderStack::stack()); - $this->assertContains('Content-Type: text/plain', HeaderStack::stack()); + $this->assertTrue(HeaderStack::has('HTTP/1.1 200 OK')); + $this->assertTrue(HeaderStack::has('Content-Type: text/plain')); } public function testListenEmitsStatusHeaderWithoutReasonPhraseIfNoReasonPhrase() @@ -176,8 +176,8 @@ public function testListenEmitsStatusHeaderWithoutReasonPhraseIfNoReasonPhrase() $this->expectOutputString('FOOBAR'); $server->listen(); - $this->assertContains('HTTP/1.1 299', HeaderStack::stack()); - $this->assertContains('Content-Type: text/plain', HeaderStack::stack()); + $this->assertTrue(HeaderStack::has('HTTP/1.1 299')); + $this->assertTrue(HeaderStack::has('Content-Type: text/plain')); } public function testEnsurePercentCharactersDoNotResultInOutputError() @@ -200,8 +200,8 @@ public function testEnsurePercentCharactersDoNotResultInOutputError() $this->expectOutputString('100%'); $server->listen(); - $this->assertContains('HTTP/1.1 200 OK', HeaderStack::stack()); - $this->assertContains('Content-Type: text/plain', HeaderStack::stack()); + $this->assertTrue(HeaderStack::has('HTTP/1.1 200 OK')); + $this->assertTrue(HeaderStack::has('Content-Type: text/plain')); } public function testEmitsHeadersWithMultipleValuesMultipleTimes() @@ -228,19 +228,16 @@ public function testEmitsHeadersWithMultipleValuesMultipleTimes() $server->listen(); - $this->assertContains('HTTP/1.1 200 OK', HeaderStack::stack()); - $this->assertContains('Content-Type: text/plain', HeaderStack::stack()); - $this->assertContains( - 'Set-Cookie: foo=bar; expires=Wed, 1 Oct 2014 10:30; path=/foo; domain=example.com', - HeaderStack::stack() + $this->assertTrue(HeaderStack::has('HTTP/1.1 200 OK')); + $this->assertTrue(HeaderStack::has('Content-Type: text/plain')); + $this->assertTrue( + HeaderStack::has('Set-Cookie: foo=bar; expires=Wed, 1 Oct 2014 10:30; path=/foo; domain=example.com') ); - $this->assertContains( - 'Set-Cookie: bar=baz; expires=Wed, 8 Oct 2014 10:30; path=/foo/bar; domain=example.com', - HeaderStack::stack() + $this->assertTrue( + HeaderStack::has('Set-Cookie: bar=baz; expires=Wed, 8 Oct 2014 10:30; path=/foo/bar; domain=example.com') ); - $stack = HeaderStack::stack(); - return $stack; + return HeaderStack::stack(); } /** @@ -249,16 +246,23 @@ public function testEmitsHeadersWithMultipleValuesMultipleTimes() */ public function testHeaderOrderIsHonoredWhenEmitted($stack) { + $header = array_pop($stack); + $this->assertContains('Content-Type: text/plain', $header); + $header = array_pop($stack); $this->assertContains( 'Set-Cookie: bar=baz; expires=Wed, 8 Oct 2014 10:30; path=/foo/bar; domain=example.com', $header ); + $header = array_pop($stack); $this->assertContains( 'Set-Cookie: foo=bar; expires=Wed, 1 Oct 2014 10:30; path=/foo; domain=example.com', $header ); + + $header = array_pop($stack); + $this->assertContains('HTTP/1.1 200 OK', $header); } public function testListenPassesCallableArgumentToCallback() diff --git a/test/TestAsset/Functions.php b/test/TestAsset/Functions.php index d8e2b74a..f1d0a5e0 100644 --- a/test/TestAsset/Functions.php +++ b/test/TestAsset/Functions.php @@ -27,7 +27,7 @@ class HeaderStack { /** - * @var array + * @var string[][] */ private static $data = []; @@ -42,9 +42,9 @@ public static function reset() /** * Push a header on the stack * - * @param string $header + * @param string[] $header */ - public static function push($header) + public static function push(array $header) { self::$data[] = $header; } @@ -52,12 +52,30 @@ public static function push($header) /** * Return the current header stack * - * @return array + * @return string[][] */ public static function stack() { return self::$data; } + + /** + * Verify if there's a header line on the stack + * + * @param string $header + * + * @return bool + */ + public static function has($header) + { + foreach (self::$data as $item) { + if ($item['header'] === $header) { + return true; + } + } + + return false; + } } /** @@ -73,9 +91,17 @@ function headers_sent() /** * Emit a header, without creating actual output artifacts * - * @param string $value + * @param string $string + * @param bool $replace + * @param int|null $statusCode */ -function header($value) +function header($string, $replace = true, $statusCode = null) { - HeaderStack::push($value); + HeaderStack::push( + [ + 'header' => $string, + 'replace' => $replace, + 'status_code' => $statusCode, + ] + ); } diff --git a/test/TestAsset/SapiResponse.php b/test/TestAsset/SapiResponse.php index 2586f2bc..11a937b2 100644 --- a/test/TestAsset/SapiResponse.php +++ b/test/TestAsset/SapiResponse.php @@ -35,9 +35,17 @@ function headers_sent() /** * Emit a header, without creating actual output artifacts * - * @param string $value + * @param string $string + * @param bool $replace + * @param int|null $http_response_code */ -function header($value) +function header($string, $replace = true, $http_response_code = null) { - HeaderStack::push($value); + HeaderStack::push( + [ + 'header' => $string, + 'replace' => $replace, + 'status_code' => $http_response_code, + ] + ); }