RequestBodyParserMiddleware does not reject requests that exceed post_max_size

Author: WyriHaximus

src/Middleware/RequestBodyBufferMiddleware.php

@@ -2,12 +2,12 @@
 
 namespace React\Http\Middleware;
 
+use OverflowException;
 use Psr\Http\Message\ServerRequestInterface;
-use React\Http\Response;
+use React\Promise\Promise;
 use React\Promise\Stream;
 use React\Stream\ReadableStreamInterface;
 use RingCentral\Psr7\BufferStream;
-use OverflowException;
 
 final class RequestBodyBufferMiddleware
 {
@@ -30,27 +30,35 @@ public function __construct($sizeLimit = null)
 
     public function __invoke(ServerRequestInterface $request, $stack)
     {
+        $sizeLimit = $this->sizeLimit;
         $body = $request->getBody();
 
         // 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');
+            $sizeLimit = 0;
         }
 
         if (!$body instanceof ReadableStreamInterface) {
             return $stack($request);
         }
 
-        return Stream\buffer($body, $this->sizeLimit)->then(function ($buffer) use ($request, $stack) {
+        return Stream\buffer($body, $sizeLimit)->then(function ($buffer) use ($request, $stack) {
             $stream = new BufferStream(strlen($buffer));
             $stream->write($buffer);
             $request = $request->withBody($stream);
 
             return $stack($request);
-        }, function($error) {
+        }, function ($error) use ($stack, $request, $body) {
             // 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');
+                return new Promise(function ($resolve, $reject) use ($stack, $request, $body) {
+                    $body->on('error', function ($error) use ($reject) {
+                        $reject($error);
+                    });
+                    $body->on('close', function () use ($stack, $request, $resolve) {
+                        $resolve($stack($request));
+                    });
+                });
             }
 
             throw $error;

tests/Middleware/RequestBodyBufferMiddlewareTest.php

@@ -2,15 +2,16 @@
 
 namespace React\Tests\Http\Middleware;
 
+use Clue\React\Block;
 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\Http\Response;
 use React\Stream\ThroughStream;
 use React\Tests\Http\TestCase;
 use RingCentral\Psr7\BufferStream;
-use Clue\React\Block;
 
 final class RequestBodyBufferMiddlewareTest extends TestCase
 {
@@ -65,7 +66,7 @@ function (ServerRequestInterface $request) use (&$exposedRequest) {
         $this->assertSame($body, $exposedRequest->getBody()->getContents());
     }
 
-    public function testExcessiveSizeImmediatelyReturnsError413ForKnownSize()
+    public function testKnownExcessiveSizedBodyIsDisgardedTheRequestIsPassedDownToTheNextMiddleware()
     {
         $loop = Factory::create();
 
@@ -79,17 +80,18 @@ public function testExcessiveSizeImmediatelyReturnsError413ForKnownSize()
         );
 
         $buffer = new RequestBodyBufferMiddleware(1);
-        $response = $buffer(
+        $response = Block\await($buffer(
             $serverRequest,
             function (ServerRequestInterface $request) {
-                return $request;
+                return new Response(200, array(), $request->getBody()->getContents());
             }
-        );
+        ), $loop);
 
-        $this->assertSame(413, $response->getStatusCode());
+        $this->assertSame(200, $response->getStatusCode());
+        $this->assertSame('', $response->getBody()->getContents());
     }
 
-    public function testExcessiveSizeReturnsError413()
+    public function testExcessiveSizeBodyIsDisgardedTheRequestIsPassedDownToTheNextMiddleware()
     {
         $loop = Factory::create();
 
@@ -105,23 +107,19 @@ public function testExcessiveSizeReturnsError413()
         $promise = $buffer(
             $serverRequest,
             function (ServerRequestInterface $request) {
-                return $request;
+                return new Response(200, array(), $request->getBody()->getContents());
             }
         );
 
         $stream->end('aa');
 
-        $exposedResponse = null;
-        $promise->then(
-            function($response) use (&$exposedResponse) {
-                $exposedResponse = $response;
-            },
+        $exposedResponse = Block\await($promise->then(
+            null,
             $this->expectCallableNever()
-        );
-
-        $this->assertSame(413, $exposedResponse->getStatusCode());
+        ), $loop);
 
-        Block\await($promise, $loop);
+        $this->assertSame(200, $exposedResponse->getStatusCode());
+        $this->assertSame('', $exposedResponse->getBody()->getContents());
     }
 
     /**
@@ -151,4 +149,30 @@ function (ServerRequestInterface $request) {
 
         Block\await($promise, $loop);
     }
+
+    public function testFullBodyStreamedBeforeCallingNextMiddleware()
+    {
+        $promiseResolved = false;
+        $middleware = new RequestBodyBufferMiddleware(3);
+        $stream = new ThroughStream();
+        $serverRequest = new ServerRequest(
+            'GET',
+            'https://example.com/',
+            array(),
+            new HttpBodyStream($stream, null)
+        );
+
+        $middleware($serverRequest, function () {
+            return new Response();
+        })->then(function () use (&$promiseResolved) {
+            $promiseResolved = true;
+        });
+
+        $stream->write('aaa');
+        $this->assertFalse($promiseResolved);
+        $stream->write('aaa');
+        $this->assertFalse($promiseResolved);
+        $stream->end('aaa');
+        $this->assertTrue($promiseResolved);
+    }
 }