Skip to content

Commit cac17e0

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '6.4' into 7.0
* 6.4: (33 commits) [Console][FrameworkBundle][HttpKernel][WebProfilerBundle] Enable profiling commands [AssetMapper] Disable profiler when the "dev server" respond Adds translations for Portuguese (pt) [AssetMapper] Link needs as="style" Allow Symfony 7.0 on Phrase translation provider [Mime] Throw InvalidArgumentException on invalid form field type inside array [Mailer][Bridges] Allow Symfony 7 [Tests] Use `JsonMockResponse` where applicable [FrameworkBundle][HttpKernel] Introduce `$buildDir` argument to `WarmableInterface::warmup` to warm read-only artefacts in `build_dir` [ErrorHandler] Fix expected missing return types [Form] Fix merging params & files when "multiple" is enabled [HttpFoundation] Do not swallow trailing `=` in cookie value Fix markdown in README files Handle Sendinblue error responses without a message key Handle Brevo error responses without a message key [Scheduler] Add failureEvent [Notifier][Bridges] Allow Symfony 7 [Mailer][Brevo][Sendinblue] Fix typo [Serializer] Fix collecting only first missing constructor argument [ErrorHandler] Fix file link format call in trace view ...
2 parents 3797659 + da6dc96 commit cac17e0

File tree

5 files changed

+73
-37
lines changed

5 files changed

+73
-37
lines changed

‎HeaderUtils.php

