Fixed memory leak by buffering body immediately.

Porter-connectors · Nov 12, 2022 · aae037d · aae037d
1 parent 2cee4fa
commit aae037d
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 19 deletions.
diff --git a/src/HttpResponse.php b/src/HttpResponse.php
@@ -3,7 +3,6 @@
 
 namespace ScriptFUSION\Porter\Net\Http;
 
-use Amp\ByteStream\Payload;
 use Amp\Http\Client\Response;
 
 /**
@@ -16,8 +15,7 @@ final class HttpResponse
     private int $statusCode;
     private string $statusPhrase;
     private ?self $previous;
-    private Payload $body;
-    private string $bodyBuffer;
+    private string $bufferedBody;
 
     public function __construct(Response $ampResponse)
     {
@@ -26,7 +24,8 @@ public function __construct(Response $ampResponse)
         $this->statusCode = $ampResponse->getStatus();
         $this->statusPhrase = $ampResponse->getReason();
         $this->previous = $ampResponse->getPreviousResponse() ? new self($ampResponse->getPreviousResponse()) : null;
-        $this->body = $ampResponse->getBody();
+        // We must buffer body immediately, otherwise we get memory leaks.
+        $this->bufferedBody = $ampResponse->getBody()->isReadable() ? $ampResponse->getBody()->buffer() : '';
     }
 
     public function __toString(): string
@@ -60,7 +59,7 @@ public function getHeader(string $name): array
 
     public function getBody(): string
     {
-        return $this->bodyBuffer ??= $this->body->buffer();
+        return $this->bufferedBody;
     }
 
     public function getStatusCode(): int

diff --git a/test/Functional/HttpConnectorTest.php b/test/Functional/HttpConnectorTest.php
@@ -23,8 +23,9 @@
 
 final class HttpConnectorTest extends TestCase
 {
-    private const HOST = '[::1]:12345';
-    private const SSL_HOST = '[::1]:6666';
+    private const IP = '[::1]';
+    private const HOST = self::IP . ':12345';
+    private const SSL_HOST = self::IP . ':6666';
     private const URI = 'feedback.php?baz=qux';
     private const DIR = __DIR__ . '/servers';
 
@@ -186,13 +187,10 @@ public function testDefaultBodyLengthTooLong(): void
     {
         $server = $this->startServer();
 
-        $this->connector = new HttpConnector();
-        $response = $this->fetch(self::buildDataSource('big.php'));
-
         $this->expectException(HttpException::class);
 
         try {
-            $response->getBody();
+            $this->fetch(self::buildDataSource('big.php'));
         } finally {
             $this->stopServer($server);
         }
@@ -206,12 +204,11 @@ public function testCustomBodyLengthTooLong(): void
         $server = $this->startServer();
 
         $this->connector = new HttpConnector((new HttpOptions)->setMaxBodyLength(1));
-        $response = $this->fetch(self::buildDataSource());
 
         $this->expectException(HttpException::class);
 
         try {
-            $response->getBody();
+            $this->fetch(self::buildDataSource());
         } finally {
             $this->stopServer($server);
         }
@@ -236,14 +233,15 @@ private function stopServer(Process $server): void
 
     private function startSsl(): string
     {
+        $ip = self::IP;
         $accept = str_replace($filter = ['[', ']'], '', self::SSL_HOST);
         $connect = str_replace($filter, '', self::HOST);
         $certificate = tempnam(sys_get_temp_dir(), '');
 
         // Create SSL tunnel process.
         Process::fromShellCommandline(
             // Generate self-signed SSL certificate in PEM format.
-            "openssl req -new -x509 -nodes -subj /CN=[::1] -keyout '$certificate' -out '$certificate'
+            "openssl req -new -x509 -nodes -subj /CN=$ip -keyout '$certificate' -out '$certificate'
 
             { stunnel4 -fd 0 || stunnel -fd 0; } <<.
                 # Disable PID to run as non-root user.
@@ -258,9 +256,7 @@ private function startSsl(): string
 ."
         )->start();
 
-        self::waitForHttpServer(function () use ($certificate): void {
-            $this->fetchViaSsl(self::createSslConnector($certificate));
-        });
+        self::waitForHttpServer(fn () => $this->fetchViaSsl(self::createSslConnector($certificate)));
 
         return $certificate;
     }
@@ -301,9 +297,8 @@ static function (\Exception $exception) {
                 }
 
                 static $handler;
-                $handler = $handler ?: new ExponentialBackoffExceptionHandler;
 
-                return $handler();
+                return ($handler ??= new ExponentialBackoffExceptionHandler)();
             }
         );
     }