Merge branch '5.1' into 5.x

* 5.1:
  [HttpClient] fix reading the body after a ClientException
  [HttpClient] fix tests and merge
This commit is contained in:
Nicolas Grekas 2020-10-12 18:54:56 +02:00
commit 444626bbfc
7 changed files with 100 additions and 67 deletions

View File

@ -0,0 +1,40 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\HttpClient\Internal;
/**
* @author Nicolas Grekas <p@tchwork.com>
*
* @internal
*/
final class Canary
{
private $canceller;
public function __construct(\Closure $canceller)
{
$this->canceller = $canceller;
}
public function cancel()
{
if (($canceller = $this->canceller) instanceof \Closure) {
$this->canceller = null;
$canceller();
}
}
public function __destruct()
{
$this->cancel();
}
}

View File

@ -30,6 +30,7 @@ use Symfony\Component\HttpClient\HttpClientTrait;
use Symfony\Component\HttpClient\Internal\AmpBody;
use Symfony\Component\HttpClient\Internal\AmpCanary;
use Symfony\Component\HttpClient\Internal\AmpClientState;
use Symfony\Component\HttpClient\Internal\Canary;
use Symfony\Component\HttpClient\Internal\ClientState;
use Symfony\Contracts\HttpClient\ResponseInterface;
@ -127,6 +128,11 @@ final class AmpResponse implements ResponseInterface, StreamableInterface
$multi->openHandles[$id] = $id;
++$multi->responseCount;
$this->canary = new Canary(static function () use ($canceller, $multi, $id) {
$canceller->cancel();
unset($multi->openHandles[$id], $multi->handlesActivity[$id]);
});
}
/**
@ -142,29 +148,14 @@ final class AmpResponse implements ResponseInterface, StreamableInterface
try {
$this->doDestruct();
} finally {
$multi = clone $this->multi;
$this->close();
// Clear the DNS cache when all requests completed
if (0 >= --$this->multi->responseCount) {
$this->multi->responseCount = 0;
$this->multi->dnsCache = [];
}
$this->multi = $multi;
}
}
/**
* {@inheritdoc}
*/
private function close(): void
{
$this->canceller->cancel();
unset($this->multi->openHandles[$this->id], $this->multi->handlesActivity[$this->id]);
}
/**
* {@inheritdoc}
*/

View File