Lines changed: 32 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -33,17 +33,21 @@ private function __construct()
3333
*
3434
* Example:
3535
*
36-
* HeaderUtils::split("da, en-gb;q=0.8", ",;")
36+
* HeaderUtils::split('da, en-gb;q=0.8', ',;')
3737
* // => ['da'], ['en-gb', 'q=0.8']]
3838
*
3939
* @param string $separators List of characters to split on, ordered by
40-
* precedence, e.g. ",", ";=", or ",;="
40+
* precedence, e.g. ',', ';=', or ',;='
4141
*
4242
* @return array Nested array with as many levels as there are characters in
4343
* $separators
4444
*/
4545
public static function split(string $header, string $separators): array
4646
{
47+
if ('' === $separators) {
48+
throw new \InvalidArgumentException('At least one separator must be specified.');
49+
}
50+
4751
$quotedSeparators = preg_quote($separators, '/');
4852

4953
preg_match_all('
@@ -77,8 +81,8 @@ public static function split(string $header, string $separators): array
7781
*
7882
* Example:
7983
*
80-
* HeaderUtils::combine([["foo", "abc"], ["bar"]])
81-
* // => ["foo" => "abc", "bar" => true]
84+
* HeaderUtils::combine([['foo', 'abc'], ['bar']])
85+
* // => ['foo' => 'abc', 'bar' => true]
8286
*/
8387
public static function combine(array $parts): array
8488
{
@@ -95,13 +99,13 @@ public static function combine(array $parts): array
9599
/**
96100
* Joins an associative array into a string for use in an HTTP header.
97101
*
98-
* The key and value of each entry are joined with "=", and all entries
102+
* The key and value of each entry are joined with '=', and all entries
99103
* are joined with the specified separator and an additional space (for
100104
* readability). Values are quoted if necessary.
101105
*
102106
* Example:
103107
*
104-
* HeaderUtils::toString(["foo" => "abc", "bar" => true, "baz" => "a b c"], ",")
108+
* HeaderUtils::toString(['foo' => 'abc', 'bar' => true, 'baz' => 'a b c'], ',')
105109
* // => 'foo=abc, bar, baz="a b c"'
106110
*/
107111
public static function toString(array $assoc, string $separator): string
@@ -252,40 +256,37 @@ public static function parseQuery(string $query, bool $ignoreBrackets = false, s
252256
private static function groupParts(array $matches, string $separators, bool $first = true): array
253257
{
254258
$separator = $separators[0];
255-
$partSeparators = substr($separators, 1);
256-
259+
$separators = substr($separators, 1);
257260
$i = 0;
261+
262+
if ('' === $separators && !$first) {
263+
$parts = [''];
264+
265+
foreach ($matches as $match) {
266+
if (!$i && isset($match['separator'])) {
267+
$i = 1;
268+
$parts[1] = '';
269+
} else {
270+
$parts[$i] .= self::unquote($match[0]);
271+
}
272+
}
273+
274+
return $parts;
275+
}
276+
277+
$parts = [];
258278
$partMatches = [];
259-
$previousMatchWasSeparator = false;
279+
260280
foreach ($matches as $match) {
261-
if (!$first && $previousMatchWasSeparator && isset($match['separator']) && $match['separator'] === $separator) {
262-
$previousMatchWasSeparator = true;
263-
$partMatches[$i][] = $match;
264-
} elseif (isset($match['separator']) && $match['separator'] === $separator) {
265-
$previousMatchWasSeparator = true;
281+
if (($match['separator'] ?? null) === $separator) {
266282
++$i;
267283
} else {
268-
$previousMatchWasSeparator = false;
269284
$partMatches[$i][] = $match;
270285
}
271286
}
272287

273-
$parts = [];
274-
if ($partSeparators) {
275-
foreach ($partMatches as $matches) {
276-
$parts[] = self::groupParts($matches, $partSeparators, false);
277-
}
278-
} else {
279-
foreach ($partMatches as $matches) {
280-
$parts[] = self::unquote($matches[0][0]);
281-
}
282-
283-
if (!$first && 2 < \count($parts)) {
284-
$parts = [
285-
$parts[0],
286-
implode($separator, \array_slice($parts, 1)),
287-
];
288-
}
288+
foreach ($partMatches as $matches) {
289+
$parts[] = '' === $separators ? self::unquote($matches[0][0]) : self::groupParts($matches, $separators, false);
289290
}
290291

291292
return $parts;

‎Request.php

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,8 @@ class Request
164164
private bool $isForwardedValid = true;
165165
private bool $isSafeContentPreferred;
166166

167+
private array $trustedValuesCache = [];
168+
167169
private static int $trustedHeaderSet = -1;
168170

169171
private const FORWARDED_PARAMS = [
@@ -1910,8 +1912,20 @@ public function isFromTrustedProxy(): bool
19101912
return self::$trustedProxies && IpUtils::checkIp($this->server->get('REMOTE_ADDR', ''), self::$trustedProxies);
19111913
}
19121914

1915+
/**
1916+
* This method is rather heavy because it splits and merges headers, and it's called by many other methods such as
1917+
* getPort(), isSecure(), getHost(), getClientIps(), getBaseUrl() etc. Thus, we try to cache the results for
1918+
* best performance.
1919+
*/
19131920
private function getTrustedValues(int $type, string $ip = null): array
19141921
{
1922+
$cacheKey = $type."\0".((self::$trustedHeaderSet & $type) ? $this->headers->get(self::TRUSTED_HEADERS[$type]) : '');
1923+
$cacheKey .= "\0".$ip."\0".$this->headers->get(self::TRUSTED_HEADERS[self::HEADER_FORWARDED]);
1924+
1925+
if (isset($this->trustedValuesCache[$cacheKey])) {
1926+
return $this->trustedValuesCache[$cacheKey];
1927+
}
1928+
19151929
$clientValues = [];
19161930
$forwardedValues = [];
19171931

@@ -1924,7 +1938,6 @@ private function getTrustedValues(int $type, string $ip = null): array
19241938
if ((self::$trustedHeaderSet & self::HEADER_FORWARDED) && (isset(self::FORWARDED_PARAMS[$type])) && $this->headers->has(self::TRUSTED_HEADERS[self::HEADER_FORWARDED])) {
19251939
$forwarded = $this->headers->get(self::TRUSTED_HEADERS[self::HEADER_FORWARDED]);
19261940
$parts = HeaderUtils::split($forwarded, ',;=');
1927-
$forwardedValues = [];
19281941
$param = self::FORWARDED_PARAMS[$type];
19291942
foreach ($parts as $subParts) {
19301943
if (null === $v = HeaderUtils::combine($subParts)[$param] ?? null) {
@@ -1946,15 +1959,15 @@ private function getTrustedValues(int $type, string $ip = null): array
19461959
}
19471960

19481961
if ($forwardedValues === $clientValues || !$clientValues) {
1949-
return $forwardedValues;
1962+
return $this->trustedValuesCache[$cacheKey] = $forwardedValues;
19501963
}
19511964

19521965
if (!$forwardedValues) {
1953-
return $clientValues;
1966+
return $this->trustedValuesCache[$cacheKey] = $clientValues;
19541967
}
19551968

19561969
if (!$this->isForwardedValid) {
1957-
return null !== $ip ? ['0.0.0.0', $ip] : [];
1970+
return $this->trustedValuesCache[$cacheKey] = null !== $ip ? ['0.0.0.0', $ip] : [];
19581971
}
19591972
$this->isForwardedValid = false;
19601973

‎Tests/CookieTest.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,9 @@ public function testFromString()
351351
$cookie = Cookie::fromString('foo=bar', true);
352352
$this->assertEquals(Cookie::create('foo', 'bar', 0, '/', null, false, false, false, null), $cookie);
353353

354+
$cookie = Cookie::fromString('foo=bar=', true);
355+
$this->assertEquals(Cookie::create('foo', 'bar=', 0, '/', null, false, false, false, null), $cookie);
356+
354357
$cookie = Cookie::fromString('foo', true);
355358
$this->assertEquals(Cookie::create('foo', null, 0, '/', null, false, false, false, null), $cookie);
356359

‎Tests/HeaderUtilsTest.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public static function provideHeaderToSplit(): array
3434
[['foo', '123, bar'], 'foo=123, bar', '='],
3535
[['foo', '123, bar'], ' foo = 123, bar ', '='],
3636
[[['foo', '123'], ['bar']], 'foo=123, bar', ',='],
37-
[[[['foo', '123']], [['bar'], ['foo', '456']]], 'foo=123, bar; foo=456', ',;='],
37+
[[[['foo', '123']], [['bar'], ['foo', '456']]], 'foo=123, bar;; foo=456', ',;='],
3838
[[[['foo', 'a,b;c=d']]], 'foo="a,b;c=d"', ',;='],
3939

4040
[['foo', 'bar'], 'foo,,,, bar', ','],
@@ -46,13 +46,15 @@ public static function provideHeaderToSplit(): array
4646

4747
[[['foo_cookie', 'foo=1&bar=2&baz=3'], ['expires', 'Tue, 22-Sep-2020 06:27:09 GMT'], ['path', '/']], 'foo_cookie=foo=1&bar=2&baz=3; expires=Tue, 22-Sep-2020 06:27:09 GMT; path=/', ';='],
4848
[[['foo_cookie', 'foo=='], ['expires', 'Tue, 22-Sep-2020 06:27:09 GMT'], ['path', '/']], 'foo_cookie=foo==; expires=Tue, 22-Sep-2020 06:27:09 GMT; path=/', ';='],
49+
[[['foo_cookie', 'foo='], ['expires', 'Tue, 22-Sep-2020 06:27:09 GMT'], ['path', '/']], 'foo_cookie=foo=; expires=Tue, 22-Sep-2020 06:27:09 GMT; path=/', ';='],
4950
[[['foo_cookie', 'foo=a=b'], ['expires', 'Tue, 22-Sep-2020 06:27:09 GMT'], ['path', '/']], 'foo_cookie=foo="a=b"; expires=Tue, 22-Sep-2020 06:27:09 GMT; path=/', ';='],
5051

5152
// These are not a valid header values. We test that they parse anyway,
5253
// and that both the valid and invalid parts are returned.
5354
[[], '', ','],
5455
[[], ',,,', ','],
55-
[['foo', 'bar', 'baz'], 'foo, "bar", "baz', ','],
56+
[[['', 'foo'], ['bar', '']], '=foo,bar=', ',='],
57+
[['foo', 'foobar', 'baz'], 'foo, foo"bar", "baz', ','],
5658
[['foo', 'bar, baz'], 'foo, "bar, baz', ','],
5759
[['foo', 'bar, baz\\'], 'foo, "bar, baz\\', ','],
5860
[['foo', 'bar, baz\\'], 'foo, "bar, baz\\\\', ','],

‎Tests/RequestTest.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2534,6 +2534,23 @@ public function testTrustedProxiesRemoteAddr($serverRemoteAddr, $trustedProxies,
25342534
$this->assertSame($result, Request::getTrustedProxies());
25352535
}
25362536

2537+
public function testTrustedValuesCache()
2538+
{
2539+
$request = Request::create('http://example.com/');
2540+
$request->server->set('REMOTE_ADDR', '3.3.3.3');
2541+
$request->headers->set('X_FORWARDED_FOR', '1.1.1.1, 2.2.2.2');
2542+
$request->headers->set('X_FORWARDED_PROTO', 'https');
2543+
2544+
$this->assertFalse($request->isSecure());
2545+
2546+
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);
2547+
$this->assertTrue($request->isSecure());
2548+
2549+
// Header is changed, cache must not be hit now
2550+
$request->headers->set('X_FORWARDED_PROTO', 'http');
2551+
$this->assertFalse($request->isSecure());
2552+
}
2553+
25372554
public static function trustedProxiesRemoteAddr()
25382555
{
25392556
return [

0 commit comments

Comments
 (0)