Skip to content

Commit

Permalink
[HttpClient] Lazily initialize CurlClientState
Browse files Browse the repository at this point in the history
  • Loading branch information
arjenm authored and nicolas-grekas committed Mar 12, 2024
1 parent c6506c4 commit 6c568df
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 21 deletions.
61 changes: 42 additions & 19 deletions src/Symfony/Component/HttpClient/CurlHttpClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface,
*/
private $logger;

private int $maxHostConnections;
private int $maxPendingPushes;

/**
* An internal object to share state between the client and its responses.
*
Expand All @@ -70,18 +73,22 @@ public function __construct(array $defaultOptions = [], int $maxHostConnections
throw new \LogicException('You cannot use the "Symfony\Component\HttpClient\CurlHttpClient" as the "curl" extension is not installed.');
}

$this->maxHostConnections = $maxHostConnections;
$this->maxPendingPushes = $maxPendingPushes;

$this->defaultOptions['buffer'] = $this->defaultOptions['buffer'] ?? \Closure::fromCallable([__CLASS__, 'shouldBuffer']);

if ($defaultOptions) {
[, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, $this->defaultOptions);
}

$this->multi = new CurlClientState($maxHostConnections, $maxPendingPushes);
}

public function setLogger(LoggerInterface $logger): void
{
$this->logger = $this->multi->logger = $logger;
$this->logger = $logger;
if (isset($this->multi)) {
$this->multi->logger = $logger;
}
}

/**
Expand All @@ -91,6 +98,8 @@ public function setLogger(LoggerInterface $logger): void
*/
public function request(string $method, string $url, array $options = []): ResponseInterface
{
$multi = $this->ensureState();

[$url, $options] = self::prepareRequest($method, $url, $options, $this->defaultOptions);
$scheme = $url['scheme'];
$authority = $url['authority'];
Expand Down Expand Up @@ -161,25 +170,25 @@ public function request(string $method, string $url, array $options = []): Respo
}

