Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit d4326b2

Browse filesBrowse 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
Copy full SHA for d4326b2

File tree

8 files changed

+67
-34
lines changed
Filter options

8 files changed

+67
-34
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/CurlHttpClient.php
+2-2Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/HttpClientTrait.php
+9-3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/NativeHttpClient.php
+6-5Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/Response/CurlResponse.php
+16-7Lines 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') && 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/Response/NativeResponse.php
+2-3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/Response/ResponseTrait.php
+11-11Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Contracts/HttpClient/ResponseInterface.php
+1Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php
+20-3Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.