diff --git a/README.md b/README.md index f95214e5..2888e500 100644 --- a/README.md +++ b/README.md @@ -773,7 +773,8 @@ their respective default values: ``` memory_limit 128M -post_max_size 8M +post_max_size 8M // capped at 64K + enable_post_data_reading 1 max_input_nesting_level 64 max_input_vars 1000 @@ -784,22 +785,30 @@ max_file_uploads 20 ``` In particular, the `post_max_size` setting limits how much memory a single -HTTP request is allowed to consume while buffering its request body. On top -of this, this class will try to avoid consuming more than 1/4 of your +HTTP request is allowed to consume while buffering its request body. This +needs to be limited because the server can process a large number of requests +concurrently, so the server may potentially consume a large amount of memory +otherwise. To support higher concurrency by default, this value is capped +at `64K`. If you assign a higher value, it will only allow `64K` by default. +If a request exceeds this limit, its request body will be ignored and it will +be processed like a request with no request body at all. See below for +explicit configuration to override this setting. + +By default, this class will try to avoid consuming more than half of your `memory_limit` for buffering multiple concurrent HTTP requests. As such, with the above default settings of `128M` max, it will try to consume no more than -`32M` for buffering multiple concurrent HTTP requests. As a consequence, it -will limit the concurrency to 4 HTTP requests with the above defaults. +`64M` for buffering multiple concurrent HTTP requests. As a consequence, it +will limit the concurrency to `1024` HTTP requests with the above defaults. It is imperative that you assign reasonable values to your PHP ini settings. -It is usually recommended to either reduce the memory a single request is -allowed to take (set `post_max_size 1M` or less) or to increase the total -memory limit to allow for more concurrent requests (set `memory_limit 512M` -or more). Failure to do so means that this class may have to disable -concurrency and only handle one request at a time. - -As an alternative to the above buffering defaults, you can also configure -the `Server` explicitly to override these defaults. You can use the +It is usually recommended to not support buffering incoming HTTP requests +with a large HTTP request body (e.g. large file uploads). If you want to +increase this buffer size, you will have to also increase the total memory +limit to allow for more concurrent requests (set `memory_limit 512M` or more) +or explicitly limit concurrency. + +In order to override the above buffering defaults, you can configure the +`Server` explicitly. You can use the [`LimitConcurrentRequestsMiddleware`](#limitconcurrentrequestsmiddleware) and [`RequestBodyBufferMiddleware`](#requestbodybuffermiddleware) (see below) to explicitly configure the total number of requests that can be handled at @@ -816,6 +825,12 @@ $server = new React\Http\Server( ); ``` +In this example, we allow processing up to 100 concurrent requests at once +and each request can buffer up to `2M`. This means you may have to keep a +maximum of `200M` of memory for incoming request body buffers. Accordingly, +you need to adjust the `memory_limit` ini setting to allow for these buffers +plus your actual application logic memory requirements (think `512M` or more). + > Internally, this class automatically assigns these middleware handlers automatically when no [`StreamingRequestMiddleware`](#streamingrequestmiddleware) is given. Accordingly, you can use this example to override all default diff --git a/src/Io/StreamingServer.php b/src/Io/StreamingServer.php index 1d41a9c3..166cd4e0 100644 --- a/src/Io/StreamingServer.php +++ b/src/Io/StreamingServer.php @@ -29,7 +29,7 @@ * object in return: * * ```php - * $server = new StreamingServer(function (ServerRequestInterface $request) { + * $server = new StreamingServer($loop, function (ServerRequestInterface $request) { * return new Response( * 200, * array( @@ -54,7 +54,7 @@ * in order to start a plaintext HTTP server like this: * * ```php - * $server = new StreamingServer($handler); + * $server = new StreamingServer($loop, $handler); * * $socket = new React\Socket\Server('0.0.0.0:8080', $loop); * $server->listen($socket); diff --git a/src/Middleware/LimitConcurrentRequestsMiddleware.php b/src/Middleware/LimitConcurrentRequestsMiddleware.php index d16402df..d6760e95 100644 --- a/src/Middleware/LimitConcurrentRequestsMiddleware.php +++ b/src/Middleware/LimitConcurrentRequestsMiddleware.php @@ -29,11 +29,12 @@ * than 10 handlers will be invoked at once: * * ```php - * $server = new Server(array( + * $server = new Server( + * $loop, * new StreamingRequestMiddleware(), * new LimitConcurrentRequestsMiddleware(10), * $handler - * )); + * ); * ``` * * Similarly, this middleware is often used in combination with the @@ -41,13 +42,14 @@ * to limit the total number of requests that can be buffered at once: * * ```php - * $server = new Server(array( + * $server = new Server( + * $loop, * new StreamingRequestMiddleware(), * new LimitConcurrentRequestsMiddleware(100), // 100 concurrent buffering handlers * new RequestBodyBufferMiddleware(2 * 1024 * 1024), // 2 MiB per request * new RequestBodyParserMiddleware(), * $handler - * )); + * ); * ``` * * More sophisticated examples include limiting the total number of requests @@ -55,14 +57,15 @@ * processes one request after another without any concurrency: * * ```php - * $server = new Server(array( + * $server = new Server( + * $loop, * new StreamingRequestMiddleware(), * new LimitConcurrentRequestsMiddleware(100), // 100 concurrent buffering handlers * new RequestBodyBufferMiddleware(2 * 1024 * 1024), // 2 MiB per request * new RequestBodyParserMiddleware(), * new LimitConcurrentRequestsMiddleware(1), // only execute 1 handler (no concurrency) * $handler - * )); + * ); * ``` * * @see RequestBodyBufferMiddleware diff --git a/src/Server.php b/src/Server.php index 7b165c37..3b5d70be 100644 --- a/src/Server.php +++ b/src/Server.php @@ -23,7 +23,7 @@ * object and expects a [response](#server-response) object in return: * * ```php - * $server = new React\Http\Server(function (Psr\Http\Message\ServerRequestInterface $request) { + * $server = new React\Http\Server($loop, function (Psr\Http\Message\ServerRequestInterface $request) { * return new React\Http\Message\Response( * 200, * array( @@ -51,7 +51,7 @@ * to start a plaintext HTTP server like this: * * ```php - * $server = new React\Http\Server($handler); + * $server = new React\Http\Server($loop, $handler); * * $socket = new React\Socket\Server('0.0.0.0:8080', $loop); * $server->listen($socket); @@ -79,7 +79,8 @@ * * ``` * memory_limit 128M - * post_max_size 8M + * post_max_size 8M // capped at 64K + * * enable_post_data_reading 1 * max_input_nesting_level 64 * max_input_vars 1000 @@ -90,29 +91,38 @@ * ``` * * In particular, the `post_max_size` setting limits how much memory a single - * HTTP request is allowed to consume while buffering its request body. On top - * of this, this class will try to avoid consuming more than 1/4 of your + * HTTP request is allowed to consume while buffering its request body. This + * needs to be limited because the server can process a large number of requests + * concurrently, so the server may potentially consume a large amount of memory + * otherwise. To support higher concurrency by default, this value is capped + * at `64K`. If you assign a higher value, it will only allow `64K` by default. + * If a request exceeds this limit, its request body will be ignored and it will + * be processed like a request with no request body at all. See below for + * explicit configuration to override this setting. + * + * By default, this class will try to avoid consuming more than half of your * `memory_limit` for buffering multiple concurrent HTTP requests. As such, with * the above default settings of `128M` max, it will try to consume no more than - * `32M` for buffering multiple concurrent HTTP requests. As a consequence, it - * will limit the concurrency to 4 HTTP requests with the above defaults. + * `64M` for buffering multiple concurrent HTTP requests. As a consequence, it + * will limit the concurrency to `1024` HTTP requests with the above defaults. * * It is imperative that you assign reasonable values to your PHP ini settings. - * It is usually recommended to either reduce the memory a single request is - * allowed to take (set `post_max_size 1M` or less) or to increase the total - * memory limit to allow for more concurrent requests (set `memory_limit 512M` - * or more). Failure to do so means that this class may have to disable - * concurrency and only handle one request at a time. + * It is usually recommended to not support buffering incoming HTTP requests + * with a large HTTP request body (e.g. large file uploads). If you want to + * increase this buffer size, you will have to also increase the total memory + * limit to allow for more concurrent requests (set `memory_limit 512M` or more) + * or explicitly limit concurrency. * - * As an alternative to the above buffering defaults, you can also configure - * the `Server` explicitly to override these defaults. You can use the + * In order to override the above buffering defaults, you can configure the + * `Server` explicitly. You can use the * [`LimitConcurrentRequestsMiddleware`](#limitconcurrentrequestsmiddleware) and * [`RequestBodyBufferMiddleware`](#requestbodybuffermiddleware) (see below) * to explicitly configure the total number of requests that can be handled at * once like this: * * ```php - * $server = new React\Http\Server(array( + * $server = new React\Http\Server( + * $loop, * new React\Http\Middleware\StreamingRequestMiddleware(), * new React\Http\Middleware\LimitConcurrentRequestsMiddleware(100), // 100 concurrent buffering handlers * new React\Http\Middleware\RequestBodyBufferMiddleware(2 * 1024 * 1024), // 2 MiB per request @@ -121,6 +131,12 @@ * )); * ``` * + * In this example, we allow processing up to 100 concurrent requests at once + * and each request can buffer up to `2M`. This means you may have to keep a + * maximum of `200M` of memory for incoming request body buffers. Accordingly, + * you need to adjust the `memory_limit` ini setting to allow for these buffers + * plus your actual application logic memory requirements (think `512M` or more). + * * > Internally, this class automatically assigns these middleware handlers * automatically when no [`StreamingRequestMiddleware`](#streamingrequestmiddleware) * is given. Accordingly, you can use this example to override all default @@ -131,10 +147,11 @@ * in memory: * * ```php - * $server = new React\Http\Server(array( + * $server = new React\Http\Server( + * $loop, * new React\Http\Middleware\StreamingRequestMiddleware(), * $handler - * )); + * ); * ``` * * In this case, it will invoke the request handler function once the HTTP @@ -149,9 +166,17 @@ final class Server extends EventEmitter { /** + * The maximum buffer size used for each request. + * + * This needs to be limited because the server can process a large number of + * requests concurrently, so the server may potentially consume a large + * amount of memory otherwise. + * + * See `RequestBodyBufferMiddleware` to override this setting. + * * @internal */ - const MAXIMUM_CONCURRENT_REQUESTS = 100; + const MAXIMUM_BUFFER_SIZE = 65536; // 64 KiB /** * @var StreamingServer @@ -189,10 +214,12 @@ public function __construct(LoopInterface $loop) $middleware = array(); if (!$streaming) { - $middleware[] = new LimitConcurrentRequestsMiddleware( - $this->getConcurrentRequestsLimit(\ini_get('memory_limit'), \ini_get('post_max_size')) - ); - $middleware[] = new RequestBodyBufferMiddleware(); + $maxSize = $this->getMaxRequestSize(); + $concurrency = $this->getConcurrentRequestsLimit(\ini_get('memory_limit'), $maxSize); + if ($concurrency !== null) { + $middleware[] = new LimitConcurrentRequestsMiddleware($concurrency); + } + $middleware[] = new RequestBodyBufferMiddleware($maxSize); // Checking for an empty string because that is what a boolean // false is returned as by ini_get depending on the PHP version. // @link http://php.net/manual/en/ini.core.php#ini.enable-post-data-reading @@ -226,7 +253,7 @@ public function __construct(LoopInterface $loop) * order to start a plaintext HTTP server like this: * * ```php - * $server = new React\Http\Server($handler); + * $server = new React\Http\Server($loop, $handler); * * $socket = new React\Socket\Server(8080, $loop); * $server->listen($socket); @@ -252,7 +279,7 @@ public function __construct(LoopInterface $loop) * `passphrase` like this: * * ```php - * $server = new React\Http\Server($handler); + * $server = new React\Http\Server($loop, $handler); * * $socket = new React\Socket\Server('tls://0.0.0.0:8443', $loop, array( * 'local_cert' => __DIR__ . '/localhost.pem' @@ -273,25 +300,28 @@ public function listen(ServerInterface $server) /** * @param string $memory_limit * @param string $post_max_size - * @return int + * @return ?int */ private function getConcurrentRequestsLimit($memory_limit, $post_max_size) { if ($memory_limit == -1) { - return self::MAXIMUM_CONCURRENT_REQUESTS; - } - - if ($post_max_size == 0) { - return 1; + return null; } - $availableMemory = IniUtil::iniSizeToBytes($memory_limit) / 4; + $availableMemory = IniUtil::iniSizeToBytes($memory_limit) / 2; $concurrentRequests = (int) \ceil($availableMemory / IniUtil::iniSizeToBytes($post_max_size)); - if ($concurrentRequests >= self::MAXIMUM_CONCURRENT_REQUESTS) { - return self::MAXIMUM_CONCURRENT_REQUESTS; - } - return $concurrentRequests; } + + /** + * @param ?string $post_max_size + * @return int + */ + private function getMaxRequestSize($post_max_size = null) + { + $maxSize = IniUtil::iniSizeToBytes($post_max_size === null ? \ini_get('post_max_size') : $post_max_size); + + return ($maxSize === 0 || $maxSize >= self::MAXIMUM_BUFFER_SIZE) ? self::MAXIMUM_BUFFER_SIZE : $maxSize; + } } diff --git a/tests/ServerTest.php b/tests/ServerTest.php index 59452815..ce19dda9 100644 --- a/tests/ServerTest.php +++ b/tests/ServerTest.php @@ -10,6 +10,7 @@ use React\Promise; use React\Http\Middleware\StreamingRequestMiddleware; use React\Stream\ReadableStreamInterface; +use React\Http\Io\IniUtil; final class ServerTest extends TestCase { @@ -224,23 +225,18 @@ public function provideIniSettingsForConcurrency() return array( 'default settings' => array( '128M', - '8M', - 4 + '64K', // 8M capped at maximum + 1024 ), - 'unlimited memory_limit limited to maximum concurrency' => array( + 'unlimited memory_limit has no concurrency limit' => array( '-1', '8M', - 100 - ), - 'unlimited post_max_size' => array( - '128M', - '0', - 1 + null ), - 'small post_max_size limited to maximum concurrency' => array( + 'small post_max_size results in high concurrency' => array( '128M', '1k', - 100 + 65536 ) ); } @@ -248,7 +244,7 @@ public function provideIniSettingsForConcurrency() /** * @param string $memory_limit * @param string $post_max_size - * @param int $expectedConcurrency + * @param ?int $expectedConcurrency * @dataProvider provideIniSettingsForConcurrency */ public function testServerConcurrency($memory_limit, $post_max_size, $expectedConcurrency) @@ -262,4 +258,100 @@ public function testServerConcurrency($memory_limit, $post_max_size, $expectedCo $this->assertEquals($expectedConcurrency, $value); } + + public function testServerGetPostMaxSizeReturnsSizeFromGivenIniSetting() + { + $server = new Server(Factory::create(), function () { }); + + $ref = new \ReflectionMethod($server, 'getMaxRequestSize'); + $ref->setAccessible(true); + + $value = $ref->invoke($server, '1k'); + + $this->assertEquals(1024, $value); + } + + public function testServerGetPostMaxSizeReturnsSizeCappedFromGivenIniSetting() + { + $server = new Server(Factory::create(), function () { }); + + $ref = new \ReflectionMethod($server, 'getMaxRequestSize'); + $ref->setAccessible(true); + + $value = $ref->invoke($server, '1M'); + + $this->assertEquals(64 * 1024, $value); + } + + public function testServerGetPostMaxSizeFromIniIsCapped() + { + if (IniUtil::iniSizeToBytes(ini_get('post_max_size')) < 64 * 1024) { + $this->markTestSkipped(); + } + + $server = new Server(Factory::create(), function () { }); + + $ref = new \ReflectionMethod($server, 'getMaxRequestSize'); + $ref->setAccessible(true); + + $value = $ref->invoke($server); + + $this->assertEquals(64 * 1024, $value); + } + + public function testConstructServerWithUnlimitedMemoryLimitDoesNotLimitConcurrency() + { + $old = ini_get('memory_limit'); + ini_set('memory_limit', '-1'); + + $server = new Server(Factory::create(), function () { }); + + ini_set('memory_limit', $old); + + $ref = new \ReflectionProperty($server, 'streamingServer'); + $ref->setAccessible(true); + + $streamingServer = $ref->getValue($server); + + $ref = new \ReflectionProperty($streamingServer, 'callback'); + $ref->setAccessible(true); + + $middlewareRunner = $ref->getValue($streamingServer); + + $ref = new \ReflectionProperty($middlewareRunner, 'middleware'); + $ref->setAccessible(true); + + $middleware = $ref->getValue($middlewareRunner); + + $this->assertTrue(is_array($middleware)); + $this->assertInstanceOf('React\Http\Middleware\RequestBodyBufferMiddleware', $middleware[0]); + } + + public function testConstructServerWithMemoryLimitDoesLimitConcurrency() + { + $old = ini_get('memory_limit'); + ini_set('memory_limit', '100M'); + + $server = new Server(Factory::create(), function () { }); + + ini_set('memory_limit', $old); + + $ref = new \ReflectionProperty($server, 'streamingServer'); + $ref->setAccessible(true); + + $streamingServer = $ref->getValue($server); + + $ref = new \ReflectionProperty($streamingServer, 'callback'); + $ref->setAccessible(true); + + $middlewareRunner = $ref->getValue($streamingServer); + + $ref = new \ReflectionProperty($middlewareRunner, 'middleware'); + $ref->setAccessible(true); + + $middleware = $ref->getValue($middlewareRunner); + + $this->assertTrue(is_array($middleware)); + $this->assertInstanceOf('React\Http\Middleware\LimitConcurrentRequestsMiddleware', $middleware[0]); + } }