bug symfony#51819 [HttpFoundation] Do not swallow trailing = in cookie value (OskarStark)

This PR was merged into the 5.4 branch.

Discussion
----------

[HttpFoundation] Do not swallow trailing `=` in cookie value

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix symfony#51814
| License       | MIT

cc `@pschultz` as you opened the bug report

Commits
-------

77ca01c [HttpFoundation] Do not swallow trailing `=` in cookie value

Author: nicolas-grekas

src/Symfony/Component/HttpFoundation/HeaderUtils.php

@@ -33,17 +33,21 @@ private function __construct()
      *
      * Example:
      *
-     *     HeaderUtils::split("da, en-gb;q=0.8", ",;")
+     *     HeaderUtils::split('da, en-gb;q=0.8', ',;')
      *     // => ['da'], ['en-gb', 'q=0.8']]
      *
      * @param string $separators List of characters to split on, ordered by
-     *                           precedence, e.g. ",", ";=", or ",;="
+     *                           precedence, e.g. ',', ';=', or ',;='
      *
      * @return array Nested array with as many levels as there are characters in
      *               $separators
      */
     public static function split(string $header, string $separators): array
     {
+        if ('' === $separators) {
+            throw new \InvalidArgumentException('At least one separator must be specified.');
+        }
+
         $quotedSeparators = preg_quote($separators, '/');
 
         preg_match_all('
@@ -77,8 +81,8 @@ public static function split(string $header, string $separators): array
      *
      * Example:
      *
-     *     HeaderUtils::combine([["foo", "abc"], ["bar"]])
-     *     // => ["foo" => "abc", "bar" => true]
+     *     HeaderUtils::combine([['foo', 'abc'], ['bar']])
+     *     // => ['foo' => 'abc', 'bar' => true]
      */
     public static function combine(array $parts): array
     {
@@ -95,13 +99,13 @@ public static function combine(array $parts): array
     /**
      * Joins an associative array into a string for use in an HTTP header.
      *
-     * The key and value of each entry are joined with "=", and all entries
+     * The key and value of each entry are joined with '=', and all entries
      * are joined with the specified separator and an additional space (for
      * readability). Values are quoted if necessary.
      *
      * Example:
      *
-     *     HeaderUtils::toString(["foo" => "abc", "bar" => true, "baz" => "a b c"], ",")
+     *     HeaderUtils::toString(['foo' => 'abc', 'bar' => true, 'baz' => 'a b c'], ',')
      *     // => 'foo=abc, bar, baz="a b c"'
      */
     public static function toString(array $assoc, string $separator): string
@@ -252,40 +256,37 @@ public static function parseQuery(string $query, bool $ignoreBrackets = false, s
     private static function groupParts(array $matches, string $separators, bool $first = true): array
     {
         $separator = $separators[0];
-        $partSeparators = substr($separators, 1);
-
+        $separators = substr($separators, 1);
         $i = 0;
+
+        if ('' === $separators && !$first) {
+            $parts = [''];
+
+            foreach ($matches as $match) {
+                if (!$i && isset($match['separator'])) {
+                    $i = 1;
+                    $parts[1] = '';
+                } else {
+                    $parts[$i] .= self::unquote($match[0]);
+                }
+            }
+
+            return $parts;
+        }
+
+        $parts = [];
         $partMatches = [];
-        $previousMatchWasSeparator = false;
+
         foreach ($matches as $match) {
-            if (!$first && $previousMatchWasSeparator && isset($match['separator']) && $match['separator'] === $separator) {
-                $previousMatchWasSeparator = true;
-                $partMatches[$i][] = $match;
-            } elseif (isset($match['separator']) && $match['separator'] === $separator) {
-                $previousMatchWasSeparator = true;
+            if (($match['separator'] ?? null) === $separator) {
                 ++$i;
             } else {
-                $previousMatchWasSeparator = false;
                 $partMatches[$i][] = $match;
             }
         }
 
-        $parts = [];
-        if ($partSeparators) {
-            foreach ($partMatches as $matches) {
-                $parts[] = self::groupParts($matches, $partSeparators, false);
-            }
-        } else {
-            foreach ($partMatches as $matches) {
-                $parts[] = self::unquote($matches[0][0]);
-            }
-
-            if (!$first && 2 < \count($parts)) {
-                $parts = [
-                    $parts[0],
-                    implode($separator, \array_slice($parts, 1)),
-                ];
-            }
+        foreach ($partMatches as $matches) {
+            $parts[] = '' === $separators ? self::unquote($matches[0][0]) : self::groupParts($matches, $separators, false);
         }
 
         return $parts;

src/Symfony/Component/HttpFoundation/Tests/CookieTest.php

@@ -324,6 +324,9 @@ public function testFromString()
         $cookie = Cookie::fromString('foo=bar', true);
         $this->assertEquals(Cookie::create('foo', 'bar', 0, '/', null, false, false, false, null), $cookie);
 
+        $cookie = Cookie::fromString('foo=bar=', true);
+        $this->assertEquals(Cookie::create('foo', 'bar=', 0, '/', null, false, false, false, null), $cookie);
+
         $cookie = Cookie::fromString('foo', true);
         $this->assertEquals(Cookie::create('foo', null, 0, '/', null, false, false, false, null), $cookie);
 

src/Symfony/Component/HttpFoundation/Tests/HeaderUtilsTest.php

@@ -34,7 +34,7 @@ public static function provideHeaderToSplit(): array
             [['foo', '123, bar'], 'foo=123, bar', '='],
             [['foo', '123, bar'], ' foo = 123, bar ', '='],
             [[['foo', '123'], ['bar']], 'foo=123, bar', ',='],
-            [[[['foo', '123']], [['bar'], ['foo', '456']]], 'foo=123, bar; foo=456', ',;='],
+            [[[['foo', '123']], [['bar'], ['foo', '456']]], 'foo=123, bar;; foo=456', ',;='],
             [[[['foo', 'a,b;c=d']]], 'foo="a,b;c=d"', ',;='],
 
             [['foo', 'bar'], 'foo,,,, bar', ','],
@@ -46,13 +46,15 @@ public static function provideHeaderToSplit(): array
 
             [[['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=/', ';='],
             [[['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=/', ';='],
+            [[['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=/', ';='],
             [[['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=/', ';='],
 
             // These are not a valid header values. We test that they parse anyway,
             // and that both the valid and invalid parts are returned.
             [[], '', ','],
             [[], ',,,', ','],
-            [['foo', 'bar', 'baz'], 'foo, "bar", "baz', ','],
+            [[['', 'foo'], ['bar', '']], '=foo,bar=', ',='],
+            [['foo', 'foobar', 'baz'], 'foo, foo"bar", "baz', ','],
             [['foo', 'bar, baz'], 'foo, "bar, baz', ','],
             [['foo', 'bar, baz\\'], 'foo, "bar, baz\\', ','],
             [['foo', 'bar, baz\\'], 'foo, "bar, baz\\\\', ','],