Merge pull request #235 from andig/chunked

Support buffering requests with unknown length (Transfer-Encoding: chunked) in RequestBodyBufferMiddleware

Author: WyriHaximus

README.md

@@ -697,16 +697,11 @@ Any incoming request that has a request body that exceeds this limit will be
 rejected with a `413` (Request Entity Too Large) error message without calling
 the next middleware handlers.
 
-Any incoming request that does not have its size defined and uses the (rare)
-`Transfer-Encoding: chunked` will be rejected with a `411` (Length Required)
-error message without calling the next middleware handlers.
-Note that this only affects incoming requests, the much more common chunked
-transfer encoding for outgoing responses is not affected.
-It is recommended to define a `Content-Length` header instead.
-Note that this does not affect normal requests without a request body
-(such as a simple `GET` request).
+The `RequestBodyBufferMiddleware` will buffer requests with bodies of known size 
+(i.e. with `Content-Length` header specified) as well as requests with bodies of 
+unknown size (i.e. with `Transfer-Encoding: chunked` header).
 
-All other requests will be buffered in memory until the request body end has
+All requests will be buffered in memory until the request body end has
 been reached and then call the next middleware handler with the complete,
 buffered request.
 Similarly, this will immediately invoke the next middleware handler for requests

composer.json

@@ -10,7 +10,7 @@
         "react/stream": "^1.0 || ^0.7.1",
         "react/promise": "^2.3 || ^1.2.1",
         "evenement/evenement": "^3.0 || ^2.0 || ^1.0",
-        "react/promise-stream": "^0.1.1"
+        "react/promise-stream": "^1.0 || ^0.1.2"
     },
     "autoload": {
         "psr-4": {

src/Middleware/RequestBodyBufferMiddleware.php

@@ -7,6 +7,7 @@
 use React\Promise\Stream;
 use React\Stream\ReadableStreamInterface;
 use RingCentral\Psr7\BufferStream;
+use OverflowException;
 
 final class RequestBodyBufferMiddleware
 {
@@ -29,27 +30,30 @@ public function __construct($sizeLimit = null)
 
     public function __invoke(ServerRequestInterface $request, $stack)
     {
-        $size = $request->getBody()->getSize();
-
-        if ($size === null) {
-            return new Response(411, array('Content-Type' => 'text/plain'), 'No Content-Length header given');
-        }
+        $body = $request->getBody();
 
-        if ($size > $this->sizeLimit) {
+        // request body of known size exceeding limit
+        if ($body->getSize() > $this->sizeLimit) {
             return new Response(413, array('Content-Type' => 'text/plain'), 'Request body exceeds allowed limit');
         }
 
-        $body = $request->getBody();
         if (!$body instanceof ReadableStreamInterface) {
             return $stack($request);
         }
 
-        return Stream\buffer($body)->then(function ($buffer) use ($request, $stack) {
+        return Stream\buffer($body, $this->sizeLimit)->then(function ($buffer) use ($request, $stack) {
             $stream = new BufferStream(strlen($buffer));
             $stream->write($buffer);
             $request = $request->withBody($stream);
 
             return $stack($request);
+        }, function($error) {
+            // request body of unknown size exceeding limit during buffering
+            if ($error instanceof OverflowException) {
+                return new Response(413, array('Content-Type' => 'text/plain'), 'Request body exceeds allowed limit');
+            }
+
+            throw $error;
         });
     }
 

tests/Middleware/RequestBodyBufferMiddlewareTest.php

@@ -3,12 +3,14 @@
 namespace React\Tests\Http\Middleware;
 
 use Psr\Http\Message\ServerRequestInterface;
+use React\EventLoop\Factory;
 use React\Http\Io\HttpBodyStream;
 use React\Http\Io\ServerRequest;
 use React\Http\Middleware\RequestBodyBufferMiddleware;
 use React\Stream\ThroughStream;
 use React\Tests\Http\TestCase;
 use RingCentral\Psr7\BufferStream;
+use Clue\React\Block;
 
 final class RequestBodyBufferMiddlewareTest extends TestCase
 {
@@ -63,45 +65,90 @@ function (ServerRequestInterface $request) use (&$exposedRequest) {
         $this->assertSame($body, $exposedRequest->getBody()->getContents());
     }
 
-    public function testUnknownSizeReturnsError411()
+    public function testExcessiveSizeImmediatelyReturnsError413ForKnownSize()
     {
-        $body = $this->getMockBuilder('Psr\Http\Message\StreamInterface')->getMock();
-        $body->expects($this->once())->method('getSize')->willReturn(null);
-
+        $loop = Factory::create();
+        
+        $stream = new ThroughStream();
+        $stream->end('aa');
         $serverRequest = new ServerRequest(
             'GET',
             'https://example.com/',
             array(),
-            $body
+            new HttpBodyStream($stream, 2)
         );
 
-        $buffer = new RequestBodyBufferMiddleware();
+        $buffer = new RequestBodyBufferMiddleware(1);
         $response = $buffer(
             $serverRequest,
-            function () {}
+            function (ServerRequestInterface $request) {
+                return $request;
+            }
         );
 
-        $this->assertSame(411, $response->getStatusCode());
+        $this->assertSame(413, $response->getStatusCode());
     }
 
     public function testExcessiveSizeReturnsError413()
     {
-        $stream = new BufferStream(2);
-        $stream->write('aa');
+        $loop = Factory::create();
 
+        $stream = new ThroughStream();
         $serverRequest = new ServerRequest(
             'GET',
             'https://example.com/',
             array(),
-            $stream
+            new HttpBodyStream($stream, null)
         );
 
         $buffer = new RequestBodyBufferMiddleware(1);
-        $response = $buffer(
+        $promise = $buffer(
             $serverRequest,
-            function () {}
+            function (ServerRequestInterface $request) {
+                return $request;
+            }
         );
 
-        $this->assertSame(413, $response->getStatusCode());
+        $stream->end('aa');
+
+        $exposedResponse = null;
+        $promise->then(
+            function($response) use (&$exposedResponse) {
+                $exposedResponse = $response;
+            },
+            $this->expectCallableNever()
+        );
+
+        $this->assertSame(413, $exposedResponse->getStatusCode());
+
+        Block\await($promise, $loop);
+    }
+
+    /**
+     * @expectedException RuntimeException
+     */
+    public function testBufferingErrorThrows()
+    {
+        $loop = Factory::create();
+        
+        $stream = new ThroughStream();
+        $serverRequest = new ServerRequest(
+            'GET',
+            'https://example.com/',
+            array(),
+            new HttpBodyStream($stream, null)
+        );
+
+        $buffer = new RequestBodyBufferMiddleware(1);
+        $promise = $buffer(
+            $serverRequest,
+            function (ServerRequestInterface $request) {
+                return $request;
+            }
+        );
+
+        $stream->emit('error', array(new \RuntimeException()));
+
+        Block\await($promise, $loop);
     }
 }