8000 bug #58776 [DependencyInjection][HttpClient][Routing] Reject URIs tha… · symfony/symfony@352786c · GitHub
[go: up one dir, main page]
Skip to content

Commit 352786c

Browse files
nicolas-grekasnicolas-grekas
committed
bug #58776 [DependencyInjection][HttpClient][Routing] Reject URIs that contain invalid characters (nicolas-grekas)
This PR was merged into the 7.2 branch. Discussion ---------- [DependencyInjection][HttpClient][Routing] Reject URIs that contain invalid characters | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | - | License | MIT The behavior of `parse_url()` doesn't match how browsers parse URLs. This PR ensures we won't accept URLs that are invalid per https://url.spec.whatwg.org/: - URLs that contain a backslash - or start/end with a control-char or a space - or contain a CR/LF/TAB character Commits ------- d73003e [DependencyInjection][Routing][HttpClient] Reject URIs that contain invalid characters
2 parents 2c9bcaa + d73003e commit 352786c

File tree

6 files changed

+86
-1
lines changed

6 files changed

+86
-1
lines changed

‎src/Symfony/Component/DependencyInjection/EnvVarProcessor.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,12 @@ public function getEnv(string $prefix, string $name, \Closure $getEnv): mixed
310310
if (!isset($params['scheme'], $params['host'])) {
311311
throw new RuntimeException(\sprintf('Invalid URL in env var "%s": scheme and host expected.', $name));
312312
}
313+
if (('\\' !== \DIRECTORY_SEPARATOR || 'file' !== $params['scheme']) && false !== ($i = strpos($env, '\\')) && $i < strcspn($env, '?#')) {
314+
throw new RuntimeException(\sprintf('Invalid URL in env var "%s": backslashes are not allowed.', $name));
315+
}
316+
if (\ord($env[0]) <= 32 || \ord($env[-1]) <= 32 || \strlen($env) !== strcspn($env, "\r\n\t")) {
317+
throw new RuntimeException(\sprintf('Invalid URL in env var "%s": leading/trailing ASCII control characters or whitespaces are not allowed.', $name));
318+
}
313319
$params += [
314320
'port' => null,
315321
'user' => null,

‎src/Symfony/Component/DependencyInjection/Tests/EnvVarProcessorTest.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -996,6 +996,27 @@ public static function provideGetEnvUrlPath()
996996
];
997997
}
998998