@ -15,6 +15,7 @@ use Psr\Log\LoggerInterface;
use Symfony\Component\HttpClient\Chunk\FirstChunk;
use Symfony\Component\HttpClient\Chunk\InformationalChunk;
use Symfony\Component\HttpClient\Exception\TransportException;
use Symfony\Component\HttpClient\Internal\Canary;
use Symfony\Component\HttpClient\Internal\ClientState;
use Symfony\Component\HttpClient\Internal\CurlClientState;
use Symfony\Contracts\HttpClient\ResponseInterface;
@ -171,6 +172,31 @@ final class CurlResponse implements ResponseInterface, StreamableInterface
// Schedule the request in a non-blocking way
$multi->openHandles[$id] = [$ch, $options];
curl_multi_add_handle($multi->handle, $ch);
$this->canary = new Canary(static function () use ($ch, $multi, $id) {
unset($multi->pauseExpiries[$id], $multi->openHandles[$id], $multi->handlesActivity[$id]);
curl_setopt($ch, \CURLOPT_PRIVATE, '_0');
if (self::$performing) {
return;
}
curl_multi_remove_handle($multi->handle, $ch);
curl_setopt_array($ch, [
\CURLOPT_NOPROGRESS => true,
\CURLOPT_PROGRESSFUNCTION => null,
\CURLOPT_HEADERFUNCTION => null,
\CURLOPT_WRITEFUNCTION => null,
\CURLOPT_READFUNCTION => null,
\CURLOPT_INFILE => null,
]);
if (!$multi->openHandles) {
// Schedule DNS cache eviction for the next request
$multi->dnsCache->evictions = $multi->dnsCache->evictions ?: $multi->dnsCache->removals;
$multi->dnsCache->removals = $multi->dnsCache->hostnames = [];
}
});
}
/**
@ -221,48 +247,11 @@ final class CurlResponse implements ResponseInterface, StreamableInterface
public function __destruct()
{
try {
if (null === $this->timeout) {
return; // Unused pushed response
}
$this->doDestruct();
} finally {
$multi = clone $this->multi;
$this->close();
if (!$this->multi->openHandles) {
// 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 = [];
}
$this->multi = $multi;
}
}
/**
* {@inheritdoc}
*/
private function close(): void
{
unset($this->multi->pauseExpiries[$this->id], $this->multi->openHandles[$this->id], $this->multi->handlesActivity[$this->id]);
curl_setopt($this->handle, \CURLOPT_PRIVATE, '_0');
if (self::$performing) {
return;
if (null === $this->timeout) {
return; // Unused pushed response
}
curl_multi_remove_handle($this->multi->handle, $this->handle);
curl_setopt_array($this->handle, [
\CURLOPT_NOPROGRESS => true,
\CURLOPT_PROGRESSFUNCTION => null,
\CURLOPT_HEADERFUNCTION => null,
\CURLOPT_WRITEFUNCTION => null,
\CURLOPT_READFUNCTION => null,
\CURLOPT_INFILE => null,
]);
$this->doDestruct();
}
/**

View File

@ -14,6 +14,7 @@ namespace Symfony\Component\HttpClient\Response;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpClient\Chunk\FirstChunk;
use Symfony\Component\HttpClient\Exception\TransportException;
use Symfony\Component\HttpClient\Internal\Canary;
use Symfony\Component\HttpClient\Internal\ClientState;
use Symfony\Component\HttpClient\Internal\NativeClientState;
use Symfony\Contracts\HttpClient\ResponseInterface;
@ -45,7 +46,7 @@ final class NativeResponse implements ResponseInterface, StreamableInterface
public function __construct(NativeClientState $multi, $context, string $url, array $options, array &$info, callable $resolver, ?callable $onProgress, ?LoggerInterface $logger)
{
$this->multi = $multi;
$this->id = (int) $context;
$this->id = $id = (int) $context;
$this->context = $context;
$this->url = $url;
$this->logger = $logger;
@ -70,6 +71,13 @@ final class NativeResponse implements ResponseInterface, StreamableInterface
$info['pause_handler'] = static function (float $duration) use (&$pauseExpiry) {
$pauseExpiry = 0 < $duration ? microtime(true) + $duration : 0;
};
$this->canary = new Canary(static function () use ($multi, $id) {
if (null !== ($host = $multi->openHandles[$id][6] ?? null) && 0 >= --$multi->hosts[$host]) {
unset($multi->hosts[$host]);
}
unset($multi->openHandles[$id], $multi->handlesActivity[$id]);
});
}
/**
@ -95,17 +103,11 @@ final class NativeResponse implements ResponseInterface, StreamableInterface
try {
$this->doDestruct();
} finally {
$multi = clone $this->multi;
$this->close();
// Clear the DNS cache when all requests completed
if (0 >= --$this->multi->responseCount) {
$this->multi->responseCount = 0;
$this->multi->dnsCache = [];
}
$this->multi = $multi;
}
}
@ -203,11 +205,8 @@ final class NativeResponse implements ResponseInterface, StreamableInterface
*/
private function close(): void
{
if (null !== ($host = $this->multi->openHandles[$this->id][6] ?? null) && 0 >= --$this->multi->hosts[$host]) {
unset($this->multi->hosts[$host]);
}
unset($this->multi->openHandles[$this->id], $this->multi->handlesActivity[$this->id]);
$this->handle = $this->buffer = $this->onProgress = null;
$this->canary->cancel();
$this->handle = $this->buffer = $this->inflate = $this->onProgress = null;
}
/**

View File

@ -41,6 +41,7 @@ trait TransportResponseTrait
private $timeout = 0;
private $inflate;
private $finalInfo;
private $canary;
private $logger;
/**
@ -81,6 +82,15 @@ trait TransportResponseTrait
$this->close();
}
/**
* Closes the response and all its network handles.
*/
protected function close(): void
{
$this->canary->cancel();
$this->inflate = null;
}
/**
* Adds pending responses to the activity list.
*/

View File

@ -319,6 +319,7 @@ abstract class HttpClientTestCase extends BaseHttpClientTestCase
// The response content-type mustn't be json as that calls getContent
// @see src/Symfony/Component/HttpClient/Exception/HttpExceptionTrait.php:58
$this->assertStringNotContainsString('json', $e->getResponse()->getHeaders(false)['content-type'][0] ?? '');
unset($e);
$r = new \ReflectionProperty($client, 'multi');
$r->setAccessible(true);

View File

@ -66,6 +66,9 @@ switch ($vars['REQUEST_URI']) {
case '/404-gzipped':
header('Content-Type: text/plain', true, 404);
ob_start('ob_gzhandler');
@ob_flush();
flush();
usleep(300000);
echo 'some text';
exit;