Merge pull request #1578 from juan-morales/fix-oom-streamhandler

Fix - OutOfMemory error in StreamHandler

src/Monolog/Handler/StreamHandler.php

@@ -25,8 +25,10 @@ use Monolog\Utils;
  */
 class StreamHandler extends AbstractProcessingHandler
 {
-    protected const MAX_CHUNK_SIZE = 2147483647;
+    /** @const int */
-
+    protected const MAX_CHUNK_SIZE = 100 * 1024 * 1024;
+    /** @var int */
+    protected $streamChunkSize = self::MAX_CHUNK_SIZE;
     /** @var resource|null */
     protected $stream;
     /** @var ?string */
@@ -50,9 +52,22 @@ class StreamHandler extends AbstractProcessingHandler
     public function __construct($stream, $level = Logger::DEBUG, bool $bubble = true, ?int $filePermission = null, bool $useLocking = false)
     {
         parent::__construct($level, $bubble);
+
+        if (($phpMemoryLimit = Utils::expandIniShorthandBytes(ini_get('memory_limit'))) !== false) {
+            if ($phpMemoryLimit > 0) {
+                // use max 10% of allowed memory for the chunk size
+                $this->streamChunkSize = max((int) ($phpMemoryLimit / 10), 10*1024);
+            }
+            // else memory is unlimited, keep the buffer to the default 100MB
+        } else {
+            // no memory limit information, use a conservative 10MB
+            $this->streamChunkSize = 10*10*1024;
+        }
+
         if (is_resource($stream)) {
             $this->stream = $stream;
-            stream_set_chunk_size($this->stream, self::MAX_CHUNK_SIZE);
+
+            stream_set_chunk_size($this->stream, $this->streamChunkSize);
         } elseif (is_string($stream)) {
             $this->url = Utils::canonicalizePath($stream);
         } else {
@@ -95,6 +110,14 @@ class StreamHandler extends AbstractProcessingHandler
         return $this->url;
     }
 
+    /**
+     * @return int
+     */
+    public function getStreamChunkSize(): int
+    {
+        return $this->streamChunkSize;
+    }
+
     /**
      * {@inheritDoc}
      */
@@ -118,7 +141,7 @@ class StreamHandler extends AbstractProcessingHandler
 
                 throw new \UnexpectedValueException(sprintf('The stream or file "%s" could not be opened in append mode: '.$this->errorMessage, $url));
             }
-            stream_set_chunk_size($stream, self::MAX_CHUNK_SIZE);
+            stream_set_chunk_size($stream, $this->streamChunkSize);
             $this->stream = $stream;
         }
 

src/Monolog/Utils.php

@@ -226,4 +226,38 @@ final class Utils
             );
         }
     }
+
+    /**
+     * Converts a string with a valid 'memory_limit' format, to bytes.
+     *
+     * @param string|false $val
+     * @return int|false Returns an integer representing bytes. Returns FALSE in case of error.
+     */
+    public static function expandIniShorthandBytes($val)
+    {
+        if (!is_string($val)) {
+            return false;
+        }
+
+        // support -1
+        if ((int) $val < 0) {
+            return (int) $val;
+        }
+
+        if (!preg_match('/^\s*(?<val>\d+)(?:\.\d+)?\s*(?<unit>[gmk]?)\s*$/i', $val, $match)) {
+            return false;
+        }
+
+        $val = (int) $match['val'];
+        switch (strtolower($match['unit'] ?? '')) {
+            case 'g':
+                $val *= 1024;
+            case 'm':
+                $val *= 1024;
+            case 'k':
+                $val *= 1024;
+        }
+
+        return $val;
+    }
 }

tests/Monolog/Handler/StreamHandlerTest.php

@@ -11,6 +11,7 @@
 
 namespace Monolog\Handler;
 
+use Monolog\Handler\StreamHandler;
 use Monolog\Test\TestCase;
 use Monolog\Logger;
 
@@ -220,4 +221,65 @@ class StreamHandlerTest extends TestCase
             ],
         ];
     }
+
+    public function provideMemoryValues()
+    {
+        return [
+            ['1M', (int) (1024*1024/10)],
+            ['10M', (int) (1024*1024)],
+            ['1024M', (int) (1024*1024*1024/10)],
+            ['1G', (int) (1024*1024*1024/10)],
+            ['2000M', (int) (2000*1024*1024/10)],
+            ['2050M', (int) (2050*1024*1024/10)],
+            ['2048M', (int) (2048*1024*1024/10)],
+            ['3G', (int) (3*1024*1024*1024/10)],
+            ['2560M', (int) (2560*1024*1024/10)],
+        ];
+    }
+
+    /**
+     * @dataProvider provideMemoryValues
+     * @return void
+     */
+    public function testPreventOOMError($phpMemory, $expectedChunkSize)
+    {
+        $previousValue = ini_set('memory_limit', $phpMemory);
+
+        if ($previousValue === false) {
+            $this->markTestSkipped('We could not set a memory limit that would trigger the error.');
+        }
+
+        $stream = tmpfile();
+
+        if ($stream === false) {
+            $this->markTestSkipped('We could not create a temp file to be use as a stream.');
+        }
+
+        $exceptionRaised = false;
+
+        $handler = new StreamHandler($stream);
+        stream_get_contents($stream, 1024);
+
+        ini_set('memory_limit', $previousValue);
+
+        $this->assertEquals($expectedChunkSize, $handler->getStreamChunkSize());
+    }
+
+    /**
+     * @return void
+     */
+    public function testSimpleOOMPrevention()
+    {
+        $previousValue = ini_set('memory_limit', '2048M');
+
+        if ($previousValue === false) {
+            $this->markTestSkipped('We could not set a memory limit that would trigger the error.');
+        }
+
+        $stream = tmpfile();
+        new StreamHandler($stream);
+        stream_get_contents($stream);
+        ini_set('memory_limit', $previousValue);
+        $this->assertTrue(true);
+    }
 }

tests/Monolog/UtilsTest.php

@@ -141,4 +141,49 @@ class UtilsTest extends \PHPUnit_Framework_TestCase
             [-1, 'UNDEFINED_ERROR'],
         ];
     }
+
+    public function provideIniValuesToConvertToBytes()
+    {
+        return [
+            ['1', 1],
+            ['2', 2],
+            ['2.5', 2],
+            ['2.9', 2],
+            ['1B', false],
+            ['1X', false],
+            ['1K', 1024],
+            ['1 K', 1024],
+            ['   5 M  ', 5*1024*1024],
+            ['1G', 1073741824],
+            ['', false],
+            [null, false],
+            ['A', false],
+            ['AA', false],
+            ['B', false],
+            ['BB', false],
+            ['G', false],
+            ['GG', false],
+            ['-1', -1],
+            ['-123', -123],
+            ['-1A', -1],
+            ['-1B', -1],
+            ['-123G', -123],
+            ['-B', false],
+            ['-A', false],
+            ['-', false],
+            [true, false],
+            [false, false],
+        ];
+    }
+
+    /**
+     * @dataProvider provideIniValuesToConvertToBytes
+     * @param mixed $input
+     * @param int|false $expected
+     */
+    public function testExpandIniShorthandBytes($input, $expected)
+    {
+        $result = Utils::expandIniShorthandBytes($input);
+        $this->assertEquals($expected, $result);
+    }
 }