Merge pull request #429 from clue-labs/close-unused-body

Explicitly close streaming response body when body MUST be empty

Author: WyriHaximus

src/Io/StreamingServer.php

@@ -330,6 +330,7 @@ public function handleResponse(ConnectionInterface $connection, ServerRequestInt
         // response to HEAD and 1xx, 204 and 304 responses MUST NOT include a body
         // exclude status 101 (Switching Protocols) here for Upgrade request handling above
         if ($method === 'HEAD' || $code === 100 || ($code > 101 && $code < 200) || $code === 204 || $code === 304) {
+            $body->close();
             $body = '';
         }
 

tests/Io/StreamingServerTest.php

@@ -765,7 +765,7 @@ function ($data) use (&$buffer) {
         $this->assertEquals('', $buffer);
     }
 
-    public function testRespomseBodyStreamAlreadyClosedWillSendEmptyBodyChunkedEncoded()
+    public function testResponseBodyStreamAlreadyClosedWillSendEmptyBodyChunkedEncoded()
     {
         $stream = new ThroughStream();
         $stream->close();
@@ -1255,9 +1255,45 @@ function ($data) use (&$buffer) {
         $this->connection->emit('data', array($data));
 
         $this->assertContainsString("HTTP/1.1 200 OK\r\n", $buffer);
+        $this->assertContainsString("\r\nContent-Length: 3\r\n", $buffer);
         $this->assertNotContainsString("bye", $buffer);
     }
 
+    public function testResponseContainsNoResponseBodyForHeadRequestWithStreamingResponse()
+    {
+        $stream = new ThroughStream();
+        $stream->on('close', $this->expectCallableOnce());
+
+        $server = new StreamingServer(Factory::create(), function (ServerRequestInterface $request) use ($stream) {
+            return new Response(
+                200,
+                array('Content-Length' => '3'),
+                $stream
+            );
+        });
+
+        $buffer = '';
+        $this->connection
+            ->expects($this->any())
+            ->method('write')
+            ->will(
+                $this->returnCallback(
+                    function ($data) use (&$buffer) {
+                        $buffer .= $data;
+                    }
+                )
+            );
+
+        $server->listen($this->socket);
+        $this->socket->emit('connection', array($this->connection));
+
+        $data = "HEAD / HTTP/1.1\r\nHost: localhost\r\n\r\n";
+        $this->connection->emit('data', array($data));
+
+        $this->assertContainsString("HTTP/1.1 200 OK\r\n", $buffer);
+        $this->assertContainsString("\r\nContent-Length: 3\r\n", $buffer);
+    }
+
     public function testResponseContainsNoResponseBodyAndNoContentLengthForNoContentStatus()
     {
         $server = new StreamingServer(Factory::create(), function (ServerRequestInterface $request) {
@@ -1287,10 +1323,45 @@ function ($data) use (&$buffer) {
         $this->connection->emit('data', array($data));
 
         $this->assertContainsString("HTTP/1.1 204 No Content\r\n", $buffer);
-        $this->assertNotContainsString("\r\n\Content-Length: 3\r\n", $buffer);
+        $this->assertNotContainsString("\r\nContent-Length: 3\r\n", $buffer);
         $this->assertNotContainsString("bye", $buffer);
     }
 
+    public function testResponseContainsNoResponseBodyAndNoContentLengthForNoContentStatusResponseWithStreamingBody()
+    {
+        $stream = new ThroughStream();
+        $stream->on('close', $this->expectCallableOnce());
+
+        $server = new StreamingServer(Factory::create(), function (ServerRequestInterface $request) use ($stream) {
+            return new Response(
+                204,
+                array('Content-Length' => '3'),
+                $stream
+            );
+        });
+
+        $buffer = '';
+        $this->connection
+            ->expects($this->any())
+            ->method('write')
+            ->will(
+                $this->returnCallback(
+                    function ($data) use (&$buffer) {
+                        $buffer .= $data;
+                    }
+                )
+            );
+
+        $server->listen($this->socket);
+        $this->socket->emit('connection', array($this->connection));
+
+        $data = "HEAD / HTTP/1.1\r\nHost: localhost\r\n\r\n";
+        $this->connection->emit('data', array($data));
+
+        $this->assertContainsString("HTTP/1.1 204 No Content\r\n", $buffer);
+        $this->assertNotContainsString("\r\nContent-Length: 3\r\n", $buffer);
+    }
+
     public function testResponseContainsNoResponseBodyForNotModifiedStatus()
     {
         $server = new StreamingServer(Factory::create(), function (ServerRequestInterface $request) {
@@ -1324,6 +1395,41 @@ function ($data) use (&$buffer) {
         $this->assertNotContainsString("bye", $buffer);
     }
 
+    public function testResponseContainsNoResponseBodyForNotModifiedStatusWithStreamingBody()
+    {
+        $stream = new ThroughStream();
+        $stream->on('close', $this->expectCallableOnce());
+
+        $server = new StreamingServer(Factory::create(), function (ServerRequestInterface $request) use ($stream) {
+            return new Response(
+                304,
+                array('Content-Length' => '3'),
+                $stream
+            );
+        });
+
+        $buffer = '';
+        $this->connection
+            ->expects($this->any())
+            ->method('write')
+            ->will(
+                $this->returnCallback(
+                    function ($data) use (&$buffer) {
+                        $buffer .= $data;
+                    }
+                )
+            );
+
+        $server->listen($this->socket);
+        $this->socket->emit('connection', array($this->connection));
+
+        $data = "GET / HTTP/1.1\r\nHost: localhost\r\n\r\n";
+        $this->connection->emit('data', array($data));
+
+        $this->assertContainsString("HTTP/1.1 304 Not Modified\r\n", $buffer);
+        $this->assertContainsString("\r\nContent-Length: 3\r\n", $buffer);
+    }
+
     public function testRequestInvalidHttpProtocolVersionWillEmitErrorAndSendErrorResponse()
     {
         $error = null;