Skip to content

Commit 1947ef0

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '7.1' into 7.2
* 7.1: 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 95d1191 + def740c commit 1947ef0

File tree

12 files changed

+131
-48
lines changed

12 files changed

+131
-48
lines changed

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

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

5353
$subnets = $this->subnets;
5454

55-
$options['on_progress'] = function (int $dlNow, int $dlSize, array $info) use ($onProgress, $subnets): void {
55+
$options['on_progress'] = static function (int $dlNow, int $dlSize, array $info) use ($onProgress, $subnets): void {
56+
static $lastUrl = '';
5657
static $lastPrimaryIp = '';
58+
59+
if ($info['url'] !== $lastUrl) {
60+
$host = trim(parse_url($info['url'], PHP_URL_HOST) ?: '', '[]');
61+
62+
if ($host && IpUtils::checkIp($host, $subnets ?? IpUtils::PRIVATE_SUBNETS)) {
63+
throw new TransportException(sprintf('Host "%s" is blocked for "%s".', $host, $info['url']));
64+
}
65+
66+
$lastUrl = $info['url'];
67+
}
68+
5769
if ($info['primary_ip'] !== $lastPrimaryIp) {
5870
if ($info['primary_ip'] && IpUtils::checkIp($info['primary_ip'], $subnets ?? IpUtils::PRIVATE_SUBNETS)) {
5971
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;
@@ -275,6 +276,8 @@ public static function createFromGlobals(): static
275276
* @param array $files The request files ($_FILES)
276277
* @param array $server The server parameters ($_SERVER)
277278
* @param string|resource|null $content The raw body data
279+
*
280+
* @throws BadRequestException When the URI is invalid
278281
*/
279282
public static function create(string $uri, string $method = 'GET', array $parameters = [], array $cookies = [], array $files = [], array $server = [], $content = null): static
280283
{
@@ -302,6 +305,20 @@ public static function create(string $uri, string $method = 'GET', array $parame
302305
throw new \InvalidArgumentException(\sprintf('Malformed URI "%s".', $uri));
303306
}
304307

308+
if (false === $components) {
309+
throw new BadRequestException('Invalid URI.');
310+
}
311+
312+
if (false !== ($i = strpos($uri, '\\')) && $i < strcspn($uri, '?#')) {
313+
throw new BadRequestException('Invalid URI: A URI cannot contain a backslash.');
314+
}
315+
if (\strlen($uri) !== strcspn($uri, "\r\n\t")) {
316+
throw new BadRequestException('Invalid URI: A URI cannot contain CR/LF/TAB characters.');
317+
}
318+
if ('' !== $uri && (\ord($uri[0]) <= 32 || \ord($uri[-1]) <= 32)) {
319+
throw new BadRequestException('Invalid URI: A URI must not start nor end with ASCII control characters or spaces.');
320+
}
321+
305322
if (isset($components['host'])) {
306323
$server['SERVER_NAME'] = $components['host'];
307324
$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;
@@ -290,9 +291,34 @@ public function testCreateWithRequestUri()
290291
$this->assertTrue($request->isSecure());
291292

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

298324
/**
@@ -2666,13 +2692,6 @@ public function testReservedFlags()
26662692
$this->assertNotSame(0b10000000, $value, \sprintf('The constant "%s" should not use the reserved value "0b10000000".', $constant));
26672693
}
26682694
}
2669-
2670-
public function testMalformedUriCreationException()
2671-
{
2672-
$this->expectException(\InvalidArgumentException::class);
2673-
$this->expectExceptionMessage('Malformed URI "/invalid-path:123".');
2674-
Request::create('/invalid-path:123');
2675-
}
26762695
}
26772696

26782697
class RequestContentProxy extends Request

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

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,6 @@ class ExecutableFinder
2929

3030
private array $suffixes = [];
3131

32-
public function __construct()
33-
{
34-
// Set common extensions on Windows.
35-
if ('\\' === \DIRECTORY_SEPARATOR) {
36-
$this->suffixes = ['.exe', '.bat', '.cmd', '.com'];
37-
}
38-
}
39-
4032
/**
4133
* Replaces default suffixes of executable.
4234
*/
@@ -75,11 +67,12 @@ public function find(string $name, ?string $default = null, array $extraDirs = [
7567
$extraDirs
7668
);
7769

78-
$suffixes = [''];
79-
if ('\\' === \DIRECTORY_SEPARATOR && $pathExt = getenv('PATHEXT')) {
80-
$suffixes = array_merge(explode(\PATH_SEPARATOR, $pathExt), $suffixes);
70+
$suffixes = $this->suffixes;
71+
if ('\\' === \DIRECTORY_SEPARATOR) {
72+
$pathExt = getenv('PATHEXT');
73+
$suffixes = array_merge($suffixes, $pathExt ? explode(\PATH_SEPARATOR, $pathExt) : ['.exe', '.bat', '.cmd', '.com']);
8174
}
82-
$suffixes = array_merge($suffixes, $this->suffixes);
75+
$suffixes = '' !== pathinfo($name, PATHINFO_EXTENSION) ? array_merge([''], $suffixes) : array_merge($suffixes, ['']);
8376
foreach ($suffixes as $suffix) {
8477
foreach ($dirs as $dir) {
8578
if ('' === $dir) {
@@ -95,12 +88,11 @@ public function find(string $name, ?string $default = null, array $extraDirs = [
9588
}
9689
}
9790

98-
if (!\function_exists('exec') || \strlen($name) !== strcspn($name, '/'.\DIRECTORY_SEPARATOR)) {
91+
if ('\\' === \DIRECTORY_SEPARATOR || !\function_exists('exec') || \strlen($name) !== strcspn($name, '/'.\DIRECTORY_SEPARATOR)) {
9992
return $default;
10093
}
10194

102-
$command = '\\' === \DIRECTORY_SEPARATOR ? 'where %s 2> NUL' : 'command -v -- %s';
103-
$execResult = exec(\sprintf($command, escapeshellarg($name)));
95+
$execResult = exec('command -v -- '.escapeshellarg($name));
10496

10597
if (($executablePath = substr($execResult, 0, strpos($execResult, \PHP_EOL) ?: null)) && @is_executable($executablePath)) {
10698
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
@@ -1592,7 +1592,14 @@ function ($m) use (&$env, $uid) {
15921592
$cmd
15931593
);
15941594

1595-
$cmd = 'cmd /V:ON /E:ON /D /C ('.str_replace("\n", ' ', $cmd).')';
1595+
static $comSpec;
1596+
1597+
if (!$comSpec && $comSpec = (new ExecutableFinder())->find('cmd.exe')) {
1598+
// Escape according to CommandLineToArgvW rules
1599+
$comSpec = '"'.preg_replace('{(\\\\*+)"}', '$1$1\"', $comSpec) .'"';
1600+
}
1601+
1602+
$cmd = ($comSpec ?? 'cmd').' /V:ON /E:ON /D /C ('.str_replace("\n", ' ', $cmd).')';
15961603
foreach ($this->processPipes->getFiles() as $offset => $filename) {
15971604
$cmd .= ' '.$offset.'>"'.$filename.'"';
15981605
}

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