Merge pull request slimphp#2673 from 1ma/fix-nonbuffer-headers

non-buffered responses fixes
vuchkov · Apr 30, 2019 · ccef5f7 · ccef5f7
2 parents e6f75e8 + 6914d32
commit ccef5f7
Show file tree

Hide file tree

Showing 7 changed files with 120 additions and 15 deletions.
diff --git a/Slim/Http/Message.php b/Slim/Http/Message.php
@@ -230,6 +230,10 @@ public function withAddedHeader($name, $value)
         $clone = clone $this;
         $clone->headers->add($name, $value);
 
+        if ($this instanceof Response && $this->body instanceof NonBufferedBody) {
+            header(sprintf('%s: %s', $name, $clone->getHeaderLine($name)));
+        }
+
         return $clone;
     }
 
@@ -251,6 +255,10 @@ public function withoutHeader($name)
         $clone = clone $this;
         $clone->headers->remove($name);
 
+        if ($this instanceof Response && $this->body instanceof NonBufferedBody) {
+            header_remove($name);
+        }
+
         return $clone;
     }
 

diff --git a/Slim/Http/Response.php b/Slim/Http/Response.php
@@ -270,13 +270,13 @@ public function getReasonPhrase()
      */
     public function withHeader($name, $value)
     {
-        if ($this->body instanceof NonBufferedBody) {
-            header("$name: $value");
-        }
-
         $clone = clone $this;
         $clone->headers->set($name, $value);
 
+        if ($this->body instanceof NonBufferedBody) {
+            header(sprintf('%s: %s', $name, $clone->getHeaderLine($name)));
+        }
+
         if ($clone->getStatusCode() === StatusCode::HTTP_OK && strtolower($name) === 'location') {
             $clone = $clone->withStatus(StatusCode::HTTP_FOUND);
         }

diff --git a/composer.json b/composer.json
@@ -51,10 +51,9 @@
         }
     },
     "autoload-dev": {
-        "files": [
-          "tests/Assets/PhpFunctionOverrides.php",
-          "tests/Assets/PhpHttpFunctionOverrides.php"
-        ]
+        "psr-4": {
+            "Slim\\Tests\\": "tests"
+        }
     },
     "scripts": {
         "test": [

diff --git a/tests/Assets/HeaderStack.php b/tests/Assets/HeaderStack.php
@@ -78,4 +78,18 @@ public static function has($header)
 
         return false;
     }
+
+    /**
+     * Remove occurrences of $header
+     *
+     * @param string $header
+     */
+    public static function remove($header)
+    {
+        foreach (self::$data as $key => $item) {
+            if (false !== strpos($item['header'], "$header:")) {
+                unset(self::$data[$key]);
+            }
+        }
+    }
 }
diff --git a/tests/Assets/PhpHttpFunctionOverrides.php b/tests/Assets/PhpHttpFunctionOverrides.php
@@ -7,6 +7,8 @@
 
 namespace Slim\Http;
 
+use Slim\Tests\Assets\HeaderStack;
+
 /**
  * Return the value of the global variable $GLOBALS['getallheaders_return'] if it exists. Otherwise the
  * function override calls the default php built-in function.
@@ -21,3 +23,26 @@ function getallheaders()
 
     return \getallheaders();
 }
+
+/**
+ * Emit a header, without creating actual output artifacts
+ *
+ * @param string   $string
+ * @param bool     $replace
+ * @param int|null $statusCode
+ */
+function header($string, $replace = true, $statusCode = null)
+{
+    HeaderStack::push(
+        [
+            'header'      => $string,
+            'replace'     => $replace,
+            'status_code' => $statusCode,
+        ]
+    );
+}
+
+function header_remove($name = null)
+{
+    HeaderStack::remove($name);
+}
diff --git a/tests/Http/NonBufferedBodyTest.php b/tests/Http/NonBufferedBodyTest.php
@@ -9,12 +9,27 @@
 
 use PHPUnit_Framework_TestCase;
 use Slim\Http\NonBufferedBody;
+use Slim\Http\Response;
+use Slim\Tests\Assets\HeaderStack;
 
 class NonBufferedBodyTest extends PHPUnit_Framework_TestCase
 {
+    protected function setUp()
+    {
+        HeaderStack::reset();
+    }
+
+    protected function tearDown()
+    {
+        HeaderStack::reset();
+    }
+
     public function testTheStreamContract()
     {
         $body = new NonBufferedBody();
+        $body->close();
+        $body->seek(0);
+        $body->rewind();
 
         self::assertSame('', (string) $body, 'Casting to string returns no data, since the class does not store any');
         self::assertNull($body->detach(), 'Returns null since there is no such underlying stream');
@@ -28,4 +43,51 @@ public function testTheStreamContract()
         self::assertSame('', $body->getContents(), 'Data cannot be retrieved once written');
         self::assertNull($body->getMetadata(), 'Metadata mechanism is not implemented');
     }
+
+    public function testWithHeader()
+    {
+        (new Response())
+            ->withBody(new NonBufferedBody())
+            ->withHeader('Foo', 'Bar');
+
+        self::assertSame([
+            [
+                'header' => 'Foo: Bar',
+                'replace' => true,
+                'status_code' => null
+            ]
+        ], HeaderStack::stack());
+    }
+
+    public function testWithAddedHeader()
+    {
+        (new Response())
+            ->withBody(new NonBufferedBody())
+            ->withHeader('Foo', 'Bar')
+            ->withAddedHeader('Foo', 'Baz');
+
+        self::assertSame([
+            [
+                'header' => 'Foo: Bar',
+                'replace' => true,
+                'status_code' => null
+            ],
+            [
+                'header' => 'Foo: Bar,Baz',
+                'replace' => true,
+                'status_code' => null
+            ]
+        ], HeaderStack::stack());
+    }
+
+
+    public function testWithoutHeader()
+    {
+        (new Response())
+            ->withBody(new NonBufferedBody())
+            ->withHeader('Foo', 'Bar')
+            ->withoutHeader('Foo');
+
+        self::assertSame([], HeaderStack::stack());
+    }
 }
diff --git a/tests/bootstrap.php b/tests/bootstrap.php
@@ -5,16 +5,13 @@
  * @license https://github.com/slimphp/Slim/blob/3.x/LICENSE.md (MIT License)
  */
 
-use Composer\Autoload\ClassLoader;
-
 // Set timezone
 date_default_timezone_set('America/New_York');
 
 // Prevent session cookies
 ini_set('session.use_cookies', 0);
 
-/** @var ClassLoader $autoloader */
-$autoloader = require dirname(__DIR__) . '/vendor/autoload.php';
-$autoloader->addPsr4('Slim\Tests\\', __DIR__);
-
-require dirname(__FILE__) . '/getallheaders.php';
+require __DIR__ . '/../vendor/autoload.php';
+require __DIR__ . '/getallheaders.php';
+require __DIR__ . '/Assets/PhpFunctionOverrides.php';
+require __DIR__ . '/Assets/PhpHttpFunctionOverrides.php';