Skip to content

Commit 0dd5cfb

Browse files
nicolas-grekasnicolas-grekas
committed
bug #62699 [HttpClient] Fix ScopingHttpClient to always pass base_uri as string instead of parsed array (santysisi)
This PR was merged into the 6.4 branch. Discussion ---------- [HttpClient] Fix `ScopingHttpClient` to always pass `base_uri` as `string` instead of parsed `array` | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix #62697 | License | MIT ### Problem * `ScopingHttpClient` parsed the `base_uri` into an `array` (scheme, path, query, etc.). * When passing this parsed value to a decorated client such as `RetryableHttpClient`, it caused errors. * According to the `HttpClientInterface` in Symfony Contracts, `base_uri` [must be a string](https://github.com/symfony/symfony/blob/6.4/src/Symfony/Contracts/HttpClient/HttpClientInterface.php#L45) ### Solution * Updated `ScopingHttpClient` so it always forwards `base_uri` as a `string`, not as a parsed array. * The change is applied for all decorated clients, not only `RetryableHttpClient`, since sending an `array` was not compliant with the `HttpClientInterface` comment. ### Additional Notes Originally, the idea was to apply this fix only when the decorated client was `RetryableHttpClient` and later add full support for parsed `base_uri` for `RetryableHttpClient` in version 8.1. However, after reviewing the `HttpClientInterface`, it became clear that sending a parsed `array` for `base_uri` was incorrect. Therefore, the behavior is now corrected universally. Commits ------- 9f80dd4 [HttpClient] Fix `ScopingHttpClient` to always pass `base_uri` as `string` instead of parsed `array`
2 parents 491c2f4 + 9f80dd4 commit 0dd5cfb

File tree

4 files changed

+39
-5
lines changed

4 files changed

+39
-5
lines changed

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,13 +170,14 @@ private static function prepareRequest(?string $method, ?string $url, array $opt
170170
unset($options['auth_basic'], $options['auth_bearer']);
171171

172172
// Parse base URI
173-
if (\is_string($options['base_uri'])) {
174-
$options['base_uri'] = self::parseUrl($options['base_uri']);
173+
if (\is_string($baseUri = $options['base_uri'] ?? null)) {
174+
$baseUri = self::parseUrl($baseUri);
175175
}
176+
unset($options['base_uri']);
176177

177178
// Validate and resolve URL
178179
$url = self::parseUrl($url, $options['query']);
179-
$url = self::resolveUrl($url, $options['base_uri'], $defaultOptions['query'] ?? []);
180+
$url = self::resolveUrl($url, $baseUri, $defaultOptions['query'] ?? []);
180181
}
181182

182183
// Finalize normalization of options

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,8 @@ private static function shiftBaseUri(array $options, array &$baseUris): array
201201
if ($baseUris) {
202202
$baseUri = 1 < \count($baseUris) ? array_shift($baseUris) : current($baseUris);
203203
$options['base_uri'] = \is_array($baseUri) ? $baseUri[array_rand($baseUri)] : $baseUri;
204+
} elseif (\is_array($options['base_uri'] ?? null)) {
205+
unset($options['base_uri']);
204206
}
205207

206208
return $options;

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,11 @@ public function request(string $method, string $url, array $options = []): Respo
5656
{
5757
$e = null;
5858
$url = self::parseUrl($url, $options['query'] ?? []);
59+
$resolved = false;
5960

6061
if (\is_string($options['base_uri'] ?? null)) {
6162
$options['base_uri'] = self::parseUrl($options['base_uri']);
63+
$resolved = true;
6264
}
6365

6466
try {
@@ -72,10 +74,15 @@ public function request(string $method, string $url, array $options = []): Respo
7274
$options = self::mergeDefaultOptions($options, $defaultOptions, true);
7375
if (\is_string($options['base_uri'] ?? null)) {
7476
$options['base_uri'] = self::parseUrl($options['base_uri']);
77+
$resolved = true;
7578
}
7679
$url = implode('', self::resolveUrl($url, $options['base_uri'] ?? null, $defaultOptions['query'] ?? []));
7780
}
7881

82+
if ($resolved) {
83+
unset($options['base_uri']);
84+
}
85+
7986
foreach ($this->defaultOptionsByRegexp as $regexp => $defaultOptions) {
8087
if (preg_match("{{$regexp}}A", $url)) {
8188
if (null === $e || $regexp !== $this->defaultRegexp) {

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

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\HttpClient\Exception\InvalidArgumentException;
1616
use Symfony\Component\HttpClient\MockHttpClient;
17+
use Symfony\Component\HttpClient\Response\MockResponse;
18+
use Symfony\Component\HttpClient\Retry\GenericRetryStrategy;
19+
use Symfony\Component\HttpClient\RetryableHttpClient;
1720
use Symfony\Component\HttpClient\ScopingHttpClient;
1821

1922
class ScopingHttpClientTest extends TestCase
@@ -94,10 +97,31 @@ public function testForBaseUri()
9497
$client = ScopingHttpClient::forBaseUri(new MockHttpClient(null, null), 'http://example.com/foo');
9598

9699
$response = $client->request('GET', '/bar');
97-
$this->assertSame('http://example.com/foo', implode('', $response->getRequestOptions()['base_uri']));
98100
$this->assertSame('http://example.com/bar', $response->getInfo('url'));
99101

100102
$response = $client->request('GET', 'http://foo.bar/');
101-
$this->assertNull($response->getRequestOptions()['base_uri']);
103+
$this->assertSame('http://foo.bar/', $response->getInfo('url'));
104+
}
105+
106+
public function testRetryableHttpClientIntegration()
107+
{
108+
$responses = [
109+
new MockResponse(info: ['http_code' => 503]),
110+
new MockResponse(info: ['http_code' => 503]),
111+
new MockResponse(info: ['http_code' => 503]),
112+
new MockResponse(),
113+
];
114+
115+
$client = ScopingHttpClient::forBaseUri(
116+
new RetryableHttpClient(
117+
new MockHttpClient($responses),
118+
new GenericRetryStrategy(delayMs: 0)
119+
),
120+
'https://foo.example.com/app/',
121+
);
122+
123+
$response = $client->request('GET', 'santysisi');
124+
$this->assertSame(200, $response->getStatusCode());
125+
$this->assertSame('https://foo.example.com/app/santysisi', $response->getInfo('url'));
102126
}
103127
}

0 commit comments

Comments
 (0)