10000 bug #59631 [HttpClient] Fix processing a NativeResponse after its cli… · mathroc/symfony@7e396bb · GitHub
[go: up one dir, main page]
Skip to content

Commit 7e396bb

Browse files
nicolas-grekasnicolas-grekas
committed
bug symfony#59631 [HttpClient] Fix processing a NativeResponse after its client has been reset (Jean-Beru)
This PR was merged into the 6.4 branch. Discussion ---------- [HttpClient] Fix processing a NativeResponse after its client has been reset | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | | License | MIT An exception is thrown in dev when a response is processed before streaming it. It is due to the HttpClientDataCollector [which resets the client](https://github.com/symfony/symfony/blob/6.4/src/Symfony/Component/HttpClient/DataCollector/HttpClientDataCollector.php#L41) between the first access to the response and the next ones. This operation resets the `NativeClientState` and its `hosts` property which is used in the `NativeReponse` when the stream is cancelled or completed. Reproducer ```php /** `@var` NativeHttpClient $appClient **/ $response = $appClient->request('GET', 'https://google.com'); return new StreamedResponse( function () use ($appClient, $response): void { foreach ($appClient->stream($response) as $chunk) { echo $chunk->getContent(); // it fails because this code is executed AFTER collecting data and so, the client reset flush(); } }, $response->getStatusCode(), // the status code is retrieved BEFORE the reset done by the HttpClientDataCollector ); ``` Commits ------- 0a4521e [HttpClient] Fix processing a NativeResponse after its client has been reset
2 parents 4d4859a + 0a4521e commit 7e396bb

File tree

2 files changed

+19
-6
lines changed

2 files changed

+19
-6
lines changed

‎src/Symfony/Component/HttpClient/Response/NativeResponse.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public function __construct(NativeClientState $multi, $context, string $url, arr
7979
};
8080

8181
$this->canary = new Canary(static function () use ($multi, $id) {
82-
if (null !== ($host = $multi->openHandles[$id][6] ?? null) && 0 >= --$multi->hosts[$host]) {
82+
if (null !== ($host = $multi->openHandles[$id][6] ?? null) && isset($multi->hosts[$host]) && 0 >= --$multi->hosts[$host]) {
8383
unset($multi->hosts[$host]);
8484
}
8585
unset($multi->openHandles[$id], $multi->handlesActivity[$id]);
@@ -123,7 +123,7 @@ private function open(): void
123123
throw new TransportException($msg);
124124
}
125125

126-
$this->logger?->info(sprintf('%s for "%s".', $msg, $url ?? $this->url));
126+
$this->logger?->info(\sprintf('%s for "%s".', $msg, $url ?? $this->url));
127127
});
128128

129129
try {
@@ -142,7 +142,7 @@ private function open(): void
142142
$this->info['request_header'] = $this->info['url']['path'].$this->info['url']['query'];
143143
}
144144

145-
$this->info['request_header'] = sprintf("> %s %s HTTP/%s \r\n", $context['http']['method'], $this->info['request_header'], $context['http']['protocol_version']);
145+
$this->info['request_header'] = \sprintf("> %s %s HTTP/%s \r\n", $context['http']['method'], $this->info['request_header'], $context['http']['protocol_version']);
146146
$this->info['request_header'] .= implode("\r\n", $context['http']['header'])."\r\n\r\n";
147147

148148
if (\array_key_exists('peer_name', $context['ssl']) && null === $context['ssl']['peer_name']) {
@@ -159,7 +159,7 @@ private function open(): void
159159
break;
160160
}
161161

162-
$this->logger?->info(sprintf('Redirecting: "%s %s"', $this->info['http_code'], $url ?? $this->url));
162+
$this->logger?->info(\sprintf('Redirecting: "%s %s"', $this->info['http_code'], $url ?? $this->url));
163163
}
164164
} catch (\Throwable $e) {
165165
$this->close();
@@ -294,15 +294,15 @@ private static function perform(ClientState $multi, ?array &$responses = null):
294294

295295
if (null === $e) {
296296
if (0 < $remaining) {
297-
$e = new TransportException(sprintf('Transfer closed with %s bytes remaining to read.', $remaining));
297+
$e = new TransportException(\sprintf('Transfer closed with %s bytes remaining to read.', $remaining));
298298
} elseif (-1 === $remaining && fwrite($buffer, '-') && '' !== stream_get_contents($buffer, -1, 0)) {
299299
$e = new TransportException('Transfer closed with outstanding data remaining from chunked response.');
300300
}
301301
}
302302

303303
$multi->handlesActivity[$i][] = null;
304304
$multi->handlesActivity[$i][] = $e;
305-
if (null !== ($host = $multi->openHandles[$i][6] ?? null) && 0 >= --$multi->hosts[$host]) {
305+
if (null !== ($host = $multi->openHandles[$i][6] ?? null) && isset($multi->hosts[$host]) && 0 >= --$multi->hosts[$host]) {
306306
unset($multi->hosts[$host]);
307307
}
308308
unset($multi->openHandles[$i]);

‎src/Symfony/Component/HttpClient/Tests/HttpClientTestCase.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,4 +695,17 @@ public function testPostToGetRedirect(int $status)
695695
$this->assertSame('GET', $body['REQUEST_METHOD']);
696696
$this->assertSame('/', $body['REQUEST_URI']);
697697
}
698+
699+
public function testResponseCanBeProcessedAfterClientReset()
700+
{
701+
$client = $this->getHttpClient(__FUNCTION__);
702+
$response = $client->request('GET', 'http://127.0.0.1:8057/timeout-body');
703+
$stream = $client->stream($response);
704+
705+
$response->getStatusCode();
706+
$client->reset();
707+
$stream->current();
708+
709+
$this->addToAssertionCount(1);
710+
}
698711
}

0 commit comments

Comments
 (0)
0