8000 bug #30491 [HttpClient] fixes (nicolas-grekas) · symfony/symfony@d4326b2 · GitHub
[go: up one dir, main page]
Skip to content

Commit d4326b2

Browse files
fabpotfabpot
committed
bug #30491 [HttpClient] fixes (nicolas-grekas)
This PR was merged into the 4.3-dev branch. Discussion ---------- [HttpClient] fixes | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Spotted while playing with the client. There is an issue with 307/308 redirects + streamed body that is not fixed here; use string bodies for now. I'm going to look for a solution in another PR. Commits ------- 3eca2b4 [HttpClient] fixes
2 parents d0d188f + 3eca2b4 commit d4326b2

File tree

8 files changed

+67
-34
lines changed

8 files changed

+67
-34
lines changed

‎src/Symfony/Component/HttpClient/CurlHttpClient.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ public function request(string $method, string $url, array $options = []): Respo
256256
}
257257
}
258258

259-
return new CurlResponse($this->multi, $ch, $options, self::createRedirectResolver($options, $host));
259+
return new CurlResponse($this->multi, $ch, $options, $method, self::createRedirectResolver($options, $host));
260260
}
261261

262262
/**
@@ -361,7 +361,7 @@ private static function createRedirectResolver(array $options, string $host): \C
361361
}
362362

363363
return static function ($ch, string $location) use ($redirectHeaders) {
364-
if ($host = parse_url($location, PHP_URL_HOST)) {
364+
if ($redirectHeaders && $host = parse_url($location, PHP_URL_HOST)) {
365365
$rawHeaders = $redirectHeaders['host'] === $host ? $redirectHeaders['with_auth'] : $redirectHeaders['no_auth'];
366366
curl_setopt($ch, CURLOPT_HTTPHEADER, $rawHeaders);
367367
}

‎src/Symfony/Component/HttpClient/HttpClientTrait.php

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,10 +201,16 @@ private static function normalizeBody($body)
201201
if ($r->isGenerator()) {
202202
$body = $body(self::$CHUNK_SIZE);
203203
$body = function () use ($body) {
204-
$chunk = $body->valid() ? $body->current() : '';
205-
$body->next();
204+
while ($body->valid()) {
205+
$chunk = $body->current();
206+
$body->next();
206207

207-
return $chunk;
208+
if ('' !== $chunk) {
209+
return $chunk;
210+
}
211+
}
212+
213+
return '';
208214
};
209215
}
210216

‎src/Symfony/Component/HttpClient/NativeHttpClient.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ public function request(string $method, string $url, array $options = []): Respo
9797
'raw_headers' => [],
9898
'url' => $url,
9999
'error' => null,
100+
'http_method' => $method,
100101
'http_code' => 0,
101102
'redirect_count' => 0,
102103
'start_time' => 0.0,
@@ -336,8 +337,8 @@ private static function createRedirectResolver(array $options, string $host, ?ar
336337
}
337338
}
338339

339-
return static function (\stdClass $multi, int $statusCode, ?string $location, $context) use ($redirectHeaders, $proxy, $noProxy, &$info, $maxRedirects, $onProgress): ?string {
340-
if (null === $location || $statusCode < 300 || 400 <= $statusCode) {
340+
return static function (\stdClass $multi, ?string $location, $context) use ($redirectHeaders, $proxy, $noProxy, &$info, $maxRedirects, $onProgress): ?string {
341+
if (null === $location || $info['http_code'] < 300 || 400 <= $info['http_code']) {
341342
$info['redirect_url'] = null;
342343

343344
return null;
@@ -356,11 +357,11 @@ private static function createRedirectResolver(array $options, string $host, ?ar
356357
$info['redirect_time'] = $now - $info['start_time'];
357358

358359
// Do like curl and browsers: turn POST to GET on 301, 302 and 303
359-
if (\in_array($statusCode, [301, 302, 303], true)) {
360+
if (\in_array($info['http_code'], [301, 302, 303], true)) {
360361
$options = stream_context_get_options($context)['http'];
361362

362-
if ('POST' === $options['method'] || 303 === $statusCode) {
363-
$options['method'] = 'HEAD' === $options['method'] ? 'HEAD' : 'GET';
363+
if ('POST' === $options['method'] || 303 === $info['http_code']) {
364+
$info['http_method'] = $options['method'] = 'HEAD' === $options['method'] ? 'HEAD' : 'GET';
364365
$options['content'] = '';
365366
$options['header'] = array_filter($options['header'], static function ($h) {
366367
return 0 !== stripos($h, 'Content-Length:') && 0 !== stripos($h, 'Content-Type:');

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

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ final class CurlResponse implements ResponseInterface
2929
/**
3030
* @internal
3131
*/
32-
public function __construct(\stdClass $multi, $ch, array $options = null, callable $resolveRedirect = null)
32+
public function __construct(\stdClass $multi, $ch, array $options = null, string $method = 'GET', callable $resolveRedirect = null)
3333
{
3434
$this->multi = $multi;
3535

@@ -42,9 +42,11 @@ public function __construct(\stdClass $multi, $ch, array $options = null, callab
4242

4343
$this->id = $id = (int) $ch;
4444
$this->timeout = $options['timeout'] ?? null;
45+
$this->info['http_method'] = $method;
4546
$this->info['user_data'] = $options['user_data'] ?? null;
4647
$this->info['start_time'] = $this->info['start_time'] ?? microtime(true);
4748
$info = &$this->info;
49+
$headers = &$this->headers;
4850

4951
if (!$info['raw_headers']) {
5052
// Used to keep track of what we're waiting for
@@ -62,8 +64,8 @@ public function __construct(\stdClass $multi, $ch, array $options = null, callab
6264
$content = ($options['buffer'] ?? true) ? $content : null;
6365
}
6466

65-
curl_setopt($ch, CURLOPT_HEADERFUNCTION, static function ($ch, string $data) use (&$info, $options, $multi, $id, &$location, $resolveRedirect): int {
66-
return self::parseHeaderLine($ch, $data, $info, $options, $multi, $id, $location, $resolveRedirect);
67+
curl_setopt($ch, CURLOPT_HEADERFUNCTION, static function ($ch, string $data) use (&$info, &$headers, $options, $multi, $id, &$location, $resolveRedirect): int {
68+
return self::parseHeaderLine($ch, $data, $info, $headers, $options, $multi, $id, $location, $resolveRedirect);
6769
});
6870

6971
if (null === $options) {
@@ -116,8 +118,6 @@ public function __construct(\stdClass $multi, $ch, array $options = null, callab
116118
curl_setopt($ch, CURLOPT_HEADERFUNCTION, null);
117119
curl_setopt($ch, CURLOPT_READFUNCTION, null);
118120
curl_setopt($ch, CURLOPT_INFILE, null);
119-
120-
$response->addRawHeaders($response->info['raw_headers']);
121121
};
122122

123123
// Schedule the request in a non-blocking way
@@ -243,15 +243,24 @@ protected static function select(\stdClass $multi, float $timeout): int
243243
/**
244244
* Parses header lines as curl yields them to us.
245245
*/
246-
private static function parseHeaderLine($ch, string $data, array &$info, ?array $options, \stdClass $multi, int $id, ?string &$location, ?callable $resolveRedirect): int
246+
private static function parseHeaderLine($ch, string $data, array &$info, array &$headers, ?array $options, \stdClass $multi, int $id, ?string &$location, ?callable $resolveRedirect): int
247247
{
248248
if (!\in_array($waitFor = @curl_getinfo($ch, CURLINFO_PRIVATE), ['headers', 'destruct'], true)) {
249249
return \strlen($data); // Ignore HTTP trailers
250250
}
251251

252252
if ("\r\n" !== $data) {
253253
// Regular header line: add it to the list
254-
$info['raw_headers'][] = substr($data, 0, -2);
254+
self::addRawHeaders([substr($data, 0, -2)], $info, $headers);
255+
256+
if (0 === strpos($data, 'HTTP') & 10000 amp;& 300 <= $info['http_code'] && $info['http_code'] < 400) {
257+
if (curl_getinfo($ch, CURLINFO_REDIRECT_COUNT) === $options['max_redirects']) {
258+
curl_setopt($ch, CURLOPT_FOLLOWLOCATION, false);
259+
} elseif (303 === $info['http_code'] || ('POST' === $info['http_method'] && \in_array($info['http_code'], [301, 302], true))) {
260+
$info['http_method'] = 'HEAD' === $info['http_method'] ? 'HEAD' : 'GET';
261+
curl_setopt($ch, CURLOPT_POSTFIELDS, '');
262+
}
263+
}
255264

256265
if (0 === stripos($data, 'Location:')) {
257266
$location = trim(substr($data, 9, -2));

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,10 @@ private function open(): void
111111
// Send request and follow redirects when needed
112112
$this->info['fopen_time'] = microtime(true);
113113
$this->handle = $h = fopen($url, 'r', false, $this->context);
114-
$this->addRawHeaders($http_response_header);
115-
$url = ($this->resolveRedirect)($this->multi, $this->statusCode, $this->headers['location'][0] ?? null, $this->context);
114+
self::addRawHeaders($http_response_header, $this->info, $this->headers);
115+
$url = ($this->resolveRedirect)($this->multi, $this->headers['location'][0] ?? null, $this->context);
116116
} while (null !== $url);
117117
} catch (\Throwable $e) {
118-
$this->statusCode = 0;
119118
$this->close();
120119
$this->multi->handlesActivity[$this->id][] = null;
121120
$this->multi->handlesActivity[$this->id][] = $e;

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
*/
2929
trait ResponseTrait
3030
{
31-
private $statusCode = 0;
3231
private $headers = [];
3332

3433
/**
@@ -43,6 +42,7 @@ trait ResponseTrait
4342

4443
private $info = [
4544
'raw_headers' => [],
45+
'http_code' => 0,
4646
'error' => null,
4747
];
4848

@@ -63,7 +63,7 @@ public function getStatusCode(): int
6363
$this->initializer = null;
6464
}
6565

66-
return $this->statusCode;
66+
return $this->info['http_code'];
6767
}
6868

6969
/**
@@ -141,35 +141,35 @@ abstract protected static function perform(\stdClass $multi, array &$responses):
141141
*/
142142
abstract protected static function select(\stdClass $multi, float $timeout): int;
143143

144-
private function addRawHeaders(array $rawHeaders): void
144+
private static function addRawHeaders(array $rawHeaders, array &$info, array &$headers): void
145145
{
146146
foreach ($rawHeaders as $h) {
147147
if (11 <= \strlen($h) && '/' === $h[4] && preg_match('#^HTTP/\d+(?:\.\d+)? ([12345]\d\d) .*#', $h, $m)) {
148-
$this->headers = [];
149-
$this->info['http_code'] = $this->statusCode = (int) $m[1];
148+
$headers = [];
149+
$info['http_code'] = (int) $m[1];
150150
} elseif (2 === \count($m = explode(':', $h, 2))) {
151-
$this->headers[strtolower($m[0])][] = ltrim($m[1]);
151+
$headers[strtolower($m[0])][] = ltrim($m[1]);
152152
}
153153

154-
$this->info['raw_headers'][] = $h;
154+
$info['raw_headers'][] = $h;
155155
}
156156

157-
if (!$this->statusCode) {
157+
if (!$info['http_code']) {
158158
throw new TransportException('Invalid or missing HTTP status line.');
159159
}
160160
}
161161

162162
private function checkStatusCode()
163163
{
164-
if (500 <= $this->statusCode) {
164+
if (500 <= $this->info['http_code']) {
165165
throw new ServerException($this);
166166
}
167167

168-
if (400 <= $this->statusCode) {
168+
if (400 <= $this->info['http_code']) {
169169
throw new ClientException($this);
170170
}
171171

172-
if (300 <= $this->statusCode) {
172+
if (300 <= $this->info['http_code']) {
173173
throw new RedirectionException($this);
174174
}
175175
}

‎src/Symfony/Contracts/HttpClient/ResponseInterface.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ public function getContent(bool $throw = true): string;
7171
* - redirect_count - the number of redirects followed while executing the request
7272
* - redirect_url - the resolved location of redirect responses, null otherwise
7373
* - start_time - the time when the request was sent or 0.0 when it's pending
74+
* - http_method - the HTTP verb of the last request
7475
* - http_code - the last response code or 0 when it is not known yet
7576
* - error - the error message when the transfer was aborted, null otherwise
7677
* - data - the value of the "data" request option, null if not set

‎src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,9 @@ public function testRedirects()
214214
$client = $this->getHttpClient();
215215
$response = $client->request('POST', 'http://localhost:8057/301', [
216216
'auth' => 'foo:bar',
217-
'body' => 'foo=bar',
217+
'body' => function () {
218+
yield 'foo=bar';
219+
},
218220
]);
219221

220222
$body = json_decode($response->getContent(), true);
@@ -236,7 +238,9 @@ public function testRedirects()
236238
'Content-Type: application/json',
237239
];
238240

239-
$filteredHeaders = array_intersect($expected, $response->getInfo('raw_headers'));
241+
$filteredHeaders = array_values(array_filter($response->getInfo('raw_headers'), function ($h) {
242+
return \in_array(substr($h, 0, 4), ['HTTP', 'Loca', 'Cont'], true) && 'Content-Encoding: gzip' !== $h;
243+
}));
240244

241245
$this->assertSame($expected, $filteredHeaders);
242246
}
@@ -261,6 +265,16 @@ public function testRelativeRedirects()
261265
public function testRedirect307()
262266
{
263267
$client = $this->getHttpClient();
268+
269+
$response = $client->request('POST', 'http://localhost:8057/307', [
270+
'body' => function () {
271+
yield 'foo=bar';
272+
},
273+
'max_redirects' => 0,
274+
]);
275+
276+
$this->assertSame(307, $response->getStatusCode());
277+
264278
$response = $client->request('POST', 'http://localhost:8057/307', [
265279
'body' => 'foo=bar',
266280
]);
@@ -297,7 +311,9 @@ public function testMaxRedirects()
297311
'Content-Type: application/json',
298312
];
299313

300-
$filteredHeaders = array_intersect($expected, $response->getInfo('raw_headers'));
314+
$filteredHeaders = array_values(array_filter($response->getInfo('raw_headers'), function ($h) {
315+
return \in_array(substr($h, 0, 4), ['HTTP', 'Loca', 'Cont'], true);
316+
}));
301317

302318
$this->assertSame($expected, $filteredHeaders);
303319
}
@@ -416,6 +432,7 @@ public function testPostCallback()
416432
$response = $client->request('POST', 'http://localhost:8057/post', [
417433
'body' => function () {
418434
yield 'foo';
435+
yield '';
419436
yield '=';
420437
yield '0123456789';
421438
},

0 commit comments

Comments
 (0)
0