999+
/**
1000+
* @testWith ["http://foo.com\\bar"]
1001+
* ["\\\\foo.com/bar"]
1002+
* ["a\rb"]
1003+
* ["a\nb"]
1004+
* ["a\tb"]
1005+
* ["\u0000foo"]
1006+
* ["foo\u0000"]
1007+
* [" foo"]
1008+
* ["foo "]
1009+
* [":"]
1010+
*/
1011+
public function testGetEnvBadUrl(string $url)
1012+
{
1013+
$this->expectException(RuntimeException::class);
1014+
1015+
(new EnvVarProcessor(new Container()))->getEnv('url', 'foo', static function () use ($url): string {
1016+
return $url;
1017+
});
1018+
}
1019+
9991020
/**
10001021
* @testWith ["", "string"]
10011022
* [null, ""]

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,16 @@ private static function resolveUrl(array $url, ?array $base, array $queryDefault
625625
*/
626626
private static function parseUrl(string $url, array $query = [], array $allowedSchemes = ['http' => 80, 'https' => 443]): array
627627
{
628+
if (false !== ($i = strpos($url, '\\')) && $i < strcspn($url, '?#')) {
629+
throw new InvalidArgumentException(\sprintf('Malformed URL "%s": backslashes are not allowed.', $url));
630+
}
631+
if (\strlen($url) !== strcspn($url, "\r\n\t")) {
632+
throw new InvalidArgumentException(\sprintf('Malformed URL "%s": CR/LF/TAB characters are not allowed.', $url));
633+
}
634+
if ('' !== $url && (\ord($url[0]) <= 32 || \ord($url[-1]) <= 32)) {
635+
throw new InvalidArgumentException(\sprintf('Malformed URL "%s": leading/trailing ASCII control characters or spaces are not allowed.', $url));
636+
}
637+
628638
if (false === $parts = parse_url($url)) {
629639
if ('/' !== ($url[0] ?? '') || false === $parts = parse_url($url.'#')) {
630640
throw new InvalidArgumentException(\sprintf('Malformed URL "%s".', $url));

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,13 +239,32 @@ public function testResolveUrlWithoutScheme()
239239
self::resolveUrl(self::parseUrl('localhost:8080'), null);
240240
}
241241

242-
public function testResolveBaseUrlWitoutScheme()
242+
public function testResolveBaseUrlWithoutScheme()
243243
{
244244
$this->expectException(InvalidArgumentException::class);
245245
$this->expectExceptionMessage('Invalid URL: scheme is missing in "//localhost:8081". Did you forget to add "http(s)://"?');
246246
self::resolveUrl(self::parseUrl('/foo'), self::parseUrl('localhost:8081'));
247247
}
248248

249+
/**
250+
* @testWith ["http://foo.com\\bar"]
251+
* ["\\\\foo.com/bar"]
252+
* ["a\rb"]
253+
* ["a\nb"]
254+
* ["a\tb"]
255+
* ["\u0000foo"]
256+
* ["foo\u0000"]
257+
* [" foo"]
258+
* ["foo "]
259+
* [":"]
260+
*/
261+
public function testParseMalformedUrl(string $url)
262+
{
263+
$this->expectException(InvalidArgumentException::class);
264+
$this->expectExceptionMessage('Malformed URL');
265+
self::parseUrl($url);
266+
}
267+
249268
/**
250269
* @dataProvider provideParseUrl
251270
*/

‎src/Symfony/Component/Routing/RequestContext.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,13 @@ public function __construct(string $baseUrl = '', string $method = 'GET', string
4747

4848
public static function fromUri(string $uri, string $host = 'localhost', string $scheme = 'http', int $httpPort = 80, int $httpsPort = 443): self
4949
{
50+
if (false !== ($i = strpos($uri, '\\')) && $i < strcspn($uri, '?#')) {
51+
$uri = '';
52+
}
53+
if ('' !== $uri && (\ord($uri[0]) <= 32 || \ord($uri[-1]) <= 32 || \strlen($uri) !== strcspn($uri, "\r\n\t"))) {
54+
$uri = '';
55+
}
56+
5057
$uri = parse_url($uri);
5158
$scheme = $uri['scheme'] ?? $scheme;
5259
$host = $uri['host'] ?? $host;

‎src/Symfony/Component/Routing/Tests/RequestContextTest.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,28 @@ public function testFromUriBeingEmpty()
8585
$this->assertSame('/', $requestContext->getPathInfo());
8686
}
8787

88+
/**
89+
* @testWith ["http://foo.com\\bar"]
90+
* ["\\\\foo.com/bar"]
91+
* ["a\rb"]
92+
* ["a\nb"]
93+
* ["a\tb"]
94+
* ["\u0000foo"]
95+
* ["foo\u0000"]
96+
* [" foo"]
97+
* ["foo "]
98+
* [":"]
99+
*/
100+
public function testFromBadUri(string $uri)
101+
{
102+
$context = RequestContext::fromUri($uri);
103+
104+
$this->assertSame('http', $context->getScheme());
105+
$this->assertSame('localhost', $context->getHost());
106+
$this->assertSame('', $context->getBaseUrl());
107+
$this->assertSame('/', $context->getPathInfo());
108+
}
109+
88110
public function testFromRequest()
89111
{
90112
$request = Request::create('https://test.com:444/foo?bar=baz');

0 commit comments

Comments
 (0)
0