Skip to content

Commit

Permalink
Implement buffering for requests with chunked transfer encoding
Browse files Browse the repository at this point in the history
  • Loading branch information
andig authored and clue committed Nov 20, 2017
1 parent 01ae9f7 commit cdf9deb
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 32 deletions.
13 changes: 4 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
20 changes: 12 additions & 8 deletions src/Middleware/RequestBodyBufferMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use React\Promise\Stream;
use React\Stream\ReadableStreamInterface;
use RingCentral\Psr7\BufferStream;
use OverflowException;

final class RequestBodyBufferMiddleware
{
Expand All @@ -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;
});
}

Expand Down
47 changes: 33 additions & 14 deletions tests/Middleware/RequestBodyBufferMiddlewareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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);
}
}

0 comments on commit cdf9deb

Please sign in to comment.