Repository: reactphp/http (php)

Issue:
Division by zero in src/Server.php line 175
https://github.com/reactphp/http/blob/248202e57195d06a4375f6d2f5c5b9ff9da3ea9e/src/Server.php#L175

The problem is, in Server.php at line 175 it is assumed that ```post_max_size``` will always be a set limit.

However, according to PHP manual: 
```Allow unlimited post size by setting post_max_size to 0.```
https://www.php.net/manual/en/ini.core.php#ini.post-max-size

reactphp/http v0.8.6
PHP 7.1.33

Pull request:
Fix calculating concurrency when `post_max_size` ini is unlimited
Setting `post_max_size` to `0` means the HTTP request body size is unlimited. This has a similar effect to setting it to a very large value (e.g. `4G`). Accordingly, we have to limit concurrency to not exceed the `memory_limit` setting (see also #342).

In other words, this is more of an edge case and it's usually not recommended to allow keeping a very large HTTP request body in memory. The `post_max_size` should be limited to a reasonable value to support decent concurrency.

Resolves #360

Patch:
diff --git a/src/Server.php b/src/Server.php
index 76428aa..cf3f9ca 100644
--- a/src/Server.php
+++ b/src/Server.php
@@ -127,7 +127,9 @@ final class Server extends EventEmitter
         }
 
         $middleware = array();
-        $middleware[] = new LimitConcurrentRequestsMiddleware($this->getConcurrentRequestsLimit());
+        $middleware[] = new LimitConcurrentRequestsMiddleware(
+            $this->getConcurrentRequestsLimit(\ini_get('memory_limit'), \ini_get('post_max_size'))
+        );
         $middleware[] = new RequestBodyBufferMiddleware();
         // Checking for an empty string because that is what a boolean
         // false is returned as by ini_get depending on the PHP version.
@@ -162,17 +164,22 @@ final class Server extends EventEmitter
     }
 
     /**
+     * @param string $memory_limit
+     * @param string $post_max_size
      * @return int
-     * @codeCoverageIgnore
      */
-    private function getConcurrentRequestsLimit()
+    private function getConcurrentRequestsLimit($memory_limit, $post_max_size)
     {
-        if (\ini_get('memory_limit') == -1) {
+        if ($memory_limit == -1) {
             return self::MAXIMUM_CONCURRENT_REQUESTS;
         }
 
-        $availableMemory = IniUtil::iniSizeToBytes(\ini_get('memory_limit')) / 4;
-        $concurrentRequests = \ceil($availableMemory / IniUtil::iniSizeToBytes(\ini_get('post_max_size')));
+        if ($post_max_size == 0) {
+            return 1;
+        }
+
+        $availableMemory = IniUtil::iniSizeToBytes($memory_limit) / 4;
+        $concurrentRequests = (int) \ceil($availableMemory / IniUtil::iniSizeToBytes($post_max_size));
 
         if ($concurrentRequests >= self::MAXIMUM_CONCURRENT_REQUESTS) {
             return self::MAXIMUM_CONCURRENT_REQUESTS;


Test patch:
diff --git a/tests/Io/IniUtilTest.php b/tests/Io/IniUtilTest.php
index 80bc422..22374eb 100644
--- a/tests/Io/IniUtilTest.php
+++ b/tests/Io/IniUtilTest.php
@@ -53,6 +53,11 @@ class IniUtilTest extends TestCase
         $this->assertEquals($output, IniUtil::iniSizeToBytes($input));
     }
 
+    public function testIniSizeToBytesWithInvalidSuffixReturnsNumberWithoutSuffix()
+    {
+        $this->assertEquals('2', IniUtil::iniSizeToBytes('2x'));
+    }
+
     public function provideInvalidInputIniSizeToBytes()
     {
         return array(
diff --git a/tests/ServerTest.php b/tests/ServerTest.php
index d1d84f5..4ca81e1 100644
--- a/tests/ServerTest.php
+++ b/tests/ServerTest.php
@@ -183,4 +183,48 @@ final class ServerTest extends TestCase
 
         return $data;
     }
+
+    public function provideIniSettingsForConcurrency()
+    {
+        return array(
+            'default settings' => array(
+                '128M',
+                '8M',
+                4
+            ),
+            'unlimited memory_limit limited to maximum concurrency' => array(
+                '-1',
+                '8M',
+                100
+            ),
+            'unlimited post_max_size' => array(
+                '128M',
+                '0',
+                1
+            ),
+            'small post_max_size limited to maximum concurrency' => array(
+                '128M',
+                '1k',
+                100
+            )
+        );
+    }
+
+    /**
+     * @param string $memory_limit
+     * @param string $post_max_size
+     * @param int    $expectedConcurrency
+     * @dataProvider provideIniSettingsForConcurrency
+     */
+    public function testServerConcurrency($memory_limit, $post_max_size, $expectedConcurrency)
+    {
+        $server = new Server(function () { });
+
+        $ref = new \ReflectionMethod($server, 'getConcurrentRequestsLimit');
+        $ref->setAccessible(true);
+
+        $value = $ref->invoke($server, $memory_limit, $post_max_size);
+
+        $this->assertEquals($expectedConcurrency, $value);
+    }
 }