[HttpClient] Fix early cleanup of pushed HTTP/2 responses

This commit is contained in:
Grégoire Pineau 2019-11-23 16:38:53 +01:00 committed by Nicolas Grekas
parent a8a9e69488
commit 0f51da6ec7
13 changed files with 254 additions and 73 deletions

View File

@ -141,6 +141,12 @@ before_install:
(cd php-$MIN_PHP && ./configure --enable-sigchild --enable-pcntl && make -j2)
fi
- |
# Install vulcain
wget https://github.com/symfony/binary-utils/releases/download/v0.1/vulcain_0.1.3_Linux_x86_64.tar.gz -O - | tar xz
sudo mv vulcain /usr/local/bin
docker pull php:7.3-alpine
- |
# php.ini configuration
for PHP in $TRAVIS_PHP_VERSION $php_extra; do
@ -307,8 +313,14 @@ install:
PHPUNIT_X="$PHPUNIT_X,legacy"
fi
if [[ $PHP = ${MIN_PHP%.*} ]]; then
tfold src/Symfony/Component/HttpClient.h2push docker run -it --rm -v $(pwd):/app -v /usr/local/bin/vulcain:/usr/local/bin/vulcain -w /app php:7.3-alpine ./phpunit src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php --filter testHttp2Push
fi
echo "$COMPONENTS" | parallel --gnu "tfold {} $PHPUNIT_X {}"
tfold src/Symfony/Component/Console.tty $PHPUNIT --group tty
if [[ $PHP = ${MIN_PHP%.*} ]]; then
export PHP=$MIN_PHP
tfold src/Symfony/Component/Process.sigchild SYMFONY_DEPRECATIONS_HELPER=weak php-$MIN_PHP/sapi/cli/php ./phpunit --colors=always src/Symfony/Component/Process/

View File