// curl's resolve feature varies by host:port but ours varies by host only, let's handle this with our own DNS map
if (isset($this->multi->dnsCache->hostnames[$host])) {
$options['resolve'] += [$host => $this->multi->dnsCache->hostnames[$host]];
if (isset($multi->dnsCache->hostnames[$host])) {
$options['resolve'] += [$host => $multi->dnsCache->hostnames[$host]];

Check failure on line 174 in src/Symfony/Component/HttpClient/CurlHttpClient.php

View workflow job for this annotation

GitHub Actions / Psalm

InvalidArrayOffset

src/Symfony/Component/HttpClient/CurlHttpClient.php:174:36: InvalidArrayOffset: Cannot create offset of type false|null|string, expecting array-key (see https://psalm.dev/115)

Check failure on line 174 in src/Symfony/Component/HttpClient/CurlHttpClient.php

View workflow job for this annotation

GitHub Actions / Psalm

InvalidArrayOffset

src/Symfony/Component/HttpClient/CurlHttpClient.php:174:36: InvalidArrayOffset: Cannot create offset of type false|null|string, expecting array-key (see https://psalm.dev/115)
}

if ($options['resolve'] || $this->multi->dnsCache->evictions) {
if ($options['resolve'] || $multi->dnsCache->evictions) {
// First reset any old DNS cache entries then add the new ones
$resolve = $this->multi->dnsCache->evictions;
$this->multi->dnsCache->evictions = [];
$resolve = $multi->dnsCache->evictions;
$multi->dnsCache->evictions = [];
$port = parse_url($authority, \PHP_URL_PORT) ?: ('http:' === $scheme ? 80 : 443);

if ($resolve && 0x072A00 > CurlClientState::$curlVersion['version_number']) {
// DNS cache removals require curl 7.42 or higher
$this->multi->reset();
$multi->reset();
}

foreach ($options['resolve'] as $host => $ip) {
$resolve[] = null === $ip ? "-$host:$port" : "$host:$port:$ip";
$this->multi->dnsCache->hostnames[$host] = $ip;
$this->multi->dnsCache->removals["-$host:$port"] = "-$host:$port";
$multi->dnsCache->hostnames[$host] = $ip;

Check failure on line 190 in src/Symfony/Component/HttpClient/CurlHttpClient.php

View workflow job for this annotation

GitHub Actions / Psalm

InvalidPropertyAssignmentValue

src/Symfony/Component/HttpClient/CurlHttpClient.php:190:17: InvalidPropertyAssignmentValue: $multi->dnsCache->hostnames with declared type 'array<array-key, string>' cannot be assigned type 'non-empty-array<array-key, mixed|null|string>' (see https://psalm.dev/145)

Check failure on line 190 in src/Symfony/Component/HttpClient/CurlHttpClient.php

View workflow job for this annotation

GitHub Actions / Psalm

InvalidPropertyAssignmentValue

src/Symfony/Component/HttpClient/CurlHttpClient.php:190:17: InvalidPropertyAssignmentValue: $multi->dnsCache->hostnames with declared type 'array<array-key, string>' cannot be assigned type 'non-empty-array<array-key, mixed|null|string>' (see https://psalm.dev/145)
$multi->dnsCache->removals["-$host:$port"] = "-$host:$port";
}

$curlopts[\CURLOPT_RESOLVE] = $resolve;
Expand Down Expand Up @@ -281,16 +290,16 @@ public function request(string $method, string $url, array $options = []): Respo
$curlopts += $options['extra']['curl'];
}

if ($pushedResponse = $this->multi->pushedResponses[$url] ?? null) {
unset($this->multi->pushedResponses[$url]);
if ($pushedResponse = $multi->pushedResponses[$url] ?? null) {
unset($multi->pushedResponses[$url]);

if (self::acceptPushForRequest($method, $options, $pushedResponse)) {
$this->logger && $this->logger->debug(sprintf('Accepting pushed response: "%s %s"', $method, $url));

// Reinitialize the pushed response with request's options
$ch = $pushedResponse->handle;
$pushedResponse = $pushedResponse->response;
$pushedResponse->__construct($this->multi, $url, $options, $this->logger);
$pushedResponse->__construct($multi, $url, $options, $this->logger);
} else {
$this->logger && $this->logger->debug(sprintf('Rejecting pushed response: "%s"', $url));
$pushedResponse = null;
Expand All @@ -300,7 +309,7 @@ public function request(string $method, string $url, array $options = []): Respo
if (!$pushedResponse) {
$ch = curl_init();
$this->logger && $this->logger->info(sprintf('Request: "%s %s"', $method, $url));
$curlopts += [\CURLOPT_SHARE => $this->multi->share];
$curlopts += [\CURLOPT_SHARE => $multi->share];
}

foreach ($curlopts as $opt => $value) {
Expand All @@ -310,7 +319,7 @@ public function request(string $method, string $url, array $options = []): Respo
}
}

return $pushedResponse ?? new CurlResponse($this->multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $host), CurlClientState::$curlVersion['version_number']);
return $pushedResponse ?? new CurlResponse($multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $host), CurlClientState::$curlVersion['version_number']);
}

/**
Expand All @@ -324,9 +333,11 @@ public function stream($responses, ?float $timeout = null): ResponseStreamInterf
throw new \TypeError(sprintf('"%s()" expects parameter 1 to be an iterable of CurlResponse objects, "%s" given.', __METHOD__, get_debug_type($responses)));
}

if (\is_resource($this->multi->handle) || $this->multi->handle instanceof \CurlMultiHandle) {
$multi = $this->ensureState();

if (\is_resource($multi->handle) || $multi->handle instanceof \CurlMultiHandle) {
$active = 0;
while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($this->multi->handle, $active)) {
while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($multi->handle, $active)) {
}
}

Expand All @@ -335,7 +346,9 @@ public function stream($responses, ?float $timeout = null): ResponseStreamInterf

public function reset()
{
$this->multi->reset();
if (isset($this->multi)) {
$this->multi->reset();
}
}

/**
Expand Down Expand Up @@ -439,6 +452,16 @@ private static function createRedirectResolver(array $options, string $host): \C
};
}

private function ensureState(): CurlClientState
{
if (!isset($this->multi)) {
$this->multi = new CurlClientState($this->maxHostConnections, $this->maxPendingPushes);
$this->multi->logger = $this->logger;
}

return $this->multi;
}

private function findConstantName(int $opt): ?string
{
$constants = array_filter(get_defined_constants(), static function ($v, $k) use ($opt) {
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/HttpClient/Tests/CurlHttpClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ public function testHandleIsReinitOnReset()
{
$httpClient = $this->getHttpClient(__FUNCTION__);

$r = new \ReflectionProperty($httpClient, 'multi');
$r = new \ReflectionMethod($httpClient, 'ensureState');
$r->setAccessible(true);
$clientState = $r->getValue($httpClient);
$clientState = $r->invoke($httpClient);
$initialShareId = $clientState->share;
$httpClient->reset();
self::assertNotSame($initialShareId, $clientState->share);
Expand Down

0 comments on commit 6c568df

Please sign in to comment.