[HttpFoundation] Fix AcceptHeader overwrites items with different parameters

Author: yoeunes

AcceptHeader.php

@@ -25,7 +25,7 @@ class_exists(AcceptHeaderItem::class);
 class AcceptHeader
 {
     /**
-     * @var AcceptHeaderItem[]
+     * @var array<string, AcceptHeaderItem>
      */
     private array $items = [];
 
@@ -73,15 +73,35 @@ public function __toString(): string
      */
     public function has(string $value): bool
     {
-        return isset($this->items[$value]);
+        $canonicalKey = $this->getCanonicalKey(AcceptHeaderItem::fromString($value));
+
+        return isset($this->items[$canonicalKey]);
     }
 
     /**
      * Returns given value's item, if exists.
      */
     public function get(string $value): ?AcceptHeaderItem
     {
-        return $this->items[$value] ?? $this->items[explode('/', $value)[0].'/*'] ?? $this->items['*/*'] ?? $this->items['*'] ?? null;
+        $queryItem = AcceptHeaderItem::fromString($value.';q=1');
+        $canonicalKey = $this->getCanonicalKey($queryItem);
+
+        if (isset($this->items[$canonicalKey])) {
+            return $this->items[$canonicalKey];
+        }
+
+        // Collect and filter matching candidates
+        if (!$candidates = array_filter($this->items, fn (AcceptHeaderItem $item) => $this->matches($item, $queryItem))) {
+            return null;
+        }
+
+        usort($candidates, fn ($a, $b) =>
+            $this->getSpecificity($b, $queryItem) <=> $this->getSpecificity($a, $queryItem) // Descending specificity
+            ?: $b->getQuality() <=> $a->getQuality() // Descending quality
+            ?: $a->getIndex() <=> $b->getIndex() // Ascending index (stability)
+        );
+
+        return reset($candidates);
     }
 
     /**
@@ -91,7 +111,7 @@ public function get(string $value): ?AcceptHeaderItem
      */
     public function add(AcceptHeaderItem $item): static
     {
-        $this->items[$item->getValue()] = $item;
+        $this->items[$this->getCanonicalKey($item)] = $item;
         $this->sorted = false;
 
         return $this;
@@ -114,7 +134,7 @@ public function all(): array
      */
     public function filter(string $pattern): self
     {
-        return new self(array_filter($this->items, fn (AcceptHeaderItem $item) => preg_match($pattern, $item->getValue())));
+        return new self(array_filter($this->items, static fn ($item) => preg_match($pattern, $item->getValue())));
     }
 
     /**
@@ -133,18 +153,154 @@ public function first(): ?AcceptHeaderItem
     private function sort(): void
     {
         if (!$this->sorted) {
-            uasort($this->items, function (AcceptHeaderItem $a, AcceptHeaderItem $b) {
-                $qA = $a->getQuality();
-                $qB = $b->getQuality();
+            uasort($this->items, static fn ($a, $b) => $b->getQuality() <=> $a->getQuality() ?: $a->getIndex() <=> $b->getIndex());
+
+            $this->sorted = true;
+        }
+    }
 
-                if ($qA === $qB) {
-                    return $a->getIndex() > $b->getIndex() ? 1 : -1;
-                }
+    /**
+     * Generates the canonical key for storing/retrieving an item.
+     */
+    private function getCanonicalKey(AcceptHeaderItem $item): string
+    {
+        $parts = [];
 
-                return $qA > $qB ? -1 : 1;
-            });
+        // Normalize and sort attributes for consistent key generation
+        $attributes = $this->getMediaParams($item);
+        ksort($attributes);
 
-            $this->sorted = true;
+        foreach ($attributes as $name => $value) {
+            if (null === $value) {
+                $parts[] = $name; // Flag parameter (e.g., "flowed")
+                continue;
+            }
+
+            // Quote values containing spaces, commas, semicolons, or equals per RFC 9110
+            // This handles cases like 'format="value with space"' or similar.
+            $quotedValue = \is_string($value) && preg_match('/[\s;,=]/', $value) ? '"'.addcslashes($value, '"\\').'"' : $value;
+
+            $parts[] = $name.'='.$quotedValue;
         }
+
+        return $item->getValue().($parts ? ';'.implode(';', $parts) : '');
+    }
+
+    /**
+     * Checks if a given header item (range) matches a queried item (value).
+     *
+     * @param AcceptHeaderItem $rangeItem The item from the Accept header (e.g., text/*;format=flowed)
+     * @param AcceptHeaderItem $queryItem The item being queried (e.g., text/plain;format=flowed;charset=utf-8)
+     */
+    private function matches(AcceptHeaderItem $rangeItem, AcceptHeaderItem $queryItem): bool
+    {
+        $rangeValue = strtolower($rangeItem->getValue());
+        $queryValue = strtolower($queryItem->getValue());
+
+        // Handle universal wildcard ranges
+        if ('*' === $rangeValue || '*/*' === $rangeValue) {
+            return $this->rangeParametersMatch($rangeItem, $queryItem);
+        }
+
+        // Queries for '*' only match wildcard ranges (handled above)
+        if ('*' === $queryValue) {
+            return false;
+        }
+
+        // Ensure media vs. non-media consistency
+        $isQueryMedia = str_contains($queryValue, '/');
+        $isRangeMedia = str_contains($rangeValue, '/');
+
+        if ($isQueryMedia !== $isRangeMedia) {
+            return false;
+        }
+
+        // Non-media: exact match only (wildcards handled above)
+        if (!$isQueryMedia) {
+            return $rangeValue === $queryValue && $this->rangeParametersMatch($rangeItem, $queryItem);
+        }
+
+        // Media type: type/subtype with wildcards
+        [$queryType, $querySubtype] = explode('/', $queryValue, 2);
+        [$rangeType, $rangeSubtype] = explode('/', $rangeValue, 2) + [1 => '*'];
+
+        if ('*' !== $rangeType && $rangeType !== $queryType) {
+            return false;
+        }
+
+        if ('*' !== $rangeSubtype && $rangeSubtype !== $querySubtype) {
+            return false;
+        }
+
+        // Parameters must match
+        return $this->rangeParametersMatch($rangeItem, $queryItem);
+    }
+
+    /**
+     * Checks if the parameters of a range item are satisfied by the query item.
+     *
+     * Parameters are case-insensitive; range params must be a subset of query params.
+     */
+    private function rangeParametersMatch(AcceptHeaderItem $rangeItem, AcceptHeaderItem $queryItem): bool
+    {
+        $queryAttributes = $this->getMediaParams($queryItem);
+        $rangeAttributes = $this->getMediaParams($rangeItem);
+
+        foreach ($rangeAttributes as $name => $rangeValue) {
+            if (!\array_key_exists($name, $queryAttributes)) {
+                return false; // Missing required param
+            }
+
+            $queryValue = $queryAttributes[$name];
+
+            if (null === $rangeValue) {
+                return null === $queryValue; // Both flags or neither
+            }
+
+            if (null === $queryValue || strtolower($queryValue) !== strtolower($rangeValue)) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    /**
+     * Calculates a specificity score for sorting: media precision + param count.
+     */
+    private function getSpecificity(AcceptHeaderItem $item, AcceptHeaderItem $queryItem): int
+    {
+        $rangeValue = strtolower($item->getValue());
+        $queryValue = strtolower($queryItem->getValue());
+
+        $paramCount = \count($this->getMediaParams($item));
+
+        $isQueryMedia = str_contains($queryValue, '/');
+        $isRangeMedia = str_contains($rangeValue, '/');
+
+        if (!$isQueryMedia && !$isRangeMedia) {
+            return ('*' !== $rangeValue ? 2000 : 1000) + $paramCount;
+        }
+
+        [$rangeType, $rangeSubtype] = explode('/', $rangeValue, 2) + [1 => '*'];
+
+        $specificity = match (true) {
+            '*' !== $rangeSubtype => 3000, // Exact subtype (text/plain)
+            '*' !== $rangeType => 2000,    // Type wildcard (text/*)
+            default => 1000,               // Full wildcard (*/* or *)
+        };
+
+        return $specificity + $paramCount;
+    }
+
+    /**
+     * Returns normalized attributes: keys lowercased, excluding 'q'.
+     */
+    private function getMediaParams(AcceptHeaderItem $item): array
+    {
+        $attributes = array_change_key_case($item->getAttributes(), \CASE_LOWER);
+        unset($attributes['q']);
+
+        return $attributes;
     }
 }

Tests/AcceptHeaderTest.php

@@ -100,6 +100,8 @@ public static function provideSortingData()
             'quality has priority' => ['*;q=0.3,ISO-8859-1,utf-8;q=0.7', ['ISO-8859-1', 'utf-8', '*']],
             'order matters when q is equal' => ['*;q=0.3,ISO-8859-1;q=0.7,utf-8;q=0.7', ['ISO-8859-1', 'utf-8', '*']],
             'order matters when q is equal2' => ['*;q=0.3,utf-8;q=0.7,ISO-8859-1;q=0.7', ['utf-8', 'ISO-8859-1', '*']],
+            'additional attributes like "format" should be handled according RFC 9110' => ['text/*;q=0.3, text/plain;q=0.7, text/plain;format=flowed, text/plain;format=fixed;q=0.4, */*;q=0.5', ['text/plain;format=flowed', 'text/plain', '*/*', 'text/plain;format=fixed', 'text/*']],
+            'additional attributes like "format" should be handled according obsoleted RFC 7231 as well' => ['text/*;q=0.3, text/html;q=0.7, text/html;level=1, text/html;level=2;q=0.4, */*;q=0.5', ['text/html;level=1', 'text/html', '*/*', 'text/html;level=2', 'text/*']],
         ];
     }
 
