diogo
/
symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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
-------

3eca2b448d [HttpClient] fixes
This commit is contained in:
Fabien Potencier 2019-03-08 18:04:23 +01:00
parent d0d188f0b9 3eca2b448d
commit d4326b238e
8 changed files with 67 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/Symfony/Component/HttpClient/CurlHttpClient.php
View File

@ -256,7 +256,7 @@ final class CurlHttpClient implements HttpClientInterface
}
}
return new CurlResponse($this->multi, $ch, $options, self::createRedirectResolver($options, $host));
return new CurlResponse($this->multi, $ch, $options, $method, self::createRedirectResolver($options, $host));
}
/**
@ -361,7 +361,7 @@ final class CurlHttpClient implements HttpClientInterface
}
return static function ($ch, string $location) use ($redirectHeaders) {
if ($host = parse_url($location, PHP_URL_HOST)) {
if ($redirectHeaders && $host = parse_url($location, PHP_URL_HOST)) {
$rawHeaders = $redirectHeaders['host'] === $host ? $redirectHeaders['with_auth'] : $redirectHeaders['no_auth'];
curl_setopt($ch, CURLOPT_HTTPHEADER, $rawHeaders);
}

12
src/Symfony/Component/HttpClient/HttpClientTrait.php
View File

@ -201,10 +201,16 @@ trait HttpClientTrait
if ($r->isGenerator()) {
$body = $body(self::$CHUNK_SIZE);
$body = function () use ($body) {
$chunk = $body->valid() ? $body->current() : '';
$body->next();
while ($body->valid()) {
$chunk = $body->current();
$body->next();
return $chunk;
if ('' !== $chunk) {
return $chunk;
}
}
return '';
};
}

11
src/Symfony/Component/HttpClient/NativeHttpClient.php
View File

@ -97,6 +97,7 @@ final class NativeHttpClient implements HttpClientInterface
'raw_headers' => [],
'url' => $url,
'error' => null,
'http_method' => $method,
'http_code' => 0,
'redirect_count' => 0,
'start_time' => 0.0,
@ -336,8 +337,8 @@ final class NativeHttpClient implements HttpClientInterface
}
}
return static function (\stdClass $multi, int $statusCode, ?string $location, $context) use ($redirectHeaders, $proxy, $noProxy, &$info, $maxRedirects, $onProgress): ?string {
if (null === $location || $statusCode < 300 || 400 <= $statusCode) {
return static function (\stdClass $multi, ?string $location, $context) use ($redirectHeaders, $proxy, $noProxy, &$info, $maxRedirects, $onProgress): ?string {
if (null === $location || $info['http_code'] < 300 || 400 <= $info['http_code']) {
$info['redirect_url'] = null;
return null;
@ -356,11 +357,11 @@ final class NativeHttpClient implements HttpClientInterface
$info['redirect_time'] = $now - $info['start_time'];
// Do like curl and browsers: turn POST to GET on 301, 302 and 303
if (\in_array($statusCode, [301, 302, 303], true)) {
if (\in_array($info['http_code'], [301, 302, 303], true)) {
$options = stream_context_get_options($context)['http'];
if ('POST' === $options['method'] || 303 === $statusCode) {
$options['method'] = 'HEAD' === $options['method'] ? 'HEAD' : 'GET';
if ('POST' === $options['method'] || 303 === $info['http_code']) {
$info['http_method'] = $options['method'] = 'HEAD' === $options['method'] ? 'HEAD' : 'GET';
$options['content'] = '';
$options['header'] = array_filter($options['header'], static function ($h) {
return 0 !== stripos($h, 'Content-Length:') && 0 !== stripos($h, 'Content-Type:');

23
src/Symfony/Component/HttpClient/Response/CurlResponse.php
View File

@ -29,7 +29,7 @@ final class CurlResponse implements ResponseInterface
/**
* @internal
*/
public function __construct(\stdClass $multi, $ch, array $options = null, callable $resolveRedirect = null)
public function __construct(\stdClass $multi, $ch, array $options = null, string $method = 'GET', callable $resolveRedirect = null)
{
$this->multi = $multi;
@ -42,9 +42,11 @@ final class CurlResponse implements ResponseInterface
$this->id = $id = (int) $ch;
$this->timeout = $options['timeout'] ?? null;
$this->info['http_method'] = $method;
$this->info['user_data'] = $options['user_data'] ?? null;
$this->info['start_time'] = $this->info['start_time'] ?? microtime(true);
$info = &$this->info;
$headers = &$this->headers;
if (!$info['raw_headers']) {
// Used to keep track of what we're waiting for
@ -62,8 +64,8 @@ final class CurlResponse implements ResponseInterface
$content = ($options['buffer'] ?? true) ? $content : null;
}
curl_setopt($ch, CURLOPT_HEADERFUNCTION, static function ($ch, string $data) use (&$info, $options, $multi, $id, &$location, $resolveRedirect): int {
return self::parseHeaderLine($ch, $data, $info, $options, $multi, $id, $location, $resolveRedirect);
curl_setopt($ch, CURLOPT_HEADERFUNCTION, static function ($ch, string $data) use (&$info, &$headers, $options, $multi, $id, &$location, $resolveRedirect): int {
return self::parseHeaderLine($ch, $data, $info, $headers, $options, $multi, $id, $location, $resolveRedirect);
});
if (null === $options) {
@ -116,8 +118,6 @@ final class CurlResponse implements ResponseInterface
curl_setopt($ch, CURLOPT_HEADERFUNCTION, null);
curl_setopt($ch, CURLOPT_READFUNCTION, null);
curl_setopt($ch, CURLOPT_INFILE, null);
$response->addRawHeaders($response->info['raw_headers']);
};
// Schedule the request in a non-blocking way
@ -243,7 +243,7 @@ final class CurlResponse implements ResponseInterface
/**
* Parses header lines as curl yields them to us.
*/
private static function parseHeaderLine($ch, string $data, array &$info, ?array $options, \stdClass $multi, int $id, ?string &$location, ?callable $resolveRedirect): int
private static function parseHeaderLine($ch, string $data, array &$info, array &$headers, ?array $options, \stdClass $multi, int $id, ?string &$location, ?callable $resolveRedirect): int
{
if (!\in_array($waitFor = @curl_getinfo($ch, CURLINFO_PRIVATE), ['headers', 'destruct'], true)) {
return \strlen($data); // Ignore HTTP trailers
@ -251,7 +251,16 @@ final class CurlResponse implements ResponseInterface
if ("\r\n" !== $data) {
// Regular header line: add it to the list
$info['raw_headers'][] = substr($data, 0, -2);
self::addRawHeaders([substr($data, 0, -2)], $info, $headers);
if (0 === strpos($data, 'HTTP') && 300 <= $info['http_code'] && $info['http_code'] < 400) {
if (curl_getinfo($ch, CURLINFO_REDIRECT_COUNT) === $options['max_redirects']) {
curl_setopt($ch, CURLOPT_FOLLOWLOCATION, false);
} elseif (303 === $info['http_code'] || ('POST' === $info['http_method'] && \in_array($info['http_code'], [301, 302], true))) {
$info['http_method'] = 'HEAD' === $info['http_method'] ? 'HEAD' : 'GET';
curl_setopt($ch, CURLOPT_POSTFIELDS, '');
}
}
if (0 === stripos($data, 'Location:')) {
$location = trim(substr($data, 9, -2));

5
src/Symfony/Component/HttpClient/Response/NativeResponse.php
View File

@ -111,11 +111,10 @@ final class NativeResponse implements ResponseInterface
// Send request and follow redirects when needed
$this->info['fopen_time'] = microtime(true);
$this->handle = $h = fopen($url, 'r', false, $this->context);
$this->addRawHeaders($http_response_header);
$url = ($this->resolveRedirect)($this->multi, $this->statusCode, $this->headers['location'][0] ?? null, $this->context);
self::addRawHeaders($http_response_header, $this->info, $this->headers);
$url = ($this->resolveRedirect)($this->multi, $this->headers['location'][0] ?? null, $this->context);
} while (null !== $url);
} catch (\Throwable $e) {
$this->statusCode = 0;
$this->close();
$this->multi->handlesActivity[$this->id][] = null;
$this->multi->handlesActivity[$this->id][] = $e;

22
src/Symfony/Component/HttpClient/Response/ResponseTrait.php
View File

@ -28,7 +28,6 @@ use Symfony\Component\HttpClient\Exception\TransportException;
*/
trait ResponseTrait
{
private $statusCode = 0;
private $headers = [];
/**
@ -43,6 +42,7 @@ trait ResponseTrait
private $info = [
'raw_headers' => [],
'http_code' => 0,
'error' => null,
];
@ -63,7 +63,7 @@ trait ResponseTrait
$this->initializer = null;
}
return $this->statusCode;
return $this->info['http_code'];
}
/**
@ -141,35 +141,35 @@ trait ResponseTrait
*/
abstract protected static function select(\stdClass $multi, float $timeout): int;
private function addRawHeaders(array $rawHeaders): void
private static function addRawHeaders(array $rawHeaders, array &$info, array &$headers): void
{
foreach ($rawHeaders as $h) {
if (11 <= \strlen($h) && '/' === $h[4] && preg_match('#^HTTP/\d+(?:\.\d+)? ([12345]\d\d) .*#', $h, $m)) {
$this->headers = [];
$this->info['http_code'] = $this->statusCode = (int) $m[1];
$headers = [];
$info['http_code'] = (int) $m[1];
} elseif (2 === \count($m = explode(':', $h, 2))) {
$this->headers[strtolower($m[0])][] = ltrim($m[1]);
$headers[strtolower($m[0])][] = ltrim($m[1]);
}
$this->info['raw_headers'][] = $h;
$info['raw_headers'][] = $h;
}
if (!$this->statusCode) {
if (!$info['http_code']) {
throw new TransportException('Invalid or missing HTTP status line.');
}
}
private function checkStatusCode()
{
if (500 <= $this->statusCode) {
if (500 <= $this->info['http_code']) {
throw new ServerException($this);
}
if (400 <= $this->statusCode) {
if (400 <= $this->info['http_code']) {
throw new ClientException($this);
}
if (300 <= $this->statusCode) {
if (300 <= $this->info['http_code']) {
throw new RedirectionException($this);
}
}

1
src/Symfony/Contracts/HttpClient/ResponseInterface.php
View File

@ -71,6 +71,7 @@ interface ResponseInterface
* - redirect_count - the number of redirects followed while executing the request
* - redirect_url - the resolved location of redirect responses, null otherwise
* - start_time - the time when the request was sent or 0.0 when it's pending
* - http_method - the HTTP verb of the last request
* - http_code - the last response code or 0 when it is not known yet
* - error - the error message when the transfer was aborted, null otherwise
* - data - the value of the "data" request option, null if not set

23
src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php
View File

@ -214,7 +214,9 @@ abstract class HttpClientTestCase extends TestCase
$client = $this->getHttpClient();
$response = $client->request('POST', 'http://localhost:8057/301', [
'auth' => 'foo:bar',
'body' => 'foo=bar',
'body' => function () {
yield 'foo=bar';
},
]);
$body = json_decode($response->getContent(), true);
@ -236,7 +238,9 @@ abstract class HttpClientTestCase extends TestCase
'Content-Type: application/json',
];
$filteredHeaders = array_intersect($expected, $response->getInfo('raw_headers'));
$filteredHeaders = array_values(array_filter($response->getInfo('raw_headers'), function ($h) {
return \in_array(substr($h, 0, 4), ['HTTP', 'Loca', 'Cont'], true) && 'Content-Encoding: gzip' !== $h;
}));
$this->assertSame($expected, $filteredHeaders);
}
@ -261,6 +265,16 @@ abstract class HttpClientTestCase extends TestCase
public function testRedirect307()
{
$client = $this->getHttpClient();
$response = $client->request('POST', 'http://localhost:8057/307', [
'body' => function () {
yield 'foo=bar';
},
'max_redirects' => 0,
]);
$this->assertSame(307, $response->getStatusCode());
$response = $client->request('POST', 'http://localhost:8057/307', [
'body' => 'foo=bar',
]);
@ -297,7 +311,9 @@ abstract class HttpClientTestCase extends TestCase
'Content-Type: application/json',
];
$filteredHeaders = array_intersect($expected, $response->getInfo('raw_headers'));
$filteredHeaders = array_values(array_filter($response->getInfo('raw_headers'), function ($h) {
return \in_array(substr($h, 0, 4), ['HTTP', 'Loca', 'Cont'], true);
}));
$this->assertSame($expected, $filteredHeaders);
}
@ -416,6 +432,7 @@ abstract class HttpClientTestCase extends TestCase
$response = $client->request('POST', 'http://localhost:8057/post', [
'body' => function () {
yield 'foo';
yield '';
yield '=';
yield '0123456789';
},