Fix additional remarks

Handle error
reactphp · Feb 19, 2017 · df65f14 · df65f14
1 parent fede0a3
commit df65f14
Show file tree

Hide file tree

Showing 2 changed files with 147 additions and 4 deletions.
diff --git a/src/Server.php b/src/Server.php
@@ -5,6 +5,7 @@
 use Evenement\EventEmitter;
 use React\Socket\ServerInterface as SocketServerInterface;
 use React\Socket\ConnectionInterface;
+use React\Stream\ReadableStream;
 
 /**
  * The `Server` class is responsible for handling incoming connections and then
@@ -137,6 +138,18 @@ public function handleRequest(ConnectionInterface $conn, Request $request)
             if (strtolower(end($transferEncodingHeader)) === 'chunked') {
                 $stream = new ChunkedDecoder($conn);
             }
+
+            // remove 'Content-Length' header accordig to: https://tools.ietf.org/html/rfc7230#section-3.3.3
+            $headers = $request->getHeaders();
+            unset($headers['Content-Length']);
+
+            $request = new Request(
+                $request->getMethod(),
+                $request->getPath(),
+                $request->getQueryParams(),
+                $request->getProtocolVersion(),
+                $headers
+            );
         }
 
         if ($request->hasHeader('Content-Length')) {
@@ -145,8 +158,8 @@ public function handleRequest(ConnectionInterface $conn, Request $request)
             $contentLength = (int)$string;
             if ((string)$contentLength !== (string)$string) {
                 // Content-Length value is not an integer or not a single integer
-                $this->emit('error', new \Exception('The value of `Content-Length` is not valid'));
-                return;
+                $this->emit('error', array(new \InvalidArgumentException('The value of `Content-Length` is not valid')));
+                return $this->writeError($conn, 400);
             }
 
             $stream = new LengthLimitedStream($conn, $contentLength);
@@ -174,9 +187,10 @@ public function handleRequest(ConnectionInterface $conn, Request $request)
         $this->emit('request', array($request, $response));
 
         if ($stream instanceof LengthLimitedStream && $contentLength === 0) {
-            // stream must emit an 'end' here, because empty data won't be emitted
+            // Content-Length is 0 and won't emit further data,
+            // 'handleData' from LengthLimitedStream won't be called anymore
             $stream->emit('end');
-            $conn->removeListener('data', array($stream, 'handleData'));
+            $stream->close();
         }
     }
 

diff --git a/tests/ServerTest.php b/tests/ServerTest.php
@@ -677,6 +677,135 @@ public function testContentLengthContainsZeroWillEmitEndEventAdditionalDataWillB
         $this->connection->emit('data', array($data));
     }
 
+    public function testContentLengthWillBeRemovedIfTransferEncodingIsSet()
+    {
+        $server = new Server($this->socket);
+
+        $dataEvent = $this->expectCallableOnceWith('hello');
+        $endEvent = $this->expectCallableOnce();
+        $closeEvent = $this->expectCallableNever();
+        $errorEvent = $this->expectCallableNever();
+
+        $server->on('request', function (Request $request, Response $response) use ($dataEvent, $endEvent, $closeEvent, $errorEvent) {
+            $request->on('data', $dataEvent);
+            $request->on('end', $endEvent);
+            $request->on('close', $closeEvent);
+            $request->on('error', $errorEvent);
+        });
+
+        $this->socket->emit('connection', array($this->connection));
+
+        $data = "GET / HTTP/1.1\r\n";
+        $data .= "Host: example.com:80\r\n";
+        $data .= "Connection: close\r\n";
+        $data .= "Content-Length: 0\r\n";
+        $data .= "Transfer-Encoding: chunked\r\n";
+        $data .= "\r\n";
+
+        $this->connection->emit('data', array($data));
+
+        $data = "5\r\nhello\r\n";
+        $data .= "0\r\n\r\n";
+
+        $this->connection->emit('data', array($data));
+    }
+
+    public function testNonIntegerContentLengthValueWillLeadToError()
+    {
+        $error = null;
+        $server = new Server($this->socket);
+        $server->on('error', function ($message) use (&$error) {
+            $error = $message;
+        });
+
+        $dataEvent = $this->expectCallableNever();
+        $endEvent = $this->expectCallableNever();
+        $closeEvent = $this->expectCallableNever();
+        $errorEvent = $this->expectCallableNever();
+
+        $server->on('request', function (Request $request, Response $response) use ($dataEvent, $endEvent, $closeEvent, $errorEvent) {
+            $request->on('data', $dataEvent);
+            $request->on('end', $endEvent);
+            $request->on('close', $closeEvent);
+            $request->on('error', $errorEvent);
+        });
+
+        $buffer = '';
+        $this->connection
+            ->expects($this->any())
+            ->method('write')
+            ->will(
+                $this->returnCallback(
+                    function ($data) use (&$buffer) {
+                        $buffer .= $data;
+                    }
+                )
+            );
+
+        $this->socket->emit('connection', array($this->connection));
+
+        $data = "GET / HTTP/1.1\r\n";
+        $data .= "Host: example.com:80\r\n";
+        $data .= "Connection: close\r\n";
+        $data .= "Content-Length: bla\r\n";
+        $data .= "\r\n";
+        $data .= "hello";
+
+        $this->connection->emit('data', array($data));
+
+        $this->assertContains("HTTP/1.1 400 Bad Request\r\n", $buffer);
+        $this->assertContains("\r\n\r\nError 400: Bad Request", $buffer);
+        $this->assertInstanceOf('InvalidArgumentException', $error);
+    }
+
+    public function testMultipleIntegerInContentLengthWillLeadToError()
+    {
+        $error = null;
+        $server = new Server($this->socket);
+        $server->on('error', function ($message) use (&$error) {
+            $error = $message;
+        });
+
+        $dataEvent = $this->expectCallableNever();
+        $endEvent = $this->expectCallableNever();
+        $closeEvent = $this->expectCallableNever();
+        $errorEvent = $this->expectCallableNever();
+
+        $server->on('request', function (Request $request, Response $response) use ($dataEvent, $endEvent, $closeEvent, $errorEvent) {
+            $request->on('data', $dataEvent);
+            $request->on('end', $endEvent);
+            $request->on('close', $closeEvent);
+            $request->on('error', $errorEvent);
+        });
+
+        $buffer = '';
+        $this->connection
+            ->expects($this->any())
+            ->method('write')
+            ->will(
+                $this->returnCallback(
+                    function ($data) use (&$buffer) {
+                        $buffer .= $data;
+                    }
+                )
+            );
+
+        $this->socket->emit('connection', array($this->connection));
+
+        $data = "GET / HTTP/1.1\r\n";
+        $data .= "Host: example.com:80\r\n";
+        $data .= "Connection: close\r\n";
+        $data .= "Content-Length: 5, 3, 4\r\n";
+        $data .= "\r\n";
+        $data .= "hello";
+
+        $this->connection->emit('data', array($data));
+
+        $this->assertContains("HTTP/1.1 400 Bad Request\r\n", $buffer);
+        $this->assertContains("\r\n\r\nError 400: Bad Request", $buffer);
+        $this->assertInstanceOf('InvalidArgumentException', $error);
+    }
+
     private function createGetRequest()
     {
         $data = "GET / HTTP/1.1\r\n";