@ -23,6 +23,7 @@ use Symfony\Component\HttpClient\Response\ResponseStream;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;
use Symfony\Contracts\HttpClient\ResponseStreamInterface;
use Symfony\Contracts\Service\ResetInterface;
/**
* A performant implementation of the HttpClientInterface contracts based on the curl extension.
@ -32,7 +33,7 @@ use Symfony\Contracts\HttpClient\ResponseStreamInterface;
*
* @author Nicolas Grekas <p@tchwork.com>
*/
final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface
final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface, ResetInterface
{
use HttpClientTrait;
use LoggerAwareTrait;
@ -324,9 +325,17 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface
return new ResponseStream(CurlResponse::stream($responses, $timeout));
}
public function __destruct()
public function reset()
{
if ($this->logger) {
foreach ($this->multi->pushedResponses as $url => $response) {
$this->logger->debug(sprintf('Unused pushed response: "%s"', $url));
}
}
$this->multi->pushedResponses = [];
$this->multi->dnsCache->evictions = $this->multi->dnsCache->evictions ?: $this->multi->dnsCache->removals;
$this->multi->dnsCache->removals = $this->multi->dnsCache->hostnames = [];
if (\is_resource($this->multi->handle)) {
if (\defined('CURLMOPT_PUSHFUNCTION')) {
@ -344,6 +353,11 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface
}
}
public function __destruct()
{
$this->reset();
}
private static function handlePush($parent, $pushed, array $requestHeaders, CurlClientState $multi, int $maxPendingPushes, ?LoggerInterface $logger): int
{
$headers = [];
@ -363,12 +377,6 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface
$url = $headers[':scheme'][0].'://'.$headers[':authority'][0];
if ($maxPendingPushes <= \count($multi->pushedResponses)) {
$logger && $logger->debug(sprintf('Rejecting pushed response from "%s" for "%s": the queue is full', $origin, $url));
return CURL_PUSH_DENY;
}
// curl before 7.65 doesn't validate the pushed ":authority" header,
// but this is a MUST in the HTTP/2 RFC; let's restrict pushes to the original host,
// ignoring domains mentioned as alt-name in the certificate for now (same as curl).
@ -378,6 +386,12 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface
return CURL_PUSH_DENY;
}
if ($maxPendingPushes <= \count($multi->pushedResponses)) {
$fifoUrl = key($multi->pushedResponses);
unset($multi->pushedResponses[$fifoUrl]);
$logger && $logger->debug(sprintf('Evicting oldest pushed response: "%s"', $fifoUrl));
}
$url .= $headers[':path'][0];
$logger && $logger->debug(sprintf('Queueing pushed response: "%s"', $url));

View File

@ -15,11 +15,12 @@ use Symfony\Component\HttpClient\TraceableHttpClient;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\DataCollector\DataCollector;
use Symfony\Component\HttpKernel\DataCollector\LateDataCollectorInterface;
/**
* @author Jérémy Romey <jeremy@free-agent.fr>
*/
final class HttpClientDataCollector extends DataCollector
final class HttpClientDataCollector extends DataCollector implements LateDataCollectorInterface
{
/**
* @var TraceableHttpClient[]
@ -38,7 +39,7 @@ final class HttpClientDataCollector extends DataCollector
*/
public function collect(Request $request, Response $response/*, \Throwable $exception = null*/)
{
$this->initData();
$this->reset();
foreach ($this->clients as $name => $client) {
[$errorCount, $traces] = $this->collectOnClient($client);
@ -53,6 +54,13 @@ final class HttpClientDataCollector extends DataCollector
}
}
public function lateCollect()
{
foreach ($this->clients as $client) {
$client->reset();
}
}
public function getClients(): array
{
return $this->data['clients'] ?? [];
@ -68,17 +76,6 @@ final class HttpClientDataCollector extends DataCollector
return $this->data['error_count'] ?? 0;
}
/**
* {@inheritdoc}
*/
public function reset()
{
$this->initData();
foreach ($this->clients as $client) {
$client->reset();
}
}
/**
* {@inheritdoc}
*/
@ -87,7 +84,7 @@ final class HttpClientDataCollector extends DataCollector
return 'http_client';
}
private function initData()
public function reset()
{
$this->data = [
'clients' => [],

View File

@ -228,15 +228,7 @@ final class CurlResponse implements ResponseInterface
} finally {
$this->close();
// Clear local caches when the only remaining handles are about pushed responses
if (!$this->multi->openHandles) {
if ($this->logger) {
foreach ($this->multi->pushedResponses as $url => $response) {
$this->logger->debug(sprintf('Unused pushed response: "%s"', $url));
}
}
$this->multi->pushedResponses = [];
// Schedule DNS cache eviction for the next request
$this->multi->dnsCache->evictions = $this->multi->dnsCache->evictions ?: $this->multi->dnsCache->removals;
$this->multi->dnsCache->removals = $this->multi->dnsCache->hostnames = [];

View File

@ -15,13 +15,14 @@ use Symfony\Component\HttpClient\Exception\InvalidArgumentException;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;
use Symfony\Contracts\HttpClient\ResponseStreamInterface;
use Symfony\Contracts\Service\ResetInterface;
/**
* Auto-configure the default options based on the requested URL.
*
* @author Anthony Martin <anthony.martin@sensiolabs.com>
*/
class ScopingHttpClient implements HttpClientInterface
class ScopingHttpClient implements HttpClientInterface, ResetInterface
{
use HttpClientTrait;
@ -90,4 +91,11 @@ class ScopingHttpClient implements HttpClientInterface
{
return $this->client->stream($responses, $timeout);
}
public function reset()
{
if ($this->client instanceof ResetInterface) {
$this->client->reset();
}
}
}

View File

@ -13,13 +13,23 @@ namespace Symfony\Component\HttpClient\Tests;
use Psr\Log\AbstractLogger;
use Symfony\Component\HttpClient\CurlHttpClient;
use Symfony\Component\Process\Exception\ProcessFailedException;
use Symfony\Component\Process\Process;
use Symfony\Contracts\HttpClient\HttpClientInterface;
/*
Tests for HTTP2 Push need a recent version of both PHP and curl. This docker command should run them:
docker run -it --rm -v $(pwd):/app -v /path/to/vulcain:/usr/local/bin/vulcain -w /app php:7.3-alpine ./phpunit src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php --filter testHttp2Push
The vulcain binary can be found at https://github.com/symfony/binary-utils/releases/download/v0.1/vulcain_0.1.3_Linux_x86_64.tar.gz - see https://github.com/dunglas/vulcain for source
*/
/**
* @requires extension curl
*/
class CurlHttpClientTest extends HttpClientTestCase
{
private static $vulcainStarted = false;
protected function getHttpClient(string $testCase): HttpClientInterface
{
return new CurlHttpClient();
@ -28,7 +38,81 @@ class CurlHttpClientTest extends HttpClientTestCase
/**
* @requires PHP 7.2.17
*/
public function testHttp2Push()
public function testHttp2PushVulcain()
{
$client = $this->getVulcainClient();
$logger = new TestLogger();
$client->setLogger($logger);
$responseAsArray = $client->request('GET', 'https://127.0.0.1:3000/json', [
'headers' => [
'Preload' => '/documents/*/id',
],
])->toArray();
foreach ($responseAsArray['documents'] as $document) {
$client->request('GET', 'https://127.0.0.1:3000'.$document['id'])->toArray();
}
$client->reset();
$expected = [
'Request: "GET https://127.0.0.1:3000/json"',
'Queueing pushed response: "https://127.0.0.1:3000/json/1"',
'Queueing pushed response: "https://127.0.0.1:3000/json/2"',
'Queueing pushed response: "https://127.0.0.1:3000/json/3"',
'Response: "200 https://127.0.0.1:3000/json"',
'Accepting pushed response: "GET https://127.0.0.1:3000/json/1"',
'Response: "200 https://127.0.0.1:3000/json/1"',
'Accepting pushed response: "GET https://127.0.0.1:3000/json/2"',
'Response: "200 https://127.0.0.1:3000/json/2"',
'Accepting pushed response: "GET https://127.0.0.1:3000/json/3"',
'Response: "200 https://127.0.0.1:3000/json/3"',
];
$this->assertSame($expected, $logger->logs);
}
/**
* @requires PHP 7.2.17
*/
public function testHttp2PushVulcainWithUnusedResponse()
{
$client = $this->getVulcainClient();
$logger = new TestLogger();
$client->setLogger($logger);
$responseAsArray = $client->request('GET', 'https://127.0.0.1:3000/json', [
'headers' => [
'Preload' => '/documents/*/id',
],
])->toArray();
$i = 0;
foreach ($responseAsArray['documents'] as $document) {
$client->request('GET', 'https://127.0.0.1:3000'.$document['id'])->toArray();
if (++$i >= 2) {
break;
}
}
$client->reset();
$expected = [
'Request: "GET https://127.0.0.1:3000/json"',
'Queueing pushed response: "https://127.0.0.1:3000/json/1"',
'Queueing pushed response: "https://127.0.0.1:3000/json/2"',
'Queueing pushed response: "https://127.0.0.1:3000/json/3"',
'Response: "200 https://127.0.0.1:3000/json"',
'Accepting pushed response: "GET https://127.0.0.1:3000/json/1"',
'Response: "200 https://127.0.0.1:3000/json/1"',
'Accepting pushed response: "GET https://127.0.0.1:3000/json/2"',
'Response: "200 https://127.0.0.1:3000/json/2"',
'Unused pushed response: "https://127.0.0.1:3000/json/3"',
];
$this->assertSame($expected, $logger->logs);
}
private function getVulcainClient(): CurlHttpClient
{
if (\PHP_VERSION_ID >= 70300 && \PHP_VERSION_ID < 70304) {
$this->markTestSkipped('PHP 7.3.0 to 7.3.3 don\'t support HTTP/2 PUSH');
@ -38,32 +122,44 @@ class CurlHttpClientTest extends HttpClientTestCase
$this->markTestSkipped('curl <7.61 is used or it is not compiled with support for HTTP/2 PUSH');
}
$logger = new class() extends AbstractLogger {
public $logs = [];
$client = new CurlHttpClient(['verify_peer' => false, 'verify_host' => false]);
public function log($level, $message, array $context = []): void
{
$this->logs[] = $message;
}
};
if (static::$vulcainStarted) {
return $client;
}
$client = new CurlHttpClient([], 6, 2);
$client->setLogger($logger);
if (200 !== $client->request('GET', 'http://127.0.0.1:8057/json')->getStatusCode()) {
$this->markTestSkipped('symfony/http-client-contracts >= 2.0.1 required');
}
$index = $client->request('GET', 'https://http2.akamai.com/');
$index->getContent();
$process = new Process(['vulcain'], null, [
'DEBUG' => 1,
'UPSTREAM' => 'http://127.0.0.1:8057',
'ADDR' => ':3000',
'KEY_FILE' => __DIR__.'/Fixtures/tls/server.key',
'CERT_FILE' => __DIR__.'/Fixtures/tls/server.crt',
]);
$process->start();
$css = $client->request('GET', 'https://http2.akamai.com/resources/push.css');
register_shutdown_function([$process, 'stop']);
sleep('\\' === \DIRECTORY_SEPARATOR ? 10 : 1);
$css->getHeaders();
if (!$process->isRunning()) {
throw new ProcessFailedException($process);
}
$expected = [
'Request: "GET https://http2.akamai.com/"',
'Queueing pushed response: "https://http2.akamai.com/resources/push.css"',
'Response: "200 https://http2.akamai.com/"',
'Accepting pushed response: "GET https://http2.akamai.com/resources/push.css"',
'Response: "200 https://http2.akamai.com/resources/push.css"',
];
$this->assertSame($expected, $logger->logs);
static::$vulcainStarted = true;
return $client;
}
}
class TestLogger extends AbstractLogger
{
public $logs = [];
public function log($level, $message, array $context = []): void
{
$this->logs[] = $message;
}
}

View File

@ -0,0 +1,20 @@
-----BEGIN CERTIFICATE-----
MIIDPjCCAiYCCQDpVvfmCZt2GzANBgkqhkiG9w0BAQsFADBhMQswCQYDVQQGEwJV
UzEUMBIGA1UEBwwLR290aGFtIENpdHkxEjAQBgNVBAMMCWxvY2FsaG9zdDEoMCYG
CSqGSIb3DQEJARYZZHVuZ2xhcyttZXJjdXJlQGdtYWlsLmNvbTAeFw0xOTAxMjMx
NTUzMzlaFw0yOTAxMjAxNTUzMzlaMGExCzAJBgNVBAYTAlVTMRQwEgYDVQQHDAtH
b3RoYW0gQ2l0eTESMBAGA1UEAwwJbG9jYWxob3N0MSgwJgYJKoZIhvcNAQkBFhlk
dW5nbGFzK21lcmN1cmVAZ21haWwuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8A
MIIBCgKCAQEAuKnXkBSJwOwkKfR58wP/yLYW9QFX2THoqN8iffangRmZwc5KLE6F
1S8jYMv3JGiJ95Ij3MezAfuBCdgPqqP8JrR1XwjR1RFZMOL/4U9R9OuMVng04PLw
L6TzKoEtZuExHUWFP0+5AYblgno2hoN/HVuox8m6zQrBNcbhTgDIjP5Hn491d9od
MtS3OxksDLr1UIOUGUWF7MQMN7lsN7rgT5qxoCkcAGAB4GPOA23HMt2zt4afDiI7
lAmuv8MKkTmBCcFe+q+U7o6wMxkjGstzAWRibtwzR4ejPwdO7se23MXCWGPvF16Z
tu1ip+e+waRus9o5UnyGaVPFAw8iCTC/KwIDAQABMA0GCSqGSIb3DQEBCwUAA4IB
AQB42AW7E57yOky8GpsKLoa9u7okwvvg8CQJ117X8a2MElBGnmMd9tjLa/pXAx2I
bN7jSTSadXiPNYCx4ueiJa4Dwy+C8YkwUbhRf3+mc7Chnz0SXouTjh7OUeeA06jS
W2VAR2pKB0pdJtAkXxIy21Juu8KF5uZqVq1oimgKw2lRUIMdKaqsrVwESk6u5Ojj
3DS40q9DzFnwKGCuZpspvMdWYLscotzLrCbnHp+guWDigEHS3CKzKbNo327nVg6X
7UjqqtPZ2mCsnUx3QTDJsr3gcSqhzmB+Q6I/0Q2Nx/aMmbsNegu+LC3GjFtL59Bv
B8pB/MxID0j47SwPKQghZvb3
-----END CERTIFICATE-----

View File

@ -0,0 +1,27 @@
-----BEGIN RSA PRIVATE KEY-----
MIIEpAIBAAKCAQEAuKnXkBSJwOwkKfR58wP/yLYW9QFX2THoqN8iffangRmZwc5K
LE6F1S8jYMv3JGiJ95Ij3MezAfuBCdgPqqP8JrR1XwjR1RFZMOL/4U9R9OuMVng0
4PLwL6TzKoEtZuExHUWFP0+5AYblgno2hoN/HVuox8m6zQrBNcbhTgDIjP5Hn491
d9odMtS3OxksDLr1UIOUGUWF7MQMN7lsN7rgT5qxoCkcAGAB4GPOA23HMt2zt4af
DiI7lAmuv8MKkTmBCcFe+q+U7o6wMxkjGstzAWRibtwzR4ejPwdO7se23MXCWGPv
F16Ztu1ip+e+waRus9o5UnyGaVPFAw8iCTC/KwIDAQABAoIBAQCczVNGe7oRADMh
EP/wM4ghhUTvHAndWrzFkFs4fJX1UKi34ZQoFTEdOZ6f1fHwj3f/qa8cDNJar5X9
puJ+siotL3Suks2iT83dbhN63SCpiM2sqvuzu3Xp7vWwNOo5fqR2x46CmQ5uVn5S
EbZ09/mbEza5FvmwnB49rLepxY6F8P+vK5ZnCZYS2SHpOxv3U9wG8gmcHRI9ejbC
X9rwuu3oT23bfbJ0tn6Qh8O3R1kXZUUXqnxsn554cZZrXg5+ygbt4HfDVWMLpqy/
5wG0FCpU8QvjF4L8qErP7TZRrWGFtti1RtACbu9LrWvO/74v54td5V28U6kqlDJR
ff4Mi4whAoGBAOBzReQIxGwoYApPyhF+ohvF39JEEXYfkzk94t6hbgyBFBFvqdFY
shT59im2P9LyDvTd5DnCIo52Sj7vM9H80tRjAA0A8okGOczk31ABbH8aZ2orU/0G
EJe4PV4r3bpLO6DKTYsicgRpXI3aHHLvYFXOVNrQKfrKCQ+GFMVuhDdRAoGBANKe
3Dn3XOq7EW42GZey1xUxrfQRJp491KXHvjYt7z7zSiUzqN+mqIqz6ngCjJWbyQsl
Ud9N9U+4rNfYYLHQ0resjxGQRtmooOHlLhT6pEplXDgQb2SmCg2u22SKkkXA7zOV
OFbNryXgkYThsA6ih8LiKM8aFn7zttRSEeTpfye7AoGBALhIzRyiuiuXpuswgdeF
YrJs8A1jB/c1i5qXHlvurT2lCYYbaZHSQj0I0r2CvrqDNhaEzStDIz5XDzTHD4Qd
EjmBo3wJyBkLPI/nZxb4ZE2jrz8znf0EasE3a2OTnrSjqqylDa/sMzM+EtkBORSB
SFaLV45lFeKs2W2eiBVmXTZRAoGAJoA7qaz6Iz6G9SqWixB6GLm4HsFz2cFbueJF
dwn2jf9TMnG7EQcaECDLX5y3rjGIEq2DxdouWaBcmChJpLeTjVfR31gMW4Vjw2dt
gRBAMAlPTkBS3Ictl0q7eCmMi4u1Liy828FFnxrp/uxyjnpPbuSAqTsPma1bYnyO
INY+FDkCgYAe9e39/vXe7Un3ysjqDUW+0OMM+kg4ulhiopzKY+QbHiSWmUUDtvcN
asqrYiX1d59e2ZNiqrlBn86I8549St81bWSrRMNf7R+WVb79RApsABeUaEoyo3lq
0UgOBM8Nt558kaja/YfJf/jwNC1DPuu5x5t38ZcqAkqrZ/HEPkFdGQ==
-----END RSA PRIVATE KEY-----

View File

@ -14,11 +14,12 @@ namespace Symfony\Component\HttpClient;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;
use Symfony\Contracts\HttpClient\ResponseStreamInterface;
use Symfony\Contracts\Service\ResetInterface;
/**
* @author Jérémy Romey <jeremy@free-agent.fr>
*/
final class TraceableHttpClient implements HttpClientInterface
final class TraceableHttpClient implements HttpClientInterface, ResetInterface
{
private $client;
private $tracedRequests = [];
@ -68,6 +69,10 @@ final class TraceableHttpClient implements HttpClientInterface
public function reset()
{
if ($this->client instanceof ResetInterface) {
$this->client->reset();
}
$this->tracedRequests = [];
}
}

View File

@ -24,7 +24,8 @@
"php": "^7.1.3",
"psr/log": "^1.0",
"symfony/http-client-contracts": "^1.1.8|^2",
"symfony/polyfill-php73": "^1.11"
"symfony/polyfill-php73": "^1.11",
"symfony/service-contracts": "^1.0|^2"
},
"require-dev": {
"guzzlehttp/promises": "^1.3.1",
@ -33,8 +34,7 @@
"psr/http-client": "^1.0",
"symfony/dependency-injection": "^4.3|^5.0",
"symfony/http-kernel": "^4.4",
"symfony/process": "^4.2|^5.0",
"symfony/service-contracts": "^1.0|^2"
"symfony/process": "^4.2|^5.0"
},
"conflict": {
"symfony/http-kernel": "<4.4"

View File

@ -155,6 +155,27 @@ switch ($vars['REQUEST_URI']) {
usleep(500);
}
exit;
case '/json':
header("Content-Type: application/json");
echo json_encode([
'documents' => [
['id' => '/json/1'],
['id' => '/json/2'],
['id' => '/json/3'],
],
]);
exit;
case '/json/1':
case '/json/2':
case '/json/3':
header("Content-Type: application/json");
echo json_encode([
'title' => $vars['REQUEST_URI'],
]);
exit;
}
header('Content-Type: application/json', true);

View File

@ -24,8 +24,6 @@ use Symfony\Contracts\HttpClient\HttpClientInterface;
*/
abstract class HttpClientTestCase extends TestCase
{
private static $server;
public static function setUpBeforeClass(): void
{
TestHttpServer::start();

View File

@ -19,31 +19,22 @@ use Symfony\Component\Process\Process;
*/
class TestHttpServer
{
private static $server;
private static $started;
public static function start()
{
if (null !== self::$server) {
if (self::$started) {
return;
}
$finder = new PhpExecutableFinder();
$process = new Process(array_merge([$finder->find(false)], $finder->findArguments(), ['-dopcache.enable=0', '-dvariables_order=EGPCS', '-S', '127.0.0.1:8057']));
$process->setWorkingDirectory(__DIR__.'/Fixtures/web');
$process->setTimeout(300);
$process->start();
self::$server = new class() {
public $process;
public function __destruct()
{
$this->process->stop();
}
};
self::$server->process = $process;
register_shutdown_function([$process, 'stop']);
sleep('\\' === \DIRECTORY_SEPARATOR ? 10 : 1);
self::$started = true;
}
}