Skip to content

Commit 343f824

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '4.4' into 5.3
* 4.4: [HttpClient] fix monitoring responses issued before reset() [Cache] workaround transient test on M1 [HttpClient] Fix dealing with "HTTP/1.1 000 " responses [HttpClient] minor change
2 parents 22e63bc + 7c2cb86 commit 343f824

File tree

6 files changed

+94
-78
lines changed

6 files changed

+94
-78
lines changed

‎CurlHttpClient.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,9 @@ public function stream($responses, float $timeout = null): ResponseStreamInterfa
311311
throw new \TypeError(sprintf('"%s()" expects parameter 1 to be an iterable of CurlResponse objects, "%s" given.', __METHOD__, get_debug_type($responses)));
312312
}
313313

314-
if (\is_resource($this->multi->handle) || $this->multi->handle instanceof \CurlMultiHandle) {
314+
if (\is_resource($mh = $this->multi->handles[0] ?? null) || $mh instanceof \CurlMultiHandle) {
315315
$active = 0;
316-
while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($this->multi->handle, $active)) {
316+
while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($mh, $active)) {
317317
}
318318
}
319319

‎Internal/CurlClientState.php

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323
*/
2424
final class CurlClientState extends ClientState
2525
{
26-
/** @var \CurlMultiHandle|resource */
27-
public $handle;
26+
/** @var array<\CurlMultiHandle|resource> */
27+
public $handles = [];
2828
/** @var PushedResponse[] */
2929
public $pushedResponses = [];
3030
/** @var DnsCache */
@@ -44,20 +44,20 @@ public function __construct(int $maxHostConnections, int $maxPendingPushes)
4444
{
4545
self::$curlVersion = self::$curlVersion ?? curl_version();
4646

47-
$this->handle = curl_multi_init();
47+
array_unshift($this->handles, $mh = curl_multi_init());
4848
$this->dnsCache = new DnsCache();
4949
$this->maxHostConnections = $maxHostConnections;
5050
$this->maxPendingPushes = $maxPendingPushes;
5151

5252
// Don't enable HTTP/1.1 pipelining: it forces responses to be sent in order
5353
if (\defined('CURLPIPE_MULTIPLEX')) {
54-
curl_multi_setopt($this->handle, \CURLMOPT_PIPELINING, \CURLPIPE_MULTIPLEX);
54+
curl_multi_setopt($mh, \CURLMOPT_PIPELINING, \CURLPIPE_MULTIPLEX);
5555
}
5656
if (\defined('CURLMOPT_MAX_HOST_CONNECTIONS')) {
57-
$maxHostConnections = curl_multi_setopt($this->handle, \CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : \PHP_INT_MAX) ? 0 : $maxHostConnections;
57+
$maxHostConnections = curl_multi_setopt($mh, \CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : \PHP_INT_MAX) ? 0 : $maxHostConnections;
5858
}
5959
if (\defined('CURLMOPT_MAXCONNECTS') && 0 < $maxHostConnections) {
60-
curl_multi_setopt($this->handle, \CURLMOPT_MAXCONNECTS, $maxHostConnections);
60+
curl_multi_setopt($mh, \CURLMOPT_MAXCONNECTS, $maxHostConnections);
6161
}
6262

6363
// Skip configuring HTTP/2 push when it's unsupported or buggy, see https://bugs.php.net/77535
@@ -70,44 +70,40 @@ public function __construct(int $maxHostConnections, int $maxPendingPushes)
7070
return;
7171
}
7272

73-
curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, function ($parent, $pushed, array $requestHeaders) use ($maxPendingPushes) {
74-
return $this->handlePush($parent, $pushed, $requestHeaders, $maxPendingPushes);
73+
// Clone to prevent a circular reference
74+
$multi = clone $this;
75+
$multi->handles = [$mh];
76+
$multi->pushedResponses = &$this->pushedResponses;
77+
$multi->logger = &$this->logger;
78+
$multi->handlesActivity = &$this->handlesActivity;
79+
$multi->openHandles = &$this->openHandles;
80+
$multi->lastTimeout = &$this->lastTimeout;
81+
82+
curl_multi_setopt($mh, \CURLMOPT_PUSHFUNCTION, static function ($parent, $pushed, array $requestHeaders) use ($multi, $maxPendingPushes) {
83+
return $multi->handlePush($parent, $pushed, $requestHeaders, $maxPendingPushes);
7584
});
7685
}
7786

7887
public function reset()
7988
{
80-
if ($this->logger) {
81-
foreach ($this->pushedResponses as $url => $response) {
82-
$this->logger->debug(sprintf('Unused pushed response: "%s"', $url));
89+
foreach ($this->pushedResponses as $url => $response) {
90+
$this->logger && $this->logger->debug(sprintf('Unused pushed response: "%s"', $url));
91+
92+
foreach ($this->handles as $mh) {
93+
curl_multi_remove_handle($mh, $response->handle);
8394
}
95+
curl_close($response->handle);
8496
}
8597

8698
$this->pushedResponses = [];
8799
$this->dnsCache->evictions = $this->dnsCache->evictions ?: $this->dnsCache->removals;
88100
$this->dnsCache->removals = $this->dnsCache->hostnames = [];
89101

90-
if (\is_resource($this->handle) || $this->handle instanceof \CurlMultiHandle) {
91-
if (\defined('CURLMOPT_PUSHFUNCTION')) {
92-
curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, null);
93-
}
94-
95-
$this->__construct($this->maxHostConnections, $this->maxPendingPushes);
102+
if (\defined('CURLMOPT_PUSHFUNCTION')) {
103+
curl_multi_setopt($this->handles[0], \CURLMOPT_PUSHFUNCTION, null);
96104
}
97-
}
98-
99-
public function __wakeup()
100-
{
101-
throw new \BadMethodCallException('Cannot unserialize '.__CLASS__);
102-
}
103105

104-
public function __destruct()
105-
{
106-
foreach ($this->openHandles as [$ch]) {
107-
if (\is_resource($ch) || $ch instanceof \CurlHandle) {
108-
curl_setopt($ch, \CURLOPT_VERBOSE, false);
109-
}
110-
}
106+
$this->__construct($this->maxHostConnections, $this->maxPendingPushes);
111107
}
112108

113109
private function handlePush($parent, $pushed, array $requestHeaders, int $maxPendingPushes): int

‎Response/CurlResponse.php

Lines changed: 45 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,9 @@ public function __construct(CurlClientState $multi, $ch, array $options = null,
106106
if (0 < $duration) {
107107
if ($execCounter === $multi->execCounter) {
108108
$multi->execCounter = !\is_float($execCounter) ? 1 + $execCounter : \PHP_INT_MIN;
109-
curl_multi_remove_handle($multi->handle, $ch);
109+
foreach ($multi->handles as $mh) {
110+
curl_multi_remove_handle($mh, $ch);
111+
}
110112
}
111113

112114
$lastExpiry = end($multi->pauseExpiries);
@@ -118,7 +120,7 @@ public function __construct(CurlClientState $multi, $ch, array $options = null,
118120
} else {
119121
unset($multi->pauseExpiries[(int) $ch]);
120122
curl_pause($ch, \CURLPAUSE_CONT);
121-
curl_multi_add_handle($multi->handle, $ch);
123+
curl_multi_add_handle($multi->handles[0], $ch);
122124
}
123125
};
124126

@@ -172,7 +174,7 @@ public function __construct(CurlClientState $multi, $ch, array $options = null,
172174
// Schedule the request in a non-blocking way
173175
$multi->lastTimeout = null;
174176
$multi->openHandles[$id] = [$ch, $options];
175-
curl_multi_add_handle($multi->handle, $ch);
177+
curl_multi_add_handle($multi->handles[0], $ch);
176178

177179
$this->canary = new Canary(static function () use ($ch, $multi, $id) {
178180
unset($multi->pauseExpiries[$id], $multi->openHandles[$id], $multi->handlesActivity[$id]);
@@ -182,7 +184,9 @@ public function __construct(CurlClientState $multi, $ch, array $options = null,
182184
return;
183185
}
184186

185-
curl_multi_remove_handle($multi->handle, $ch);
187+
foreach ($multi->handles as $mh) {
188+
curl_multi_remove_handle($mh, $ch);
189+
}
186190
curl_setopt_array($ch, [
187191
\CURLOPT_NOPROGRESS => true,
188192
\CURLOPT_PROGRESSFUNCTION => null,
@@ -264,7 +268,7 @@ public function __destruct()
264268
*/
265269
private static function schedule(self $response, array &$runningResponses): void
266270
{
267-
if (isset($runningResponses[$i = (int) $response->multi->handle])) {
271+
if (isset($runningResponses[$i = (int) $response->multi->handles[0]])) {
268272
$runningResponses[$i][1][$response->id] = $response;
269273
} else {
270274
$runningResponses[$i] = [$response->multi, [$response->id => $response]];
@@ -297,38 +301,47 @@ private static function perform(ClientState $multi, array &$responses = null): v
297301
try {
298302
self::$performing = true;
299303
++$multi->execCounter;
300-
$active = 0;
301-
while (\CURLM_CALL_MULTI_PERFORM === ($err = curl_multi_exec($multi->handle, $active)));
302-
303-
if (\CURLM_OK !== $err) {
304-
throw new TransportException(curl_multi_strerror($err));
305-
}
306304

307-
while ($info = curl_multi_info_read($multi->handle)) {
308-
if (\CURLMSG_DONE !== $info['msg']) {
309-
continue;
305+
foreach ($multi->handles as $i => $mh) {
306+
$active = 0;
307+
while (\CURLM_CALL_MULTI_PERFORM === ($err = curl_multi_exec($mh, $active))) {
310308
}
311-
$result = $info['result'];
312-
$id = (int) $ch = $info['handle'];
313-
$waitFor = @curl_getinfo($ch, \CURLINFO_PRIVATE) ?: '_0';
314309

315-
if (\in_array($result, [\CURLE_SEND_ERROR, \CURLE_RECV_ERROR, /*CURLE_HTTP2*/ 16, /*CURLE_HTTP2_STREAM*/ 92], true) && $waitFor[1] && 'C' !== $waitFor[0]) {
316-
curl_multi_remove_handle($multi->handle, $ch);
317-
$waitFor[1] = (string) ((int) $waitFor[1] - 1); // decrement the retry counter
318-
curl_setopt($ch, \CURLOPT_PRIVATE, $waitFor);
319-
curl_setopt($ch, \CURLOPT_FORBID_REUSE, true);
310+
if (\CURLM_OK !== $err) {
311+
throw new TransportException(curl_multi_strerror($err));
312+
}
320313

321-
if (0 === curl_multi_add_handle($multi->handle, $ch)) {
314+
while ($info = curl_multi_info_read($mh)) {
315+
if (\CURLMSG_DONE !== $info['msg']) {
322316
continue;
323317
}
324-
}
318+
$result = $info['result'];
319+
$id = (int) $ch = $info['handle'];
320+
$waitFor = @curl_getinfo($ch, \CURLINFO_PRIVATE) ?: '_0';
321+
322+
if (\in_array($result, [\CURLE_SEND_ERROR, \CURLE_RECV_ERROR, /*CURLE_HTTP2*/ 16, /*CURLE_HTTP2_STREAM*/ 92], true) && $waitFor[1] && 'C' !== $waitFor[0]) {
323+
curl_multi_remove_handle($mh, $ch);
324+
$waitFor[1] = (string) ((int) $waitFor[1] - 1); // decrement the retry counter
325+
curl_setopt($ch, \CURLOPT_PRIVATE, $waitFor);
326+
curl_setopt($ch, \CURLOPT_FORBID_REUSE, true);
327+
328+
if (0 === curl_multi_add_handle($mh, $ch)) {
329+
continue;
330+
}
331+
}
325332

326-
if (\CURLE_RECV_ERROR === $result && 'H' === $waitFor[0] && 400 <= ($responses[(int) $ch]->info['http_code'] ?? 0)) {
327-
$multi->handlesActivity[$id][] = new FirstChunk();
333+
if (\CURLE_RECV_ERROR === $result && 'H' === $waitFor[0] && 400 <= ($responses[(int) $ch]->info['http_code'] ?? 0)) {
334+
$multi->handlesActivity[$id][] = new FirstChunk();
335+
}
336+
337+
$multi->handlesActivity[$id][] = null;
338+
$multi->handlesActivity[$id][] = \in_array($result, [\CURLE_OK, \CURLE_TOO_MANY_REDIRECTS], true) || '_0' === $waitFor || curl_getinfo($ch, \CURLINFO_SIZE_DOWNLOAD) === curl_getinfo($ch, \CURLINFO_CONTENT_LENGTH_DOWNLOAD) ? null : new TransportException(sprintf('%s for "%s".', curl_strerror($result), curl_getinfo($ch, \CURLINFO_EFFECTIVE_URL)));
328339
}
329340

330-
$multi->handlesActivity[$id][] = null;
331-
$multi->handlesActivity[$id][] = \in_array($result, [\CURLE_OK, \CURLE_TOO_MANY_REDIRECTS], true) || '_0' === $waitFor || curl_getinfo($ch, \CURLINFO_SIZE_DOWNLOAD) === curl_getinfo($ch, \CURLINFO_CONTENT_LENGTH_DOWNLOAD) ? null : new TransportException(sprintf('%s for "%s".', curl_strerror($result), curl_getinfo($ch, \CURLINFO_EFFECTIVE_URL)));
341+
if (!$active && 0 < $i) {
342+
curl_multi_close($mh);
343+
unset($multi->handles[$i]);
344+
}
332345
}
333346
} finally {
334347
self::$performing = false;
@@ -358,11 +371,11 @@ private static function select(ClientState $multi, float $timeout): int
358371

359372
unset($multi->pauseExpiries[$id]);
360373
curl_pause($multi->openHandles[$id][0], \CURLPAUSE_CONT);
361-
curl_multi_add_handle($multi->handle, $multi->openHandles[$id][0]);
374+
curl_multi_add_handle($multi->handles[0], $multi->openHandles[$id][0]);
362375
}
363376
}
364377

365-
if (0 !== $selected = curl_multi_select($multi->handle, $timeout)) {
378+
if (0 !== $selected = curl_multi_select($multi->handles[array_key_last($multi->handles)], $timeout)) {
366379
return $selected;
367380
}
368381

@@ -385,15 +398,8 @@ private static function parseHeaderLine($ch, string $data, array &$info, array &
385398
}
386399

387400
if ('' !== $data) {
388-
try {
389-
// Regular header line: add it to the list
390-
self::addResponseHeaders([$data], $info, $headers);
391-
} catch (TransportException $e) {
392-
$multi->handlesActivity[$id][] = null;
393-
$multi->handlesActivity[$id][] = $e;
394-
395-
return \strlen($data);
396-
}
401+
// Regular header line: add it to the list
402+
self::addResponseHeaders([$data], $info, $headers);
397403

398404
if (!str_starts_with($data, 'HTTP/')) {
399405
if (0 === stripos($data, 'Location:')) {

‎Response/TransportResponseTrait.php

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ abstract protected static function select(ClientState $multi, float $timeout): i
109109
private static function addResponseHeaders(array $responseHeaders, array &$info, array &$headers, string &$debug = ''): void
110110
{
111111
foreach ($responseHeaders as $h) {
112-
if (11 <= \strlen($h) && '/' === $h[4] && preg_match('#^HTTP/\d+(?:\.\d+)? ([1-9]\d\d)(?: |$)#', $h, $m)) {
112+
if (11 <= \strlen($h) && '/' === $h[4] && preg_match('#^HTTP/\d+(?:\.\d+)? (\d\d\d)(?: |$)#', $h, $m)) {
113113
if ($headers) {
114114
$debug .= "< \r\n";
115115
$headers = [];
@@ -124,10 +124,6 @@ private static function addResponseHeaders(array $responseHeaders, array &$info,
124124
}
125125

126126
$debug .= "< \r\n";
127-
128-
if (!$info['http_code']) {
129-
throw new TransportException(sprintf('Invalid or missing HTTP status line for "%s".', implode('', $info['url'])));
130-
}
131127
}
132128

133129
/**

‎Tests/CurlHttpClientTest.php

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,20 @@ public function testHandleIsReinitOnReset()
6666
$r = new \ReflectionProperty($httpClient, 'multi');
6767
$r->setAccessible(true);
6868
$clientState = $r->getValue($httpClient);
69-
$initialHandleId = (int) $clientState->handle;
69+
$initialHandleId = (int) $clientState->handles[0];
7070
$httpClient->reset();
71-
self::assertNotSame($initialHandleId, (int) $clientState->handle);
71+
self::assertNotSame($initialHandleId, (int) $clientState->handles[0]);
72+
}
73+
74+
public function testProcessAfterReset()
75+
{
76+
$client = $this->getHttpClient(__FUNCTION__);
77+
78+
$response = $client->request('GET', 'http://127.0.0.1:8057/json');
79+
80+
$client->reset();
81+
82+
$this->assertSame(['application/json'], $response->getHeaders()['content-type']);
7283
}
7384

7485
public function testOverridingRefererUsingCurlOptions()

‎Tests/MockHttpClientTest.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,13 @@ public function invalidResponseFactoryProvider()
187187
];
188188
}
189189

190+
public function testZeroStatusCode()
191+
{
192+
$client = new MockHttpClient(new MockResponse('', ['response_headers' => ['HTTP/1.1 000 ']]));
193+
$response = $client->request('GET', 'https://foo.bar');
194+
$this->assertSame(0, $response->getStatusCode());
195+
}
196+
190197
public function testThrowExceptionInBodyGenerator()
191198
{
192199
$mockHttpClient = new MockHttpClient([

0 commit comments

Comments
 (0)