From 53bb5b45d9f6821f9114ceff46561d079c5d1d3d Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sat, 16 May 2020 14:09:30 +0200 Subject: [PATCH 01/20] updated version to 5.2 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 344d90024..b170a9378 100644 --- a/composer.json +++ b/composer.json @@ -39,7 +39,7 @@ "minimum-stability": "dev", "extra": { "branch-alias": { - "dev-master": "5.1-dev" + "dev-master": "5.2-dev" } } } From 04c90558d7e8c98238aa3ace852d2a7326325f80 Mon Sep 17 00:00:00 2001 From: Vladimir Reznichenko Date: Sat, 2 May 2020 20:09:15 +0200 Subject: [PATCH 02/20] SCA: file_exists -> is_dir|is_file in foundation and kernel --- BinaryFileResponse.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BinaryFileResponse.php b/BinaryFileResponse.php index 30eff944d..f952eddd2 100644 --- a/BinaryFileResponse.php +++ b/BinaryFileResponse.php @@ -302,7 +302,7 @@ public function sendContent() fclose($out); fclose($file); - if ($this->deleteFileAfterSend && file_exists($this->file->getPathname())) { + if ($this->deleteFileAfterSend && is_file($this->file->getPathname())) { unlink($this->file->getPathname()); } From 1b66b861f3ff55fb39c13822a0fc915018b5ac84 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sat, 13 Jun 2020 20:18:57 +0200 Subject: [PATCH 03/20] [HttpFoundation] add `HeaderUtils::parseQuery()`: it does the same as `parse_str()` but preserves dots in variable names --- CHANGELOG.md | 5 ++++ HeaderUtils.php | 58 +++++++++++++++++++++++++++++++++++++++ Request.php | 4 +-- Tests/HeaderUtilsTest.php | 37 +++++++++++++++++++++++++ Tests/RequestTest.php | 2 +- 5 files changed, 103 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60fab8381..f806ee699 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +5.2.0 +----- + + * added `HeaderUtils::parseQuery()`: it does the same as `parse_str()` but preserves dots in variable names + 5.1.0 ----- diff --git a/HeaderUtils.php b/HeaderUtils.php index 5866e3b2b..bd9d04dfa 100644 --- a/HeaderUtils.php +++ b/HeaderUtils.php @@ -193,6 +193,64 @@ public static function makeDisposition(string $disposition, string $filename, st return $disposition.'; '.self::toString($params, ';'); } + /** + * Like parse_str(), but preserves dots in variable names. + */ + public static function parseQuery(string $query, bool $ignoreBrackets = false, string $separator = '&'): array + { + $q = []; + + foreach (explode($separator, $query) as $v) { + if (false !== $i = strpos($v, "\0")) { + $v = substr($v, 0, $i); + } + + if (false === $i = strpos($v, '=')) { + $k = urldecode($v); + $v = ''; + } else { + $k = urldecode(substr($v, 0, $i)); + $v = substr($v, $i); + } + + if (false !== $i = strpos($k, "\0")) { + $k = substr($k, 0, $i); + } + + $k = ltrim($k, ' '); + + if ($ignoreBrackets) { + $q[$k][] = urldecode(substr($v, 1)); + + continue; + } + + if (false === $i = strpos($k, '[')) { + $q[] = bin2hex($k).$v; + } else { + $q[] = substr_replace($k, bin2hex(substr($k, 0, $i)), 0, $i).$v; + } + } + + if ($ignoreBrackets) { + return $q; + } + + parse_str(implode('&', $q), $q); + + $query = []; + + foreach ($q as $k => $v) { + if (false !== $i = strpos($k, '_')) { + $query[substr_replace($k, hex2bin(substr($k, 0, $i)).'[', 0, 1 + $i)] = $v; + } else { + $query[hex2bin($k)] = $v; + } + } + + return $query; + } + private static function groupParts(array $matches, string $separators): array { $separator = $separators[0]; diff --git a/Request.php b/Request.php index e737a7e58..ce1e779ea 100644 --- a/Request.php +++ b/Request.php @@ -399,7 +399,7 @@ public static function create(string $uri, string $method = 'GET', array $parame $queryString = ''; if (isset($components['query'])) { - parse_str(html_entity_decode($components['query']), $qs); + $qs = HeaderUtils::parseQuery(html_entity_decode($components['query'])); if ($query) { $query = array_replace($qs, $query); @@ -660,7 +660,7 @@ public static function normalizeQueryString(?string $qs) return ''; } - parse_str($qs, $qs); + $qs = HeaderUtils::parseQuery($qs); ksort($qs); return http_build_query($qs, '', '&', PHP_QUERY_RFC3986); diff --git a/Tests/HeaderUtilsTest.php b/Tests/HeaderUtilsTest.php index d2b19ca84..b8ad4dfd0 100644 --- a/Tests/HeaderUtilsTest.php +++ b/Tests/HeaderUtilsTest.php @@ -129,4 +129,41 @@ public function provideMakeDispositionFail() ['attachment', 'föö.html'], ]; } + + /** + * @dataProvider provideParseQuery + */ + public function testParseQuery(string $query, string $expected = null) + { + $this->assertSame($expected ?? $query, http_build_query(HeaderUtils::parseQuery($query), '', '&')); + } + + public function provideParseQuery() + { + return [ + ['a=b&c=d'], + ['a.b=c'], + ['a+b=c'], + ["a\0b=c", 'a='], + ['a%00b=c', 'a=c'], + ['a[b=c', 'a%5Bb=c'], + ['a]b=c', 'a%5Db=c'], + ['a[b]=c', 'a%5Bb%5D=c'], + ['a[b][c.d]=c', 'a%5Bb%5D%5Bc.d%5D=c'], + ['a%5Bb%5D=c'], + ]; + } + + public function testParseCookie() + { + $query = 'a.b=c; def%5Bg%5D=h'; + $this->assertSame($query, http_build_query(HeaderUtils::parseQuery($query, false, ';'), '', '; ')); + } + + public function testParseQueryIgnoreBrackets() + { + $this->assertSame(['a.b' => ['A', 'B']], HeaderUtils::parseQuery('a.b=A&a.b=B', true)); + $this->assertSame(['a.b[]' => ['A']], HeaderUtils::parseQuery('a.b[]=A', true)); + $this->assertSame(['a.b[]' => ['A']], HeaderUtils::parseQuery('a.b%5B%5D=A', true)); + } } diff --git a/Tests/RequestTest.php b/Tests/RequestTest.php index 99cd54884..8986be52c 100644 --- a/Tests/RequestTest.php +++ b/Tests/RequestTest.php @@ -807,7 +807,7 @@ public function getQueryStringNormalizationData() ['foo=1&foo=2', 'foo=2', 'merges repeated parameters'], ['pa%3Dram=foo%26bar%3Dbaz&test=test', 'pa%3Dram=foo%26bar%3Dbaz&test=test', 'works with encoded delimiters'], ['0', '0=', 'allows "0"'], - ['Foo Bar&Foo%20Baz', 'Foo_Bar=&Foo_Baz=', 'normalizes encoding in keys'], + ['Foo Bar&Foo%20Baz', 'Foo%20Bar=&Foo%20Baz=', 'normalizes encoding in keys'], ['bar=Foo Bar&baz=Foo%20Baz', 'bar=Foo%20Bar&baz=Foo%20Baz', 'normalizes encoding in values'], ['foo=bar&&&test&&', 'foo=bar&test=', 'removes unneeded delimiters'], ['formula=e=m*c^2', 'formula=e%3Dm%2Ac%5E2', 'correctly treats only the first "=" as delimiter and the next as value'], From 238083cc37875e86c6a3692b0aa27a8f6be33be4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Thu, 2 Jul 2020 11:52:45 +0200 Subject: [PATCH 04/20] [HttpFoundation] Added File::getContent() --- CHANGELOG.md | 1 + File/File.php | 11 +++++++++++ Tests/File/FileTest.php | 7 +++++++ 3 files changed, 19 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f806ee699..a3aaa04d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ CHANGELOG ----- * added `HeaderUtils::parseQuery()`: it does the same as `parse_str()` but preserves dots in variable names + * added `File::getContent()` 5.1.0 ----- diff --git a/File/File.php b/File/File.php index e8762459d..99e33c56c 100644 --- a/File/File.php +++ b/File/File.php @@ -104,6 +104,17 @@ public function move(string $directory, string $name = null) return $target; } + public function getContent(): string + { + $content = file_get_contents($this->getPathname()); + + if (false === $content) { + throw new FileException(sprintf('Could not get the content of the file "%s".', $this->getPathname())); + } + + return $content; + } + /** * @return self */ diff --git a/Tests/File/FileTest.php b/Tests/File/FileTest.php index 2ef259ed1..ddc9c1a84 100644 --- a/Tests/File/FileTest.php +++ b/Tests/File/FileTest.php @@ -85,6 +85,13 @@ public function testMoveWithNewName() @unlink($targetPath); } + public function testGetContent() + { + $file = new File(__FILE__); + + $this->assertStringEqualsFile(__FILE__, $file->getContent()); + } + public function getFilenameFixtures() { return [ From d3a250632a328c56aef3245611e536353ea88e61 Mon Sep 17 00:00:00 2001 From: Evgeny Anisiforov Date: Mon, 3 Aug 2020 19:33:24 +0200 Subject: [PATCH 05/20] [HttpFoundation] add support for X_FORWARDED_PREFIX header --- CHANGELOG.md | 5 +++++ Request.php | 40 +++++++++++++++++++++++++++++--------- Tests/RequestTest.php | 45 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3aaa04d6..f36d045e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +5.3.0 +----- + +* added support for `X-Forwarded-Prefix` header + 5.2.0 ----- diff --git a/Request.php b/Request.php index ce1e779ea..2fccdedb7 100644 --- a/Request.php +++ b/Request.php @@ -39,13 +39,16 @@ class_exists(ServerBag::class); */ class Request { - const HEADER_FORWARDED = 0b00001; // When using RFC 7239 - const HEADER_X_FORWARDED_FOR = 0b00010; - const HEADER_X_FORWARDED_HOST = 0b00100; - const HEADER_X_FORWARDED_PROTO = 0b01000; - const HEADER_X_FORWARDED_PORT = 0b10000; - const HEADER_X_FORWARDED_ALL = 0b11110; // All "X-Forwarded-*" headers - const HEADER_X_FORWARDED_AWS_ELB = 0b11010; // AWS ELB doesn't send X-Forwarded-Host + const HEADER_FORWARDED = 0b000001; // When using RFC 7239 + const HEADER_X_FORWARDED_FOR = 0b000010; + const HEADER_X_FORWARDED_HOST = 0b000100; + const HEADER_X_FORWARDED_PROTO = 0b001000; + const HEADER_X_FORWARDED_PORT = 0b010000; + const HEADER_X_FORWARDED_PREFIX = 0b100000; + + const HEADER_X_FORWARDED_ALL = 0b011110; // All "X-Forwarded-*" headers sent by "usual" reverse proxy + const HEADER_X_FORWARDED_AWS_ELB = 0b011010; // AWS ELB doesn't send X-Forwarded-Host + const HEADER_X_FORWARDED_TRAEFIK = 0b111110; // All "X-Forwarded-*" headers sent by Traefik reverse proxy const METHOD_HEAD = 'HEAD'; const METHOD_GET = 'GET'; @@ -237,6 +240,7 @@ class Request self::HEADER_X_FORWARDED_HOST => 'X_FORWARDED_HOST', self::HEADER_X_FORWARDED_PROTO => 'X_FORWARDED_PROTO', self::HEADER_X_FORWARDED_PORT => 'X_FORWARDED_PORT', + self::HEADER_X_FORWARDED_PREFIX => 'X_FORWARDED_PREFIX', ]; /** @@ -894,6 +898,24 @@ public function getBasePath() * @return string The raw URL (i.e. not urldecoded) */ public function getBaseUrl() + { + $trustedPrefix = ''; + + // the proxy prefix must be prepended to any prefix being needed at the webserver level + if ($this->isFromTrustedProxy() && $trustedPrefixValues = $this->getTrustedValues(self::HEADER_X_FORWARDED_PREFIX)) { + $trustedPrefix = rtrim($trustedPrefixValues[0], '/'); + } + + return $trustedPrefix.$this->getBaseUrlReal(); + } + + /** + * Returns the real base URL received by the webserver from which this request is executed. + * The URL does not include trusted reverse proxy prefix. + * + * @return string The raw URL (i.e. not urldecoded) + */ + private function getBaseUrlReal() { if (null === $this->baseUrl) { $this->baseUrl = $this->prepareBaseUrl(); @@ -1910,7 +1932,7 @@ protected function preparePathInfo() $requestUri = '/'.$requestUri; } - if (null === ($baseUrl = $this->getBaseUrl())) { + if (null === ($baseUrl = $this->getBaseUrlReal())) { return $requestUri; } @@ -2014,7 +2036,7 @@ private function getTrustedValues(int $type, string $ip = null): array } } - if ((self::$trustedHeaderSet & self::HEADER_FORWARDED) && $this->headers->has(self::$trustedHeaders[self::HEADER_FORWARDED])) { + if ((self::$trustedHeaderSet & self::HEADER_FORWARDED) && (isset(self::$forwardedParams[$type])) && $this->headers->has(self::$trustedHeaders[self::HEADER_FORWARDED])) { $forwarded = $this->headers->get(self::$trustedHeaders[self::HEADER_FORWARDED]); $parts = HeaderUtils::split($forwarded, ',;='); $forwardedValues = []; diff --git a/Tests/RequestTest.php b/Tests/RequestTest.php index 8986be52c..358d2b140 100644 --- a/Tests/RequestTest.php +++ b/Tests/RequestTest.php @@ -2278,6 +2278,51 @@ public function testTrustedHost() $this->assertSame(443, $request->getPort()); } + public function testTrustedPrefix() + { + Request::setTrustedProxies(['1.1.1.1'], Request::HEADER_X_FORWARDED_TRAEFIK); + + //test with index deployed under root + $request = Request::create('/method'); + $request->server->set('REMOTE_ADDR', '1.1.1.1'); + $request->headers->set('X-Forwarded-Prefix', '/myprefix'); + $request->headers->set('Forwarded', 'host=localhost:8080'); + + $this->assertSame('/myprefix', $request->getBaseUrl()); + $this->assertSame('/myprefix', $request->getBasePath()); + $this->assertSame('/method', $request->getPathInfo()); + } + + public function testTrustedPrefixWithSubdir() + { + Request::setTrustedProxies(['1.1.1.1'], Request::HEADER_X_FORWARDED_TRAEFIK); + + $server = [ + 'SCRIPT_FILENAME' => '/var/hidden/app/public/public/index.php', + 'SCRIPT_NAME' => '/public/index.php', + 'PHP_SELF' => '/public/index.php', + ]; + + //test with index file deployed in subdir, i.e. local dev server (insecure!!) + $request = Request::create('/public/method', 'GET', [], [], [], $server); + $request->server->set('REMOTE_ADDR', '1.1.1.1'); + $request->headers->set('X-Forwarded-Prefix', '/prefix'); + $request->headers->set('Forwarded', 'host=localhost:8080'); + + $this->assertSame('/prefix/public', $request->getBaseUrl()); + $this->assertSame('/prefix/public', $request->getBasePath()); + $this->assertSame('/method', $request->getPathInfo()); + } + + public function testTrustedPrefixEmpty() + { + //check that there is no error, if no prefix is provided + Request::setTrustedProxies(['1.1.1.1'], Request::HEADER_X_FORWARDED_TRAEFIK); + $request = Request::create('/method'); + $request->server->set('REMOTE_ADDR', '1.1.1.1'); + $this->assertSame('', $request->getBaseUrl()); + } + public function testTrustedPort() { Request::setTrustedProxies(['1.1.1.1'], -1); From 3081f76fa297650e0152377d7194ef043ccb4733 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sat, 22 Aug 2020 08:38:27 +0200 Subject: [PATCH 06/20] Fix CHANGELOG --- CHANGELOG.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f36d045e6..85868b22a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,14 +1,10 @@ CHANGELOG ========= -5.3.0 ------ - -* added support for `X-Forwarded-Prefix` header - 5.2.0 ----- +* added support for `X-Forwarded-Prefix` header * added `HeaderUtils::parseQuery()`: it does the same as `parse_str()` but preserves dots in variable names * added `File::getContent()` From 8a47a27fe2c484d9303355d270d6246835b1c1c2 Mon Sep 17 00:00:00 2001 From: Zmey Date: Thu, 10 Sep 2020 23:01:01 +0300 Subject: [PATCH 07/20] [SecurityBundle] Comma separated ips for security.access_control --- CHANGELOG.md | 3 ++- RequestMatcher.php | 6 +++++- Tests/RequestMatcherTest.php | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85868b22a..babb87c2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,10 @@ CHANGELOG 5.2.0 ----- -* added support for `X-Forwarded-Prefix` header + * added support for `X-Forwarded-Prefix` header * added `HeaderUtils::parseQuery()`: it does the same as `parse_str()` but preserves dots in variable names * added `File::getContent()` + * added ability to use comma separated ip addresses for `RequestMatcher::matchIps()` 5.1.0 ----- diff --git a/RequestMatcher.php b/RequestMatcher.php index c32c5cdce..ab778ea58 100644 --- a/RequestMatcher.php +++ b/RequestMatcher.php @@ -125,7 +125,11 @@ public function matchIp(string $ip) */ public function matchIps($ips) { - $this->ips = null !== $ips ? (array) $ips : []; + $ips = null !== $ips ? (array) $ips : []; + + $this->ips = array_reduce($ips, static function (array $ips, string $ip) { + return array_merge($ips, preg_split('/\s*,\s*/', $ip)); + }, []); } /** diff --git a/Tests/RequestMatcherTest.php b/Tests/RequestMatcherTest.php index 57e9c3d30..6fab4e712 100644 --- a/Tests/RequestMatcherTest.php +++ b/Tests/RequestMatcherTest.php @@ -163,4 +163,38 @@ public function testAttributes() $matcher->matchAttribute('foo', 'babar'); $this->assertFalse($matcher->matches($request)); } + + public function testIps() + { + $matcher = new RequestMatcher(); + + $request = Request::create('', 'GET', [], [], [], ['REMOTE_ADDR' => '127.0.0.1']); + + $matcher->matchIp('127.0.0.1'); + $this->assertTrue($matcher->matches($request)); + + $matcher->matchIp('192.168.0.1'); + $this->assertFalse($matcher->matches($request)); + + $matcher->matchIps('127.0.0.1'); + $this->assertTrue($matcher->matches($request)); + + $matcher->matchIps('127.0.0.1, ::1'); + $this->assertTrue($matcher->matches($request)); + + $matcher->matchIps('192.168.0.1, ::1'); + $this->assertFalse($matcher->matches($request)); + + $matcher->matchIps(['127.0.0.1', '::1']); + $this->assertTrue($matcher->matches($request)); + + $matcher->matchIps(['192.168.0.1', '::1']); + $this->assertFalse($matcher->matches($request)); + + $matcher->matchIps(['1.1.1.1', '2.2.2.2', '127.0.0.1, ::1']); + $this->assertTrue($matcher->matches($request)); + + $matcher->matchIps(['1.1.1.1', '2.2.2.2', '192.168.0.1, ::1']); + $this->assertFalse($matcher->matches($request)); + } } From ea83943009fdb6fdf5791e6e4beee25d8bcebc74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20H=C3=BCnig?= Date: Sun, 30 Aug 2020 21:07:14 +0200 Subject: [PATCH 08/20] [HttpFoundation][Cache][Messenger] Replace redis "DEL" commands with "UNLINK" --- Session/Storage/Handler/RedisSessionHandler.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Session/Storage/Handler/RedisSessionHandler.php b/Session/Storage/Handler/RedisSessionHandler.php index 180f63fdf..756a48808 100644 --- a/Session/Storage/Handler/RedisSessionHandler.php +++ b/Session/Storage/Handler/RedisSessionHandler.php @@ -89,6 +89,18 @@ protected function doWrite(string $sessionId, string $data): bool */ protected function doDestroy(string $sessionId): bool { + static $unlink = true; + + if ($unlink) { + try { + $this->redis->unlink($this->prefix.$sessionId); + + return true; + } catch (\Throwable $e) { + $unlink = false; + } + } + $this->redis->del($this->prefix.$sessionId); return true; From ec46f1bbae80de20ec9baa5d1cfabc7bf5844d2f Mon Sep 17 00:00:00 2001 From: Wouter de Jong Date: Sat, 26 Sep 2020 20:00:19 +0200 Subject: [PATCH 09/20] Added request rate limiters and improved login throttling This allows limiting on different elements of a request. This is usefull to e.g. prevent breadth-first attacks, by allowing to enforce a limit on both IP and IP+username. --- CHANGELOG.md | 1 + RateLimiter/AbstractRequestRateLimiter.php | 59 +++++++++++++++++++++ RateLimiter/RequestRateLimiterInterface.php | 32 +++++++++++ 3 files changed, 92 insertions(+) create mode 100644 RateLimiter/AbstractRequestRateLimiter.php create mode 100644 RateLimiter/RequestRateLimiterInterface.php diff --git a/CHANGELOG.md b/CHANGELOG.md index babb87c2b..277efc4fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ CHANGELOG * added `HeaderUtils::parseQuery()`: it does the same as `parse_str()` but preserves dots in variable names * added `File::getContent()` * added ability to use comma separated ip addresses for `RequestMatcher::matchIps()` + * added `RateLimiter\RequestRateLimiterInterface` and `RateLimiter\AbstractRequestRateLimiter` 5.1.0 ----- diff --git a/RateLimiter/AbstractRequestRateLimiter.php b/RateLimiter/AbstractRequestRateLimiter.php new file mode 100644 index 000000000..430c150ca --- /dev/null +++ b/RateLimiter/AbstractRequestRateLimiter.php @@ -0,0 +1,59 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\HttpFoundation\RateLimiter; + +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\RateLimiter\Limit; +use Symfony\Component\RateLimiter\LimiterInterface; +use Symfony\Component\RateLimiter\NoLimiter; + +/** + * An implementation of RequestRateLimiterInterface that + * fits most use-cases. + * + * @author Wouter de Jong + * + * @experimental in Symfony 5.2 + */ +abstract class AbstractRequestRateLimiter implements RequestRateLimiterInterface +{ + public function consume(Request $request): Limit + { + $limiters = $this->getLimiters($request); + if (0 === \count($limiters)) { + $limiters = [new NoLimiter()]; + } + + $minimalLimit = null; + foreach ($limiters as $limiter) { + $limit = $limiter->consume(1); + + if (null === $minimalLimit || $limit->getRemainingTokens() < $minimalLimit->getRemainingTokens()) { + $minimalLimit = $limit; + } + } + + return $minimalLimit; + } + + public function reset(): void + { + foreach ($this->getLimiters($request) as $limiter) { + $limiter->reset(); + } + } + + /** + * @return LimiterInterface[] a set of limiters using keys extracted from the request + */ + abstract protected function getLimiters(Request $request): array; +} diff --git a/RateLimiter/RequestRateLimiterInterface.php b/RateLimiter/RequestRateLimiterInterface.php new file mode 100644 index 000000000..21d763771 --- /dev/null +++ b/RateLimiter/RequestRateLimiterInterface.php @@ -0,0 +1,32 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\HttpFoundation\RateLimiter; + +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\RateLimiter\Limit; + +/** + * A special type of limiter that deals with requests. + * + * This allows to limit on different types of information + * from the requests. + * + * @author Wouter de Jong + * + * @experimental in Symfony 5.2 + */ +interface RequestRateLimiterInterface +{ + public function consume(Request $request): Limit; + + public function reset(): void; +} From af8a5353c61d7d656768cba750e35d846228ea32 Mon Sep 17 00:00:00 2001 From: Nyholm Date: Thu, 17 Sep 2020 16:41:40 +0200 Subject: [PATCH 10/20] [HttpFoundation] Add Request::toArray() for JSON content --- CHANGELOG.md | 1 + Exception/JsonException.php | 21 +++++++++++++++++++++ Request.php | 29 +++++++++++++++++++++++++++++ Tests/RequestTest.php | 25 +++++++++++++++++++++++++ 4 files changed, 76 insertions(+) create mode 100644 Exception/JsonException.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 277efc4fa..17d380456 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ CHANGELOG * added `HeaderUtils::parseQuery()`: it does the same as `parse_str()` but preserves dots in variable names * added `File::getContent()` * added ability to use comma separated ip addresses for `RequestMatcher::matchIps()` + * added `Request::toArray()` to parse a JSON request body to an array * added `RateLimiter\RequestRateLimiterInterface` and `RateLimiter\AbstractRequestRateLimiter` 5.1.0 diff --git a/Exception/JsonException.php b/Exception/JsonException.php new file mode 100644 index 000000000..5990e760e --- /dev/null +++ b/Exception/JsonException.php @@ -0,0 +1,21 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\HttpFoundation\Exception; + +/** + * Thrown by Request::toArray() when the content cannot be JSON-decoded. + * + * @author Tobias Nyholm + */ +final class JsonException extends \UnexpectedValueException implements RequestExceptionInterface +{ +} diff --git a/Request.php b/Request.php index 7c59428b7..286c32961 100644 --- a/Request.php +++ b/Request.php @@ -12,6 +12,7 @@ namespace Symfony\Component\HttpFoundation; use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException; +use Symfony\Component\HttpFoundation\Exception\JsonException; use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException; use Symfony\Component\HttpFoundation\Session\SessionInterface; @@ -1569,6 +1570,34 @@ public function getContent(bool $asResource = false) return $this->content; } + /** + * Gets the request body decoded as array, typically from a JSON payload. + * + * @throws JsonException When the body cannot be decoded to an array + */ + public function toArray(): array + { + if ('' === $content = $this->getContent()) { + throw new JsonException('Response body is empty.'); + } + + try { + $content = json_decode($content, true, 512, \JSON_BIGINT_AS_STRING | (\PHP_VERSION_ID >= 70300 ? \JSON_THROW_ON_ERROR : 0)); + } catch (\JsonException $e) { + throw new JsonException('Could not decode request body.', $e->getCode(), $e); + } + + if (\PHP_VERSION_ID < 70300 && \JSON_ERROR_NONE !== json_last_error()) { + throw new JsonException('Could not decode request body: '.json_last_error_msg(), json_last_error()); + } + + if (!\is_array($content)) { + throw new JsonException(sprintf('JSON content was expected to decode to an array, "%s" returned.', get_debug_type($content))); + } + + return $content; + } + /** * Gets the Etags. * diff --git a/Tests/RequestTest.php b/Tests/RequestTest.php index 358d2b140..5d466afe2 100644 --- a/Tests/RequestTest.php +++ b/Tests/RequestTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\HttpFoundation\Tests; use PHPUnit\Framework\TestCase; +use Symfony\Component\HttpFoundation\Exception\JsonException; use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException; use Symfony\Component\HttpFoundation\InputBag; use Symfony\Component\HttpFoundation\ParameterBag; @@ -1250,6 +1251,30 @@ public function provideOverloadedMethods() ]; } + public function testToArrayEmpty() + { + $req = new Request(); + $this->expectException(JsonException::class); + $this->expectExceptionMessage('Response body is empty.'); + $req->toArray(); + } + + public function testToArrayNonJson() + { + $req = new Request([], [], [], [], [], [], 'foobar'); + $this->expectException(JsonException::class); + $this->expectExceptionMessageMatches('|Could not decode request body.+|'); + $req->toArray(); + } + + public function testToArray() + { + $req = new Request([], [], [], [], [], [], json_encode([])); + $this->assertEquals([], $req->toArray()); + $req = new Request([], [], [], [], [], [], json_encode(['foo' => 'bar'])); + $this->assertEquals(['foo' => 'bar'], $req->toArray()); + } + /** * @dataProvider provideOverloadedMethods */ From 90460dcc443b4280874275e672cd50f1e14c0bff Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Sat, 3 Oct 2020 23:39:59 +0100 Subject: [PATCH 11/20] Remove array return type from Request::toArray() --- Request.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Request.php b/Request.php index 286c32961..1e64183a5 100644 --- a/Request.php +++ b/Request.php @@ -1574,8 +1574,10 @@ public function getContent(bool $asResource = false) * Gets the request body decoded as array, typically from a JSON payload. * * @throws JsonException When the body cannot be decoded to an array + * + * @return array */ - public function toArray(): array + public function toArray() { if ('' === $content = $this->getContent()) { throw new JsonException('Response body is empty.'); From 4e5fd3695247fb09946fbb7904385b17c2c77346 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Sun, 4 Oct 2020 10:29:39 +0200 Subject: [PATCH 12/20] [Mime] Prefer .jpg instead of .jpeg --- Tests/File/UploadedFileTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/File/UploadedFileTest.php b/Tests/File/UploadedFileTest.php index 5565b0384..50374aa08 100644 --- a/Tests/File/UploadedFileTest.php +++ b/Tests/File/UploadedFileTest.php @@ -87,11 +87,11 @@ public function testGuessClientExtensionWithIncorrectMimeType() $file = new UploadedFile( __DIR__.'/Fixtures/test.gif', 'original.gif', - 'image/jpeg', + 'image/png', null ); - $this->assertEquals('jpeg', $file->guessClientExtension()); + $this->assertEquals('png', $file->guessClientExtension()); } public function testCaseSensitiveMimeType() From 59a3f34344db5ba46df7110b852220279e55ec4a Mon Sep 17 00:00:00 2001 From: Andreas Date: Thu, 1 Oct 2020 13:38:00 +0200 Subject: [PATCH 13/20] [HttpFoundation] Expired cookies string representation consistency & tests --- Cookie.php | 25 +++++++++++++++++++------ Tests/CookieTest.php | 29 +++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/Cookie.php b/Cookie.php index fa03ac3b9..90f73add1 100644 --- a/Cookie.php +++ b/Cookie.php @@ -62,8 +62,9 @@ public static function fromString(string $cookie, bool $decode = false) $value = isset($part[1]) ? ($decode ? urldecode($part[1]) : $part[1]) : null; $data = HeaderUtils::combine($parts) + $data; + $data['expires'] = self::expiresTimestamp($data['expires']); - if (isset($data['max-age'])) { + if (isset($data['max-age']) && ($data['max-age'] > 0 || $data['expires'] > time())) { $data['expires'] = time() + (int) $data['max-age']; } @@ -102,7 +103,7 @@ public function __construct(string $name, string $value = null, $expire = 0, ?st $this->name = $name; $this->value = $value; $this->domain = $domain; - $this->expire = $this->withExpires($expire)->expire; + $this->expire = self::expiresTimestamp($expire); $this->path = empty($path) ? '/' : $path; $this->secure = $secure; $this->httpOnly = $httpOnly; @@ -144,6 +145,21 @@ public function withDomain(?string $domain): self * @return static */ public function withExpires($expire = 0): self + { + $cookie = clone $this; + $cookie->expire = self::expiresTimestamp($expire); + + return $cookie; + } + + /** + * Converts expires formats to a unix timestamp. + * + * @param int|string|\DateTimeInterface $expire + * + * @return int + */ + private static function expiresTimestamp($expire = 0) { // convert expiration time to a Unix timestamp if ($expire instanceof \DateTimeInterface) { @@ -156,10 +172,7 @@ public function withExpires($expire = 0): self } } - $cookie = clone $this; - $cookie->expire = 0 < $expire ? (int) $expire : 0; - - return $cookie; + return 0 < $expire ? (int) $expire : 0; } /** diff --git a/Tests/CookieTest.php b/Tests/CookieTest.php index ef9c13e4c..e291442e1 100644 --- a/Tests/CookieTest.php +++ b/Tests/CookieTest.php @@ -363,4 +363,33 @@ public function testSetSecureDefault() $this->assertFalse($cookie->isSecure()); } + + public function testMaxAge() + { + $futureDateOneHour = gmdate('D, d-M-Y H:i:s T', time() + 3600); + + $cookie = Cookie::fromString('foo=bar; Max-Age=3600; path=/'); + $this->assertEquals('foo=bar; expires='.$futureDateOneHour.'; Max-Age=3600; path=/', $cookie->__toString()); + + $cookie = Cookie::fromString('foo=bar; expires='.$futureDateOneHour.'; Max-Age=3600; path=/'); + $this->assertEquals('foo=bar; expires='.$futureDateOneHour.'; Max-Age=3600; path=/', $cookie->__toString()); + + $futureDateHalfHour = gmdate('D, d-M-Y H:i:s T', time() + 1800); + + // Max-Age value takes precedence before expires + $cookie = Cookie::fromString('foo=bar; expires='.$futureDateHalfHour.'; Max-Age=3600; path=/'); + $this->assertEquals('foo=bar; expires='.$futureDateOneHour.'; Max-Age=3600; path=/', $cookie->__toString()); + } + + public function testExpiredWithMaxAge() + { + $cookie = Cookie::fromString('foo=bar; expires=Fri, 20-May-2011 15:25:52 GMT; Max-Age=0; path=/'); + $this->assertEquals('foo=bar; expires=Fri, 20-May-2011 15:25:52 GMT; Max-Age=0; path=/', $cookie->__toString()); + + $futureDate = gmdate('D, d-M-Y H:i:s T', time() + 864000); + + $cookie = Cookie::fromString('foo=bar; expires='.$futureDate.'; Max-Age=0; path=/'); + $this->assertEquals(time(), $cookie->getExpiresTime()); + $this->assertEquals('foo=bar; expires='.gmdate('D, d-M-Y H:i:s T', $cookie->getExpiresTime()).'; Max-Age=0; path=/', $cookie->__toString()); + } } From 890616825181d199e6e8a1f6583ca260fccc368b Mon Sep 17 00:00:00 2001 From: Kevin Bond Date: Sun, 18 Oct 2020 18:14:51 -0400 Subject: [PATCH 14/20] fix RequestRateLimiterInterface::reset() --- RateLimiter/AbstractRequestRateLimiter.php | 2 +- RateLimiter/RequestRateLimiterInterface.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/RateLimiter/AbstractRequestRateLimiter.php b/RateLimiter/AbstractRequestRateLimiter.php index 430c150ca..cf6d1e1bc 100644 --- a/RateLimiter/AbstractRequestRateLimiter.php +++ b/RateLimiter/AbstractRequestRateLimiter.php @@ -45,7 +45,7 @@ public function consume(Request $request): Limit return $minimalLimit; } - public function reset(): void + public function reset(Request $request): void { foreach ($this->getLimiters($request) as $limiter) { $limiter->reset(); diff --git a/RateLimiter/RequestRateLimiterInterface.php b/RateLimiter/RequestRateLimiterInterface.php index 21d763771..d37427760 100644 --- a/RateLimiter/RequestRateLimiterInterface.php +++ b/RateLimiter/RequestRateLimiterInterface.php @@ -28,5 +28,5 @@ interface RequestRateLimiterInterface { public function consume(Request $request): Limit; - public function reset(): void; + public function reset(Request $request): void; } From f5f580425e3ce3451864dd21677570ae3601803a Mon Sep 17 00:00:00 2001 From: Kevin Bond Date: Fri, 16 Oct 2020 12:07:55 -0400 Subject: [PATCH 15/20] [RateLimiter] rename Limit to RateLimit and add RateLimit::getLimit() --- RateLimiter/AbstractRequestRateLimiter.php | 14 +++++++------- RateLimiter/RequestRateLimiterInterface.php | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/RateLimiter/AbstractRequestRateLimiter.php b/RateLimiter/AbstractRequestRateLimiter.php index cf6d1e1bc..f700ebcd8 100644 --- a/RateLimiter/AbstractRequestRateLimiter.php +++ b/RateLimiter/AbstractRequestRateLimiter.php @@ -12,9 +12,9 @@ namespace Symfony\Component\HttpFoundation\RateLimiter; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\RateLimiter\Limit; use Symfony\Component\RateLimiter\LimiterInterface; use Symfony\Component\RateLimiter\NoLimiter; +use Symfony\Component\RateLimiter\RateLimit; /** * An implementation of RequestRateLimiterInterface that @@ -26,23 +26,23 @@ */ abstract class AbstractRequestRateLimiter implements RequestRateLimiterInterface { - public function consume(Request $request): Limit + public function consume(Request $request): RateLimit { $limiters = $this->getLimiters($request); if (0 === \count($limiters)) { $limiters = [new NoLimiter()]; } - $minimalLimit = null; + $minimalRateLimit = null; foreach ($limiters as $limiter) { - $limit = $limiter->consume(1); + $rateLimit = $limiter->consume(1); - if (null === $minimalLimit || $limit->getRemainingTokens() < $minimalLimit->getRemainingTokens()) { - $minimalLimit = $limit; + if (null === $minimalRateLimit || $rateLimit->getRemainingTokens() < $minimalRateLimit->getRemainingTokens()) { + $minimalRateLimit = $rateLimit; } } - return $minimalLimit; + return $minimalRateLimit; } public function reset(Request $request): void diff --git a/RateLimiter/RequestRateLimiterInterface.php b/RateLimiter/RequestRateLimiterInterface.php index d37427760..c7d31ba5d 100644 --- a/RateLimiter/RequestRateLimiterInterface.php +++ b/RateLimiter/RequestRateLimiterInterface.php @@ -12,7 +12,7 @@ namespace Symfony\Component\HttpFoundation\RateLimiter; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\RateLimiter\Limit; +use Symfony\Component\RateLimiter\RateLimit; /** * A special type of limiter that deals with requests. @@ -26,7 +26,7 @@ */ interface RequestRateLimiterInterface { - public function consume(Request $request): Limit; + public function consume(Request $request): RateLimit; public function reset(Request $request): void; } From fe7e126ff311b8db9c7811de15a1f364a70b1239 Mon Sep 17 00:00:00 2001 From: Nyholm Date: Wed, 21 Oct 2020 23:15:23 +0200 Subject: [PATCH 16/20] [RateLimiter] Moved classes implementing LimiterInterface to a new namespace --- RateLimiter/AbstractRequestRateLimiter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RateLimiter/AbstractRequestRateLimiter.php b/RateLimiter/AbstractRequestRateLimiter.php index f700ebcd8..ddcdf6ca0 100644 --- a/RateLimiter/AbstractRequestRateLimiter.php +++ b/RateLimiter/AbstractRequestRateLimiter.php @@ -13,7 +13,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\RateLimiter\LimiterInterface; -use Symfony\Component\RateLimiter\NoLimiter; +use Symfony\Component\RateLimiter\Policy\NoLimiter; use Symfony\Component\RateLimiter\RateLimit; /** From 769ec40212c0fcf56650e5cc6c33b80e8624f36f Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 28 Oct 2020 11:26:01 +0100 Subject: [PATCH 17/20] [HttpFoundation] Deprecate not passing a `Closure` together with `FILTER_CALLBACK` to `ParameterBag::filter()` --- CHANGELOG.md | 1 + InputBag.php | 5 +++++ ParameterBag.php | 5 +++++ Tests/InputBagTest.php | 11 +++++++++++ Tests/ParameterBagTest.php | 14 ++++++++++++++ 5 files changed, 36 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17d380456..49220fb31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ CHANGELOG * added ability to use comma separated ip addresses for `RequestMatcher::matchIps()` * added `Request::toArray()` to parse a JSON request body to an array * added `RateLimiter\RequestRateLimiterInterface` and `RateLimiter\AbstractRequestRateLimiter` + * deprecated not passing a `Closure` together with `FILTER_CALLBACK` to `ParameterBag::filter()`; wrap your filter in a closure instead. 5.1.0 ----- diff --git a/InputBag.php b/InputBag.php index a45c9529e..2c21748e0 100644 --- a/InputBag.php +++ b/InputBag.php @@ -103,6 +103,11 @@ public function filter(string $key, $default = null, int $filter = \FILTER_DEFAU } } + if ((\FILTER_CALLBACK & $filter) && !(($options['options'] ?? null) instanceof \Closure)) { + trigger_deprecation('symfony/http-foundation', '5.2', 'Not passing a Closure together with FILTER_CALLBACK to "%s()" is deprecated. Wrap your filter in a closure instead.', __METHOD__); + // throw new \InvalidArgumentException(sprintf('A Closure must be passed to "%s()" when FILTER_CALLBACK is used, "%s" given.', __METHOD__, get_debug_type($options['options'] ?? null))); + } + return filter_var($value, $filter, $options); } } diff --git a/ParameterBag.php b/ParameterBag.php index 1de54859a..c6e2dff27 100644 --- a/ParameterBag.php +++ b/ParameterBag.php @@ -194,6 +194,11 @@ public function filter(string $key, $default = null, int $filter = \FILTER_DEFAU $options['flags'] = \FILTER_REQUIRE_ARRAY; } + if ((\FILTER_CALLBACK & $filter) && !(($options['options'] ?? null) instanceof \Closure)) { + trigger_deprecation('symfony/http-foundation', '5.2', 'Not passing a Closure together with FILTER_CALLBACK to "%s()" is deprecated. Wrap your filter in a closure instead.', __METHOD__); + // throw new \InvalidArgumentException(sprintf('A Closure must be passed to "%s()" when FILTER_CALLBACK is used, "%s" given.', __METHOD__, get_debug_type($options['options'] ?? null))); + } + return filter_var($value, $filter, $options); } diff --git a/Tests/InputBagTest.php b/Tests/InputBagTest.php index 870057e41..30c97607a 100644 --- a/Tests/InputBagTest.php +++ b/Tests/InputBagTest.php @@ -45,6 +45,17 @@ public function testFilterArray() $this->assertSame([12, 8], $result); } + /** + * @group legacy + */ + public function testFilterCallback() + { + $bag = new InputBag(['foo' => 'bar']); + + $this->expectDeprecation('Since symfony/http-foundation 5.2: Not passing a Closure together with FILTER_CALLBACK to "Symfony\Component\HttpFoundation\InputBag::filter()" is deprecated. Wrap your filter in a closure instead.'); + $this->assertSame('BAR', $bag->filter('foo', null, \FILTER_CALLBACK, ['options' => 'strtoupper'])); + } + /** * @group legacy */ diff --git a/Tests/ParameterBagTest.php b/Tests/ParameterBagTest.php index e5f436e8e..3c83d9e38 100644 --- a/Tests/ParameterBagTest.php +++ b/Tests/ParameterBagTest.php @@ -12,11 +12,14 @@ namespace Symfony\Component\HttpFoundation\Tests; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\HttpFoundation\Exception\BadRequestException; use Symfony\Component\HttpFoundation\ParameterBag; class ParameterBagTest extends TestCase { + use ExpectDeprecationTrait; + public function testConstructor() { $this->testAll(); @@ -176,6 +179,17 @@ public function testFilter() $this->assertEquals(['bang'], $bag->filter('array', ''), '->filter() gets a value of parameter as an array'); } + /** + * @group legacy + */ + public function testFilterCallback() + { + $bag = new ParameterBag(['foo' => 'bar']); + + $this->expectDeprecation('Since symfony/http-foundation 5.2: Not passing a Closure together with FILTER_CALLBACK to "Symfony\Component\HttpFoundation\ParameterBag::filter()" is deprecated. Wrap your filter in a closure instead.'); + $this->assertSame('BAR', $bag->filter('foo', null, \FILTER_CALLBACK, ['options' => 'strtoupper'])); + } + public function testGetIterator() { $parameters = ['foo' => 'bar', 'hello' => 'world']; From 46ff6a7c7a747cf87ca4ca2701048c6adc9c29de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Deruss=C3=A9?= Date: Sun, 1 Nov 2020 22:31:33 +0100 Subject: [PATCH 18/20] Deprecate HEADER_X_FORWARDED_ALL constant --- CHANGELOG.md | 2 ++ Request.php | 10 +++++++--- Tests/RequestTest.php | 42 +++++++++++++++++++++++++++++++----------- 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49220fb31..fef21b872 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ CHANGELOG * added `Request::toArray()` to parse a JSON request body to an array * added `RateLimiter\RequestRateLimiterInterface` and `RateLimiter\AbstractRequestRateLimiter` * deprecated not passing a `Closure` together with `FILTER_CALLBACK` to `ParameterBag::filter()`; wrap your filter in a closure instead. + * Deprecated the `Request::HEADER_X_FORWARDED_ALL` constant, use either `HEADER_X_FORWARDED_FOR | HEADER_X_FORWARDED_HOST | HEADER_X_FORWARDED_PORT | HEADER_X_FORWARDED_PROTO` or `HEADER_X_FORWARDED_AWS_ELB` or `HEADER_X_FORWARDED_TRAEFIK` constants instead. + 5.1.0 ----- diff --git a/Request.php b/Request.php index 4b0696a4e..34cd49246 100644 --- a/Request.php +++ b/Request.php @@ -47,9 +47,10 @@ class Request const HEADER_X_FORWARDED_PORT = 0b010000; const HEADER_X_FORWARDED_PREFIX = 0b100000; - const HEADER_X_FORWARDED_ALL = 0b011110; // All "X-Forwarded-*" headers sent by "usual" reverse proxy - const HEADER_X_FORWARDED_AWS_ELB = 0b011010; // AWS ELB doesn't send X-Forwarded-Host - const HEADER_X_FORWARDED_TRAEFIK = 0b111110; // All "X-Forwarded-*" headers sent by Traefik reverse proxy + /** @deprecated since Symfony 5.2, use either "HEADER_X_FORWARDED_FOR | HEADER_X_FORWARDED_HOST | HEADER_X_FORWARDED_PORT | HEADER_X_FORWARDED_PROTO" or "HEADER_X_FORWARDED_AWS_ELB" or "HEADER_X_FORWARDED_TRAEFIK" constants instead. */ + const HEADER_X_FORWARDED_ALL = 0b1011110; // All "X-Forwarded-*" headers sent by "usual" reverse proxy + const HEADER_X_FORWARDED_AWS_ELB = 0b0011010; // AWS ELB doesn't send X-Forwarded-Host + const HEADER_X_FORWARDED_TRAEFIK = 0b0111110; // All "X-Forwarded-*" headers sent by Traefik reverse proxy const METHOD_HEAD = 'HEAD'; const METHOD_GET = 'GET'; @@ -593,6 +594,9 @@ public function overrideGlobals() */ public static function setTrustedProxies(array $proxies, int $trustedHeaderSet) { + if (self::HEADER_X_FORWARDED_ALL === $trustedHeaderSet) { + trigger_deprecation('symfony/http-fundation', '5.2', 'The "HEADER_X_FORWARDED_ALL" constant is deprecated, use either "HEADER_X_FORWARDED_FOR | HEADER_X_FORWARDED_HOST | HEADER_X_FORWARDED_PORT | HEADER_X_FORWARDED_PROTO" or "HEADER_X_FORWARDED_AWS_ELB" or "HEADER_X_FORWARDED_TRAEFIK" constants instead.'); + } self::$trustedProxies = array_reduce($proxies, function ($proxies, $proxy) { if ('REMOTE_ADDR' !== $proxy) { $proxies[] = $proxy; diff --git a/Tests/RequestTest.php b/Tests/RequestTest.php index 5d466afe2..6b01446c2 100644 --- a/Tests/RequestTest.php +++ b/Tests/RequestTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\HttpFoundation\Tests; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\HttpFoundation\Exception\JsonException; use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException; use Symfony\Component\HttpFoundation\InputBag; @@ -22,6 +23,8 @@ class RequestTest extends TestCase { + use ExpectDeprecationTrait; + protected function tearDown(): void { Request::setTrustedProxies([], -1); @@ -867,7 +870,7 @@ public function testGetPort() $this->assertEquals(80, $port, 'Without trusted proxies FORWARDED_PROTO and FORWARDED_PORT are ignored.'); - Request::setTrustedProxies(['1.1.1.1'], Request::HEADER_X_FORWARDED_ALL); + Request::setTrustedProxies(['1.1.1.1'], Request::HEADER_X_FORWARDED_PROTO | Request::HEADER_X_FORWARDED_PORT); $request = Request::create('http://example.com', 'GET', [], [], [], [ 'HTTP_X_FORWARDED_PROTO' => 'https', 'HTTP_X_FORWARDED_PORT' => '8443', @@ -1091,7 +1094,7 @@ public function testGetClientIpsWithConflictingHeaders($httpForwarded, $httpXFor 'HTTP_X_FORWARDED_FOR' => $httpXForwardedFor, ]; - Request::setTrustedProxies(['88.88.88.88'], Request::HEADER_X_FORWARDED_ALL | Request::HEADER_FORWARDED); + Request::setTrustedProxies(['88.88.88.88'], Request::HEADER_X_FORWARDED_FOR | Request::HEADER_FORWARDED); $request->initialize([], [], [], [], [], $server); @@ -1349,7 +1352,7 @@ public function testOverrideGlobals() $request->headers->set('X_FORWARDED_PROTO', 'https'); - Request::setTrustedProxies(['1.1.1.1'], Request::HEADER_X_FORWARDED_ALL); + Request::setTrustedProxies(['1.1.1.1'], Request::HEADER_X_FORWARDED_PROTO); $this->assertFalse($request->isSecure()); $request->server->set('REMOTE_ADDR', '1.1.1.1'); $this->assertTrue($request->isSecure()); @@ -1830,7 +1833,7 @@ private function getRequestInstanceForClientIpTests(string $remoteAddr, ?string } if ($trustedProxies) { - Request::setTrustedProxies($trustedProxies, Request::HEADER_X_FORWARDED_ALL); + Request::setTrustedProxies($trustedProxies, Request::HEADER_X_FORWARDED_FOR); } $request->initialize([], [], [], [], [], $server); @@ -1873,35 +1876,35 @@ public function testTrustedProxiesXForwardedFor() $this->assertFalse($request->isSecure()); // disabling proxy trusting - Request::setTrustedProxies([], Request::HEADER_X_FORWARDED_ALL); + Request::setTrustedProxies([], Request::HEADER_X_FORWARDED_FOR); $this->assertEquals('3.3.3.3', $request->getClientIp()); $this->assertEquals('example.com', $request->getHost()); $this->assertEquals(80, $request->getPort()); $this->assertFalse($request->isSecure()); // request is forwarded by a non-trusted proxy - Request::setTrustedProxies(['2.2.2.2'], Request::HEADER_X_FORWARDED_ALL); + Request::setTrustedProxies(['2.2.2.2'], Request::HEADER_X_FORWARDED_FOR); $this->assertEquals('3.3.3.3', $request->getClientIp()); $this->assertEquals('example.com', $request->getHost()); $this->assertEquals(80, $request->getPort()); $this->assertFalse($request->isSecure()); // trusted proxy via setTrustedProxies() - Request::setTrustedProxies(['3.3.3.3', '2.2.2.2'], Request::HEADER_X_FORWARDED_ALL); + Request::setTrustedProxies(['3.3.3.3', '2.2.2.2'], Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_HOST | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO); $this->assertEquals('1.1.1.1', $request->getClientIp()); $this->assertEquals('foo.example.com', $request->getHost()); $this->assertEquals(443, $request->getPort()); $this->assertTrue($request->isSecure()); // trusted proxy via setTrustedProxies() - Request::setTrustedProxies(['3.3.3.4', '2.2.2.2'], Request::HEADER_X_FORWARDED_ALL); + Request::setTrustedProxies(['3.3.3.4', '2.2.2.2'], Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_HOST | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO); $this->assertEquals('3.3.3.3', $request->getClientIp()); $this->assertEquals('example.com', $request->getHost()); $this->assertEquals(80, $request->getPort()); $this->assertFalse($request->isSecure()); // check various X_FORWARDED_PROTO header values - Request::setTrustedProxies(['3.3.3.3', '2.2.2.2'], Request::HEADER_X_FORWARDED_ALL); + Request::setTrustedProxies(['3.3.3.3', '2.2.2.2'], Request::HEADER_X_FORWARDED_PROTO); $request->headers->set('X_FORWARDED_PROTO', 'ssl'); $this->assertTrue($request->isSecure()); @@ -2377,7 +2380,7 @@ public function testTrustedPort() public function testTrustedPortDoesNotDefaultToZero() { - Request::setTrustedProxies(['1.1.1.1'], Request::HEADER_X_FORWARDED_ALL); + Request::setTrustedProxies(['1.1.1.1'], Request::HEADER_X_FORWARDED_FOR); $request = Request::create('/'); $request->server->set('REMOTE_ADDR', '1.1.1.1'); @@ -2393,7 +2396,7 @@ public function testTrustedPortDoesNotDefaultToZero() public function testTrustedProxiesRemoteAddr($serverRemoteAddr, $trustedProxies, $result) { $_SERVER['REMOTE_ADDR'] = $serverRemoteAddr; - Request::setTrustedProxies($trustedProxies, Request::HEADER_X_FORWARDED_ALL); + Request::setTrustedProxies($trustedProxies, Request::HEADER_X_FORWARDED_FOR); $this->assertSame($result, Request::getTrustedProxies()); } @@ -2464,6 +2467,23 @@ public function preferSafeContentData() ], ]; } + + /** + * @group legacy + */ + public function testXForwarededAllConstantDeprecated() + { + $this->expectDeprecation('Since symfony/http-fundation 5.2: The "HEADER_X_FORWARDED_ALL" constant is deprecated, use either "HEADER_X_FORWARDED_FOR | HEADER_X_FORWARDED_HOST | HEADER_X_FORWARDED_PORT | HEADER_X_FORWARDED_PROTO" or "HEADER_X_FORWARDED_AWS_ELB" or "HEADER_X_FORWARDED_TRAEFIK" constants instead.'); + + Request::setTrustedProxies([], Request::HEADER_X_FORWARDED_ALL); + } + + public function testReservedFlags() + { + foreach ((new \ReflectionClass(Request::class))->getConstants() as $constant => $value) { + $this->assertNotSame(0b10000000, $value, sprintf('The constant "%s" should not use the reserved value "0b10000000".', $constant)); + } + } } class RequestContentProxy extends Request From 3272231ad27feb749e4f6e21714efd2431793a62 Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Fri, 20 Nov 2020 16:42:02 +0100 Subject: [PATCH 19/20] [HttpFoundation] Deprecate BinaryFileResponse::create(). --- BinaryFileResponse.php | 4 +++ CHANGELOG.md | 1 + Tests/BinaryFileResponseTest.php | 43 ++++++++++++++++++++++++-------- 3 files changed, 38 insertions(+), 10 deletions(-) diff --git a/BinaryFileResponse.php b/BinaryFileResponse.php index 930ea23f3..b2e2d9e40 100644 --- a/BinaryFileResponse.php +++ b/BinaryFileResponse.php @@ -65,9 +65,13 @@ public function __construct($file, int $status = 200, array $headers = [], bool * @param bool $autoLastModified Whether the Last-Modified header should be automatically set * * @return static + * + * @deprecated since Symfony 5.2, use __construct() instead. */ public static function create($file = null, int $status = 200, array $headers = [], bool $public = true, string $contentDisposition = null, bool $autoEtag = false, bool $autoLastModified = true) { + trigger_deprecation('symfony/http-foundation', '5.2', 'The "%s()" method is deprecated, use "new %s()" instead.', __METHOD__, static::class); + return new static($file, $status, $headers, $public, $contentDisposition, $autoEtag, $autoLastModified); } diff --git a/CHANGELOG.md b/CHANGELOG.md index fef21b872..a5ba6f720 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ CHANGELOG * added `RateLimiter\RequestRateLimiterInterface` and `RateLimiter\AbstractRequestRateLimiter` * deprecated not passing a `Closure` together with `FILTER_CALLBACK` to `ParameterBag::filter()`; wrap your filter in a closure instead. * Deprecated the `Request::HEADER_X_FORWARDED_ALL` constant, use either `HEADER_X_FORWARDED_FOR | HEADER_X_FORWARDED_HOST | HEADER_X_FORWARDED_PORT | HEADER_X_FORWARDED_PROTO` or `HEADER_X_FORWARDED_AWS_ELB` or `HEADER_X_FORWARDED_TRAEFIK` constants instead. + * Deprecated `BinaryFileResponse::create()`, use `__construct()` instead 5.1.0 diff --git a/Tests/BinaryFileResponseTest.php b/Tests/BinaryFileResponseTest.php index 2efbc2b8a..31d6bc2c0 100644 --- a/Tests/BinaryFileResponseTest.php +++ b/Tests/BinaryFileResponseTest.php @@ -11,6 +11,7 @@ namespace Symfony\Component\HttpFoundation\Tests; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\HttpFoundation\BinaryFileResponse; use Symfony\Component\HttpFoundation\File\Stream; use Symfony\Component\HttpFoundation\Request; @@ -19,6 +20,8 @@ class BinaryFileResponseTest extends ResponseTestCase { + use ExpectDeprecationTrait; + public function testConstruction() { $file = __DIR__.'/../README.md'; @@ -29,6 +32,26 @@ public function testConstruction() $this->assertTrue($response->headers->has('Last-Modified')); $this->assertFalse($response->headers->has('Content-Disposition')); + $response = new BinaryFileResponse($file, 404, [], true, ResponseHeaderBag::DISPOSITION_INLINE); + $this->assertEquals(404, $response->getStatusCode()); + $this->assertFalse($response->headers->has('ETag')); + $this->assertEquals('inline; filename=README.md', $response->headers->get('Content-Disposition')); + } + + /** + * @group legacy + */ + public function testConstructionLegacy() + { + $file = __DIR__.'/../README.md'; + $this->expectDeprecation('Since symfony/http-foundation 5.2: The "Symfony\Component\HttpFoundation\BinaryFileResponse::create()" method is deprecated, use "new Symfony\Component\HttpFoundation\BinaryFileResponse()" instead.'); + $response = BinaryFileResponse::create($file, 404, ['X-Header' => 'Foo'], true, null, true, true); + $this->assertEquals(404, $response->getStatusCode()); + $this->assertEquals('Foo', $response->headers->get('X-Header')); + $this->assertTrue($response->headers->has('ETag')); + $this->assertTrue($response->headers->has('Last-Modified')); + $this->assertFalse($response->headers->has('Content-Disposition')); + $response = BinaryFileResponse::create($file, 404, [], true, ResponseHeaderBag::DISPOSITION_INLINE); $this->assertEquals(404, $response->getStatusCode()); $this->assertFalse($response->headers->has('ETag')); @@ -83,7 +106,7 @@ public function testSetContentDispositionGeneratesSafeFallbackFilenameForWrongly */ public function testRequests($requestRange, $offset, $length, $responseRange) { - $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream'])->setAutoEtag(); + $response = (new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']))->setAutoEtag(); // do a request to get the ETag $request = Request::create('/'); @@ -115,7 +138,7 @@ public function testRequests($requestRange, $offset, $length, $responseRange) */ public function testRequestsWithoutEtag($requestRange, $offset, $length, $responseRange) { - $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']); + $response = new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']); // do a request to get the LastModified $request = Request::create('/'); @@ -156,7 +179,7 @@ public function provideRanges() public function testRangeRequestsWithoutLastModifiedDate() { // prevent auto last modified - $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream'], true, null, false, false); + $response = new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream'], true, null, false, false); // prepare a request for a range of the testing file $request = Request::create('/'); @@ -177,7 +200,7 @@ public function testRangeRequestsWithoutLastModifiedDate() */ public function testFullFileRequests($requestRange) { - $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream'])->setAutoEtag(); + $response = (new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']))->setAutoEtag(); // prepare a request for a range of the testing file $request = Request::create('/'); @@ -213,7 +236,7 @@ public function testRangeOnPostMethod() { $request = Request::create('/', 'POST'); $request->headers->set('Range', 'bytes=10-20'); - $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']); + $response = new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']); $file = fopen(__DIR__.'/File/Fixtures/test.gif', 'r'); $data = fread($file, 35); @@ -231,7 +254,7 @@ public function testRangeOnPostMethod() public function testUnpreparedResponseSendsFullFile() { - $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200); + $response = new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200); $data = file_get_contents(__DIR__.'/File/Fixtures/test.gif'); @@ -247,7 +270,7 @@ public function testUnpreparedResponseSendsFullFile() */ public function testInvalidRequests($requestRange) { - $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream'])->setAutoEtag(); + $response = (new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']))->setAutoEtag(); // prepare a request for a range of the testing file $request = Request::create('/'); @@ -278,7 +301,7 @@ public function testXSendfile($file) $request->headers->set('X-Sendfile-Type', 'X-Sendfile'); BinaryFileResponse::trustXSendfileTypeHeader(); - $response = BinaryFileResponse::create($file, 200, ['Content-Type' => 'application/octet-stream']); + $response = new BinaryFileResponse($file, 200, ['Content-Type' => 'application/octet-stream']); $response->prepare($request); $this->expectOutputString(''); @@ -338,7 +361,7 @@ public function testDeleteFileAfterSend() public function testAcceptRangeOnUnsafeMethods() { $request = Request::create('/', 'POST'); - $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']); + $response = new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']); $response->prepare($request); $this->assertEquals('none', $response->headers->get('Accept-Ranges')); @@ -347,7 +370,7 @@ public function testAcceptRangeOnUnsafeMethods() public function testAcceptRangeNotOverriden() { $request = Request::create('/', 'POST'); - $response = BinaryFileResponse::create(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']); + $response = new BinaryFileResponse(__DIR__.'/File/Fixtures/test.gif', 200, ['Content-Type' => 'application/octet-stream']); $response->headers->set('Accept-Ranges', 'foo'); $response->prepare($request); From ba0aeb1f49a2e687446fb82b828e957dfd999fe1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anderson=20M=C3=BCller?= Date: Fri, 20 Nov 2020 10:31:29 +0100 Subject: [PATCH 20/20] [HttpFoundation] Typo on deprecation package name --- Request.php | 2 +- Tests/RequestTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Request.php b/Request.php index 08f54a978..7df9a962b 100644 --- a/Request.php +++ b/Request.php @@ -595,7 +595,7 @@ public function overrideGlobals() public static function setTrustedProxies(array $proxies, int $trustedHeaderSet) { if (self::HEADER_X_FORWARDED_ALL === $trustedHeaderSet) { - trigger_deprecation('symfony/http-fundation', '5.2', 'The "HEADER_X_FORWARDED_ALL" constant is deprecated, use either "HEADER_X_FORWARDED_FOR | HEADER_X_FORWARDED_HOST | HEADER_X_FORWARDED_PORT | HEADER_X_FORWARDED_PROTO" or "HEADER_X_FORWARDED_AWS_ELB" or "HEADER_X_FORWARDED_TRAEFIK" constants instead.'); + trigger_deprecation('symfony/http-foundation', '5.2', 'The "HEADER_X_FORWARDED_ALL" constant is deprecated, use either "HEADER_X_FORWARDED_FOR | HEADER_X_FORWARDED_HOST | HEADER_X_FORWARDED_PORT | HEADER_X_FORWARDED_PROTO" or "HEADER_X_FORWARDED_AWS_ELB" or "HEADER_X_FORWARDED_TRAEFIK" constants instead.'); } self::$trustedProxies = array_reduce($proxies, function ($proxies, $proxy) { if ('REMOTE_ADDR' !== $proxy) { diff --git a/Tests/RequestTest.php b/Tests/RequestTest.php index 3ed919b89..0b9cfefa2 100644 --- a/Tests/RequestTest.php +++ b/Tests/RequestTest.php @@ -2503,7 +2503,7 @@ public function preferSafeContentData() */ public function testXForwarededAllConstantDeprecated() { - $this->expectDeprecation('Since symfony/http-fundation 5.2: The "HEADER_X_FORWARDED_ALL" constant is deprecated, use either "HEADER_X_FORWARDED_FOR | HEADER_X_FORWARDED_HOST | HEADER_X_FORWARDED_PORT | HEADER_X_FORWARDED_PROTO" or "HEADER_X_FORWARDED_AWS_ELB" or "HEADER_X_FORWARDED_TRAEFIK" constants instead.'); + $this->expectDeprecation('Since symfony/http-foundation 5.2: The "HEADER_X_FORWARDED_ALL" constant is deprecated, use either "HEADER_X_FORWARDED_FOR | HEADER_X_FORWARDED_HOST | HEADER_X_FORWARDED_PORT | HEADER_X_FORWARDED_PROTO" or "HEADER_X_FORWARDED_AWS_ELB" or "HEADER_X_FORWARDED_TRAEFIK" constants instead.'); Request::setTrustedProxies([], Request::HEADER_X_FORWARDED_ALL); }