Skip to content

Commit def740c

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '6.4' into 7.1
* 6.4: Do not read from argv on non-CLI SAPIs [Process] Use %PATH% before %CD% to load the shell on Windows [HttpFoundation] Reject URIs that contain invalid characters [HttpClient] Filter private IPs before connecting when Host == IP
2 parents 28ba29d + 236222a commit def740c

File tree

12 files changed

+132
-39
lines changed

12 files changed

+132
-39
lines changed

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,20 @@ public function request(string $method, string $url, array $options = []): Respo
5656

5757
$subnets = $this->subnets;
5858

59-
$options['on_progress'] = function (int $dlNow, int $dlSize, array $info) use ($onProgress, $subnets): void {
59+
$options['on_progress'] = static function (int $dlNow, int $dlSize, array $info) use ($onProgress, $subnets): void {
60+
static $lastUrl = '';
6061
static $lastPrimaryIp = '';
62+
63+
if ($info['url'] !== $lastUrl) {
64+
$host = trim(parse_url($info['url'], PHP_URL_HOST) ?: '', '[]');
65+
66+
if ($host && IpUtils::checkIp($host, $subnets ?? IpUtils::PRIVATE_SUBNETS)) {
67+
throw new TransportException(sprintf('Host "%s" is blocked for "%s".', $host, $info['url']));
68+
}
69+
70+
$lastUrl = $info['url'];
71+
}
72+
6173
if ($info['primary_ip'] !== $lastPrimaryIp) {
6274
if ($info['primary_ip'] && IpUtils::checkIp($info['primary_ip'], $subnets ?? IpUtils::PRIVATE_SUBNETS)) {
6375
throw new TransportException(sprintf('IP "%s" is blocked for "%s".', $info['primary_ip'], $info['url']));

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

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,10 @@ public static function getExcludeData(): array
6565
/**
6666
* @dataProvider getExcludeData
6767
*/
68-
public function testExclude(string $ipAddr, $subnets, bool $mustThrow)
68+
public function testExcludeByIp(string $ipAddr, $subnets, bool $mustThrow)
6969
{
7070
$content = 'foo';
71-
$url = sprintf('http://%s/', 0 < substr_count($ipAddr, ':') ? sprintf('[%s]', $ipAddr) : $ipAddr);
71+
$url = sprintf('http://%s/', strtr($ipAddr, '.:', '--'));
7272

7373
if ($mustThrow) {
7474
$this->expectException(TransportException::class);
@@ -85,6 +85,29 @@ public function testExclude(string $ipAddr, $subnets, bool $mustThrow)
8585
}
8686
}
8787

88+
/**
89+
* @dataProvider getExcludeData
90+
*/
91+
public function testExcludeByHost(string $ipAddr, $subnets, bool $mustThrow)
92+
{
93+
$content = 'foo';
94+
$url = sprintf('http://%s/', str_contains($ipAddr, ':') ? sprintf('[%s]', $ipAddr) : $ipAddr);
95+
96+
if ($mustThrow) {
97+
$this->expectException(TransportException::class);
98+
$this->expectExceptionMessage(sprintf('Host "%s" is blocked for "%s".', $ipAddr, $url));
99+
}
100+
101+
$previousHttpClient = $this->getHttpClientMock($url, $ipAddr, $content);
102+
$client = new NoPrivateNetworkHttpClient($previousHttpClient, $subnets);
103+
$response = $client->request('GET', $url);
104+
105+
if (!$mustThrow) {
106+
$this->assertEquals($content, $response->getContent());
107+
$this->assertEquals(200, $response->getStatusCode());
108+
}
109+
}
110+
88111
public function testCustomOnProgressCallback()
89112
{
90113
$ipAddr = '104.26.14.6';

‎src/Symfony/Component/HttpFoundation/Request.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\HttpFoundation;
1313

14+
use Symfony\Component\HttpFoundation\Exception\BadRequestException;
1415
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
1516
use Symfony\Component\HttpFoundation\Exception\JsonException;
1617
use Symfony\Component\HttpFoundation\Exception\SessionNotFoundException;
@@ -276,6 +277,8 @@ public static function createFromGlobals(): static
276277
* @param array $files The request files ($_FILES)
277278
* @param array $server The server parameters ($_SERVER)
278279
* @param string|resource|null $content The raw body data
280+
*
281+
* @throws BadRequestException When the URI is invalid
279282
*/
280283
public static function create(string $uri, string $method = 'GET', array $parameters = [], array $cookies = [], array $files = [], array $server = [], $content = null): static
281284
{
@@ -303,6 +306,20 @@ public static function create(string $uri, string $method = 'GET', array $parame
303306
throw new \InvalidArgumentException(sprintf('Malformed URI "%s".', $uri));
304307
}
305308

309+
if (false === $components) {
310+
throw new BadRequestException('Invalid URI.');
311+
}
312+
313+
if (false !== ($i = strpos($uri, '\\')) && $i < strcspn($uri, '?#')) {
314+
throw new BadRequestException('Invalid URI: A URI cannot contain a backslash.');
315+
}
316+
if (\strlen($uri) !== strcspn($uri, "\r\n\t")) {
317+
throw new BadRequestException('Invalid URI: A URI cannot contain CR/LF/TAB characters.');
318+
}
319+
if ('' !== $uri && (\ord($uri[0]) <= 32 || \ord($uri[-1]) <= 32)) {
320+
throw new BadRequestException('Invalid URI: A URI must not start nor end with ASCII control characters or spaces.');
321+
}
322+
306323
if (isset($components['host'])) {
307324
$server['SERVER_NAME'] = $components['host'];
308325
$server['HTTP_HOST'] = $components['host'];

‎src/Symfony/Component/HttpFoundation/Tests/RequestTest.php

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\HttpFoundation\Tests;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\HttpFoundation\Exception\BadRequestException;
1516
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
1617
use Symfony\Component\HttpFoundation\Exception\JsonException;
1718
use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException;
@@ -289,9 +290,34 @@ public function testCreateWithRequestUri()
289290
$this->assertTrue($request->isSecure());
290291

291292
// Fragment should not be included in the URI
292-
$request = Request::create('http://test.com/foo#bar');
293-
$request->server->set('REQUEST_URI', 'http://test.com/foo#bar');
293+
$request = Request::create('http://test.com/foo#bar\\baz');
294+
$request->server->set('REQUEST_URI', 'http://test.com/foo#bar\\baz');
294295
$this->assertEquals('http://test.com/foo', $request->getUri());
296+
297+
$request = Request::create('http://test.com/foo?bar=f\\o');
298+
$this->assertEquals('http://test.com/foo?bar=f%5Co', $request->getUri());
299+
$this->assertEquals('/foo', $request->getPathInfo());
300+
$this->assertEquals('bar=f%5Co', $request->getQueryString());
301+
}
302+
303+
/**
304+
* @testWith ["http://foo.com\\bar"]
305+
* ["\\\\foo.com/bar"]
306+
* ["a\rb"]
307+
* ["a\nb"]
308+
* ["a\tb"]
309+
* ["\u0000foo"]
310+
* ["foo\u0000"]
311+
* [" foo"]
312+
* ["foo "]
313+
* [":"]
314+
*/
315+
public function testCreateWithBadRequestUri(string $uri)
316+
{
317+
$this->expectException(BadRequestException::class);
318+
$this->expectExceptionMessage('Invalid URI');
319+
320+
Request::create($uri);
295321
}
296322

297323
/**
@@ -2655,13 +2681,6 @@ public function testReservedFlags()
26552681
$this->assertNotSame(0b10000000, $value, sprintf('The constant "%s" should not use the reserved value "0b10000000".', $constant));
26562682
}
26572683
}
2658-
2659-
public function testMalformedUriCreationException()
2660-
{
2661-
$this->expectException(\InvalidArgumentException::class);
2662-
$this->expectExceptionMessage('Malformed URI "/invalid-path:123".');
2663-
Request::create('/invalid-path:123');
2664-
}
26652684
}
26662685

26672686
class RequestContentProxy extends Request

‎src/Symfony/Component/Process/ExecutableFinder.php

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
*/
2020
class ExecutableFinder
2121
{
22-
private array $suffixes = ['.exe', '.bat', '.cmd', '.com'];
2322
private const CMD_BUILTINS = [
2423
'assoc', 'break', 'call', 'cd', 'chdir', 'cls', 'color', 'copy', 'date',
2524
'del', 'dir', 'echo', 'endlocal', 'erase', 'exit', 'for', 'ftype', 'goto',
@@ -28,6 +27,8 @@ class ExecutableFinder
2827
'setlocal', 'shift', 'start', 'time', 'title', 'type', 'ver', 'vol',
2928
];
3029

30+
private array $suffixes = [];
31+
3132
/**
3233
* Replaces default suffixes of executable.
3334
*/
@@ -63,11 +64,13 @@ public function find(string $name, ?string $default = null, array $extraDirs = [
6364
$extraDirs
6465
);
6566

66-
$suffixes = [''];
67+
$suffixes = [];
6768
if ('\\' === \DIRECTORY_SEPARATOR) {
6869
$pathExt = getenv('PATHEXT');
69-
$suffixes = array_merge($pathExt ? explode(\PATH_SEPARATOR, $pathExt) : $this->suffixes, $suffixes);
70+
$suffixes = $this->suffixes;
71+
$suffixes = array_merge($suffixes, $pathExt ? explode(\PATH_SEPARATOR, $pathExt) : ['.exe', '.bat', '.cmd', '.com']);
7072
}
73+
$suffixes = '' !== pathinfo($name, PATHINFO_EXTENSION) ? array_merge([''], $suffixes) : array_merge($suffixes, ['']);
7174
foreach ($suffixes as $suffix) {
7275
foreach ($dirs as $dir) {
7376
if ('' === $dir) {
@@ -83,12 +86,11 @@ public function find(string $name, ?string $default = null, array $extraDirs = [
8386
}
8487
}
8588

86-
if (!\function_exists('exec') || \strlen($name) !== strcspn($name, '/'.\DIRECTORY_SEPARATOR)) {
89+
if ('\\' === \DIRECTORY_SEPARATOR || !\function_exists('exec') || \strlen($name) !== strcspn($name, '/'.\DIRECTORY_SEPARATOR)) {
8790
return $default;
8891
}
8992

90-
$command = '\\' === \DIRECTORY_SEPARATOR ? 'where %s 2> NUL' : 'command -v -- %s';
91-
$execResult = exec(\sprintf($command, escapeshellarg($name)));
93+
$execResult = exec('command -v -- '.escapeshellarg($name));
9294

9395
if (($executablePath = substr($execResult, 0, strpos($execResult, \PHP_EOL) ?: null)) && @is_executable($executablePath)) {
9496
return $executablePath;

‎src/Symfony/Component/Process/PhpExecutableFinder.php

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,8 @@ public function __construct()
3232
public function find(bool $includeArgs = true): string|false
3333
{
3434
if ($php = getenv('PHP_BINARY')) {
35-
if (!is_executable($php)) {
36-
if (!\function_exists('exec') || \strlen($php) !== strcspn($php, '/'.\DIRECTORY_SEPARATOR)) {
37-
return false;
38-
}
39-
40-
$command = '\\' === \DIRECTORY_SEPARATOR ? 'where %s 2> NUL' : 'command -v -- %s';
41-
$execResult = exec(\sprintf($command, escapeshellarg($php)));
42-
if (!$php = substr($execResult, 0, strpos($execResult, \PHP_EOL) ?: null)) {
43-
return false;
44-
}
45-
if (!is_executable($php)) {
46-
return false;
47-
}
35+
if (!is_executable($php) && !$php = $this->executableFinder->find($php)) {
36+
return false;
4837
}
4938

5039
if (@is_dir($php)) {

‎src/Symfony/Component/Process/Process.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1585,7 +1585,14 @@ function ($m) use (&$env, $uid) {
15851585
$cmd
15861586
);
15871587

1588-
$cmd = 'cmd /V:ON /E:ON /D /C ('.str_replace("\n", ' ', $cmd).')';
1588+
static $comSpec;
1589+
1590+
if (!$comSpec && $comSpec = (new ExecutableFinder())->find('cmd.exe')) {
1591+
// Escape according to CommandLineToArgvW rules
1592+
$comSpec = '"'.preg_replace('{(\\\\*+)"}', '$1$1\"', $comSpec) .'"';
1593+
}
1594+
1595+
$cmd = ($comSpec ?? 'cmd').' /V:ON /E:ON /D /C ('.str_replace("\n", ' ', $cmd).')';
15891596
foreach ($this->processPipes->getFiles() as $offset => $filename) {
15901597
$cmd .= ' '.$offset.'>"'.$filename.'"';
15911598
}

‎src/Symfony/Component/Runtime/SymfonyRuntime.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public function __construct(array $options = [])
9595

9696
if (isset($options['env'])) {
9797
$_SERVER[$envKey] = $options['env'];
98-
} elseif (isset($_SERVER['argv']) && class_exists(ArgvInput::class)) {
98+
} elseif (empty($_GET) && isset($_SERVER['argv']) && class_exists(ArgvInput::class)) {
9999
$this->options = $options;
100100
$this->getInput();
101101
}
@@ -203,6 +203,10 @@ protected static function register(GenericRuntime $runtime): GenericRuntime
203203

204204
private function getInput(): ArgvInput
205205
{
206+
if (!empty($_GET) && filter_var(ini_get('register_argc_argv'), \FILTER_VALIDATE_BOOL)) {
207+
throw new \Exception('CLI applications cannot be run safely on non-CLI SAPIs with register_argc_argv=On.');
208+
}
209+
206210
if (isset($this->input)) {
207211
return $this->input;
208212
}

‎src/Symfony/Component/Runtime/Tests/phpt/kernel-loop.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@ require __DIR__.'/kernel-loop.php';
1111

1212
?>
1313
--EXPECTF--
14-
OK Kernel foo_bar
15-
OK Kernel foo_bar
14+
OK Kernel (env=dev) foo_bar
15+
OK Kernel (env=dev) foo_bar
1616
0

‎src/Symfony/Component/Runtime/Tests/phpt/kernel.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,19 @@
1717

1818
class TestKernel implements HttpKernelInterface
1919
{
20+
private string $env;
2021
private string $var;
2122

22-
public function __construct(string $var)
23+
public function __construct(string $env, string $var)
2324
{
25+
$this->env = $env;
2426
$this->var = $var;
2527
}
2628

2729
public function handle(Request $request, $type = self::MAIN_REQUEST, $catch = true): Response
2830
{
29-
return new Response('OK Kernel '.$this->var);
31+
return new Response('OK Kernel (env='.$this->env.') '.$this->var);
3032
}
3133
}
3234

33-
return fn (array $context) => new TestKernel($context['SOME_VAR']);
35+
return fn (array $context) => new TestKernel($context['APP_ENV'], $context['SOME_VAR']);

0 commit comments

Comments
 (0)