Memory leak in SecureConnector about http-client HOT 4 CLOSED

Well this was fun :)

For those who prefer to skip to the conclusion right away: there's a memory leak in PHP (7.0)'s validation of SAN fields in SSL certificates.

I started by testing against the example server you created and indeed, no memory leak. Back to testing with a real server though and the leak was immediately obvious. A few hours later and a few miles down the rabbit hole it clicked: connecting to an SSL certificate that only provides a commonName (the default when you create a self-signed certificate for development) everything is fine. But if the SSL certificate contains SAN entries (like pretty much every certificate in the wild) there's a leak.

Reproduction is simple. Create a new SSL certificate with two SAN entries and let your server use it:

openssl req \
    -newkey rsa:2048 \
    -x509 \
    -nodes \
    -keyout server.key \
    -new \
    -out server.crt \
    -subj /CN=127.0.0.1 \
    -reqexts SAN \
    -extensions SAN \
    -config <(cat /etc/ssl/openssl.cnf \
        <(printf '[SAN]\nsubjectAltName=DNS:127.0.0.1,DNS:127.0.0.2')) \
    -sha256 \
    -days 3650

cat server.crt server.key > localhost-san.pem

php server.php 127.0.0.1:8000 ./localhost-san.pem 

Then test with this client and the memory leak is back:

$options = [
    'ssl' => [
        'verify_peer' => false
    ]
];

while (true) {
    $response = file_get_contents('https://127.0.0.1:8000', false, stream_context_create($options));
}

Adding the verify_peer_name => false context option makes it go away again.

Alas, since the problem is in PHP itself I'll have to take this elsewhere. I'll submit a bugreport there once I can confirm the problem still exists in PHP 7.2.6. You can probably close this, thanks for pointing me in the right direction @clue :)

from http-client.

FYI, the problem still exists in PHP 7.2.7. I reported an issue here: https://bugs.php.net/bug.php?id=76542

from http-client.

@laszlovl Thank you for reporting!

I've just tried to reproduce this locally, but can not confirm any memory issues.

Here's the client side script I've kept running for a few minutes:

<?php

use React\HttpClient\Client;
use React\Socket\Connector;

require __DIR__ . '/../vendor/autoload.php';

$url = isset($argv[1]) ? $argv[1] : 'https://127.0.0.1:8080/';

$loop = React\EventLoop\Factory::create();
$connector = new Connector($loop, array('tls' => array('verify_peer' => false)));
$client = new Client($loop, $connector);

$send = function () use ($client, $url, &$send){
    $request = $client->request('GET', $url);

    $request->on('error', function (\Exception $e) {
        echo $e;
    });

    $request->on('close', function () use ($send) {
        echo '.';
        $send();
    });

    $request->end();
};
$send();

$loop->addPeriodicTimer(1.0, function () {
    echo PHP_EOL . round(memory_get_usage() / 1024 / 1024, 1) . 'M ' . gc_collect_cycles();
});

$loop->run();

For the server side, I've used our own server examples from https://github.com/reactphp/http/blob/master/examples/11-hello-world-https.php and https://github.com/reactphp/http/blob/master/examples/01-hello-world.php.

I've kept this running for a few minutes and both the memory reported from within PHP and from top output seem to be rather constant at a few megabytes only.

I can confirm that updating dependencies has a very significant effect, as we've recently introduced some significant memory improvements for react/event-loop and react/promise. Can you verify that composer outdated reports you're using all the latest components?

from http-client.

@laszlovl Thank you very much for reporting back and letting us know! I'm happy to hear both that you've been able to track this down and also that this is not a bug here :-)

If you file an upstream issue, make sure to link to it from here so we can follow along 👍

I believe this has been answered, so I'm closing this for now. Please come back with more details if this problem persists and we can reopen this 👍

from http-client.