@@ -109,7 +111,7 @@ public static function provideSortingData()
     public function testDefaultValue($acceptHeader, $value, $expectedQuality)
     {
         $header = AcceptHeader::fromString($acceptHeader);
-        $this->assertSame($expectedQuality, $header->get($value)->getQuality());
+        $this->assertSame($expectedQuality, $header->get($value)?->getQuality());
     }
 
     public static function provideDefaultValueData()
@@ -128,5 +130,50 @@ public static function provideDefaultValueData()
         yield ['*;q=0.3, ISO-8859-1;q=0.7, utf-8;q=0.7', '*', 0.3];
         yield ['*;q=0.3, ISO-8859-1;q=0.7, utf-8;q=0.7', 'utf-8', 0.7];
         yield ['*;q=0.3, ISO-8859-1;q=0.7, utf-8;q=0.7', 'SHIFT_JIS', 0.3];
+        yield 'additional attributes like "format" should be handled according RFC 9110' => ['text/*;q=0.3, text/plain;q=0.7, text/plain;format=flowed, text/plain;format=fixed;q=0.4, */*;q=0.5', 'text/plain;format=flowed', 1.0];
+        yield 'additional attributes like "format" should be handled according obsoleted RFC 7231 as well' => ['text/*;q=0.3, text/html;q=0.7, text/html;level=1, text/html;level=2;q=0.4, */*;q=0.5', 'text/html;level=1', 1.0];
+
+        // Edge cases for case-insensitivity
+        yield 'case-insensitive param names' => ['text/plain;format=flowed;q=0.8, text/plain;Format=fixed', 'text/plain;format=fixed', 1.0];
+        yield 'case-insensitive charset' => ['text/plain;Charset=utf-8', 'text/plain;charset=utf-8', 1.0];
+
+        // Quoted values and specials
+        yield 'quoted value with space' => ['text/plain;param="value with space"', 'text/plain;param="value with space"', 1.0];
+        yield 'quoted value with backslash' => ['text/plain;param="value\\with\\backslash"', 'text/plain;param="value\\with\\backslash"', 1.0];
+        yield 'mismatched quoted' => ['text/plain;param="value with space"', 'text/plain;param=value with space', 1.0];
+
+        // Flag params or empty
+        yield 'flag param' => ['text/plain;flowed;q=0.9', 'text/plain;flowed', 0.9];
+        yield 'empty param value' => ['text/plain;param=', 'text/plain;param=""', 1.0];
+        yield 'missing required flag' => ['text/plain;flowed', 'text/plain', null];
+
+        // Extra params in query
+        yield 'extra param in query' => ['text/plain;format=flowed', 'text/plain;format=flowed;charset=utf-8', 1.0];
+        yield 'missing required param in query' => ['text/plain;format=flowed', 'text/plain;charset=utf-8', null];
+        yield 'wildcard with param' => ['text/*;format=flowed', 'text/plain;format=flowed', 1.0];
+        yield 'wildcard missing param' => ['text/*;format=flowed', 'text/plain', null];
+
+        // Wildcards and specificity
+        yield 'specificity priority' => ['*/*;q=0.1, text/*;format=flowed;q=0.5, text/plain;q=0.8', 'text/plain;format=flowed', 0.8];
+        yield 'wildcard with param match' => ['*/*;param=test', 'text/plain;param=test', 1.0];
+        yield 'wildcard with param no match' => ['*/*;param=test', 'text/plain', null];
+
+        // Non-media types
+        yield 'charset wildcard' => ['utf-8;q=0.9, *;q=0.5', 'iso-8859-1', 0.5];
+        yield 'language star' => ['*;q=0.5', 'en-US', 0.5];
+        yield 'non-media */*' => ['*/*;q=0.5', 'utf-8', 0.5];
+
+        // Ties and duplicates
+        yield 'duplicate params tie on index' => ['text/plain;format=flowed;q=0.8, text/plain;format=flowed;q=0.8', 'text/plain;format=flowed', 0.8];
+        yield 'param count tie' => ['text/plain;q=0.5, text/plain;format=flowed;q=0.5', 'text/plain;format=flowed;extra=foo', 0.5];
+
+        // Invalid/malformed
+        yield 'non-media invalid' => ['text', 'text', 1.0];
+        yield 'invalid subtype' => ['text/', 'text/plain', null];
+        yield 'empty header' => ['', 'text/plain', null];
+
+        // Mixed case types
+        yield 'mixed case type' => ['Text/Plain;Format=flowed', 'text/plain;format=flowed', 1.0];
+        yield 'mixed case charset' => ['UTF-8;q=0.9', 'utf-8', 0.9];
     }
 }