diff --git a/README.md b/README.md index 3396c345..0286f3e3 100644 --- a/README.md +++ b/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 diff --git a/composer.json b/composer.json index 6e577877..f6a4fe24 100644 --- a/composer.json +++ b/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": { diff --git a/src/Middleware/RequestBodyBufferMiddleware.php b/src/Middleware/RequestBodyBufferMiddleware.php index 93ba2126..9a5a3a48 100644 --- a/src/Middleware/RequestBodyBufferMiddleware.php +++ b/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'); + } + + return $error; }); } diff --git a/tests/Middleware/RequestBodyBufferMiddlewareTest.php b/tests/Middleware/RequestBodyBufferMiddlewareTest.php index 8afc9fa1..f5ef7f05 100644 --- a/tests/Middleware/RequestBodyBufferMiddlewareTest.php +++ b/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,62 @@ 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); } }