Merge commit from fork

[3.x] fix cases where a malformed HTTP request with 'Transfer-Encoding: chunked' could lead to ReactPHP looping forever

Author: WyriHaximus

src/Io/ChunkedDecoder.php

@@ -156,12 +156,20 @@ public function handleData($data)
                 $this->transferredSize = 0;
                 $this->buffer = (string)\substr($this->buffer, 2);
             } elseif ($this->chunkSize === 0) {
+                if ($positionCrlf === false) {
+                    // end chunk received, but trailer is incomplete
+                    // trailer shouldn't be bigger than 1024 bytes
+                    if (isset($this->buffer[static::MAX_CHUNK_HEADER_SIZE])) {
+                        $this->handleError(new Exception('Trailer size bigger than ' . static::MAX_CHUNK_HEADER_SIZE . ' bytes'));
+                    }
+                    return;
+                }
                 // end chunk received, skip all trailer data
                 $this->buffer = (string)\substr($this->buffer, $positionCrlf);
             }
 
-            if ($positionCrlf !== 0 && $this->chunkSize !== 0 && $this->chunkSize === $this->transferredSize && \strlen($this->buffer) > 2) {
-                // the first 2 characters are not CRLF, send error event
+            if ($positionCrlf !== 0 && $this->chunkSize !== 0 && $this->chunkSize === $this->transferredSize && \strlen($this->buffer) >= 2) {
+                // chunk is completely transferred, but the following two bytes are not a CRLF, send error event
                 $this->handleError(new Exception('Chunk does not end with a CRLF'));
                 return;
             }

tests/Io/ChunkedDecoderTest.php

@@ -502,6 +502,57 @@ public function testEndChunkWithMultipleTrailersWillBeIgnored()
         $this->input->emit('data', ["0\r\nFoo: a\r\nBar: b\r\nBaz: c\r\n\r\n"]);
     }
 
+    public function testEndChunkWithIncompleteTrailerWithoutCrlfWillWaitForAdditionalDataAndNotCauseInfiniteLoop()
+    {
+        $this->parser->on('data', $this->expectCallableNever());
+        $this->parser->on('error', $this->expectCallableNever());
+        $this->parser->on('end', $this->expectCallableNever());
+        $this->parser->on('close', $this->expectCallableNever());
+
+        // malformed end chunk with trailing data but without a terminating CRLF
+        // must not loop forever, but wait for additional data
+        $this->input->emit('data', ["0\r\nab"]);
+    }
+
+    public function testEndChunkWithIncompleteTrailerWillEndOnceTrailerIsCompleted()
+    {
+        $this->parser->on('data', $this->expectCallableNever());
+        $this->parser->on('error', $this->expectCallableNever());
+        $this->parser->on('end', $this->expectCallableOnce());
+        $this->parser->on('close', $this->expectCallableOnce());
+
+        $this->input->emit('data', ["0\r\nab"]);
+        $this->input->emit('data', ["\r\n\r\n"]);
+    }
+
+    public function testEndChunkWithIncompleteTrailerIsTooBig()
+    {
+        $this->parser->on('data', $this->expectCallableNever());
+        $this->parser->on('close', $this->expectCallableOnce());
+        $this->parser->on('end', $this->expectCallableNever());
+        $this->parser->on('error', $this->expectCallableOnce());
+
+        $data = '';
+        for ($i = 0; $i < 1025; $i++) {
+            $data .= 'a';
+        }
+
+        // incomplete trailer must not be buffered without any limit
+        $this->input->emit('data', ["0\r\n" . $data]);
+    }
+
+    public function testChunkFollowedByExactlyTwoNonCrlfBytesWillErrorAndNotCauseInfiniteLoop()
+    {
+        $this->parser->on('data', $this->expectCallableOnceWith('ab'));
+        $this->parser->on('error', $this->expectCallableOnce());
+        $this->parser->on('end', $this->expectCallableNever());
+        $this->parser->on('close', $this->expectCallableOnce());
+
+        // completed chunk followed by exactly two bytes that are not a CRLF must not
+        // loop forever, but report an invalid chunk terminator
+        $this->input->emit('data', ["2\r\nabXY"]);
+    }
+
     public function testLeadingZerosInInvalidChunk()
     {
         $this->parser->on('data', $this->expectCallableNever());