Merge branch 'hotfix/273'

Close #273
zendframework · Oct 12, 2017 · c8664b9 · c8664b9
2 parents fb7f06e + 8b2bbca
commit c8664b9
Show file tree

Hide file tree

Showing 9 changed files with 129 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,15 +2,18 @@
 
 All notable changes to this project will be documented in this file, in reverse chronological order by release.
 
-## 1.6.1 - TBD
+## 1.6.1 - 2017-10-12
 
 ### Added
 
 - Nothing.
 
 ### 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
 
@@ -22,7 +25,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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -27,7 +27,7 @@
 class HeaderStack
 {
     /**
-     * @var array
+     * @var string[][]
      */
     private static $data = [];
 
@@ -42,22 +42,40 @@ 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;
     }
 
     /**
      * 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
@@ -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,
+        ]
+    );
 }