Skip to content

Commit 53ba9dd

Browse files
yoeunesnicolas-grekas
yoeunes
authored and
nicolas-grekas
committed
[HttpFoundation] Fix AcceptHeader overwrites items with different parameters
1 parent 55a5d41 commit 53ba9dd

File tree

3 files changed

+488
-5
lines changed

3 files changed

+488
-5
lines changed

‎src/Symfony/Component/HttpFoundation/AcceptHeader.php‎

Lines changed: 179 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class_exists(AcceptHeaderItem::class);
2525
class AcceptHeader
2626
{
2727
/**
28-
* @var AcceptHeaderItem[]
28+
* @var array<string, AcceptHeaderItem>
2929
*/
3030
private array $items = [];
3131

@@ -73,15 +73,31 @@ public function __toString(): string
7373
*/
7474
public function has(string $value): bool
7575
{
76-
return isset($this->items[$value]);
76+
$canonicalKey = $this->getCanonicalKey(AcceptHeaderItem::fromString($value));
77+
78+
return isset($this->items[$canonicalKey]);
7779
}
7880

7981
/**
8082
* Returns given value's item, if exists.
8183
*/
8284
public function get(string $value): ?AcceptHeaderItem
8385
{
84-
return $this->items[$value] ?? $this->items[explode('/', $value)[0].'/*'] ?? $this->items['*/*'] ?? $this->items['*'] ?? null;
86+
$queryItem = AcceptHeaderItem::fromString($value.';q=1');
87+
$canonicalKey = $this->getCanonicalKey($queryItem);
88+
89+
if (isset($this->items[$canonicalKey])) {
90+
return $this->items[$canonicalKey];
91+
}
92+
93+
// Collect and filter matching candidates
94+
if (!$candidates = array_filter($this->items, fn (AcceptHeaderItem $item) => $this->matches($item, $queryItem))) {
95+
return null;
96+
}
97+
98+
$this->sortCandidates($candidates, $queryItem);
99+
100+
return reset($candidates) ?: null;
85101
}
86102

87103
/**
@@ -91,7 +107,7 @@ public function get(string $value): ?AcceptHeaderItem
91107
*/
92108
public function add(AcceptHeaderItem $item): static
93109
{
94-
$this->items[$item->getValue()] = $item;
110+
$this->items[$this->getCanonicalKey($item)] = $item;
95111
$this->sorted = false;
96112

97113
return $this;
@@ -147,4 +163,163 @@ private function sort(): void
147163
$this->sorted = true;
148164
}
149165
}
166+
167+
/**
168+
* Generates the canonical key for storing/retrieving an item.
169+
*/
170+
private function getCanonicalKey(AcceptHeaderItem $item): string
171+
{
172+
$parts = [];
173+
174+
// Normalize and sort attributes for consistent key generation
175+
$attributes = $this->getMediaParams($item);
176+
ksort($attributes);
177+
178+
foreach ($attributes as $name => $value) {
179+
if (null === $value) {
180+
$parts[] = $name; // Flag parameter (e.g., "flowed")
181+
continue;
182+
}
183+
184+
// Quote values containing spaces, commas, semicolons, or equals per RFC 9110
185+
// This handles cases like 'format="value with space"' or similar.
186+
$quotedValue = (\is_string($value) && 1 === preg_match('/[\s;,=]/', $value))
187+
? '"'.addcslashes($value, '"\\').'"'
188+
: $value;
189+
190+
$parts[] = $name.'='.$quotedValue;
191+
}
192+
193+
return $item->getValue().($parts ? ';'.implode(';', $parts) : '');
194+
}
195+
196+
/**
197+
* Sorts candidates by specificity (desc), quality (desc), and index (asc).
198+
*
199+
* @param AcceptHeaderItem[] $candidates
200+
*/
201+
private function sortCandidates(array &$candidates, AcceptHeaderItem $queryItem): void
202+
{
203+
usort($candidates, fn (AcceptHeaderItem $a, AcceptHeaderItem $b) => $this->getSpecificity($b, $queryItem) <=> $this->getSpecificity($a, $queryItem) // Descending specificity
204+
?: $b->getQuality() <=> $a->getQuality() // Descending quality
205+
?: $a->getIndex() <=> $b->getIndex() // Ascending index (stability)
206+
);
207+
}
208+
209+
/**
210+
* Checks if a given header item (range) matches a queried item (value).
211+
*
212+
* @param AcceptHeaderItem $rangeItem The item from the Accept header (e.g., text/*;format=flowed)
213+
* @param AcceptHeaderItem $queryItem The item being queried (e.g., text/plain;format=flowed;charset=utf-8)
214+
*/
215+
private function matches(AcceptHeaderItem $rangeItem, AcceptHeaderItem $queryItem): bool
216+
{
217+
$rangeValue = strtolower($rangeItem->getValue());
218+
$queryValue = strtolower($queryItem->getValue());
219+
220+
// Handle universal wildcard ranges
221+
if ('*' === $rangeValue || '*/*' === $rangeValue) {
222+
return $this->rangeParametersMatch($rangeItem, $queryItem);
223+
}
224+
225+
// Queries for '*' only match wildcard ranges (handled above)
226+
if ('*' === $queryValue) {
227+
return false;
228+
}
229+
230+
// Ensure media vs. non-media consistency
231+
$isQueryMedia = str_contains($queryValue, '/');
232+
$isRangeMedia = str_contains($rangeValue, '/');
233+
if ($isQueryMedia !== $isRangeMedia) {
234+
return false;
235+
}
236+
237+
// Non-media: exact match only (wildcards handled above)
238+
if (!$isQueryMedia) {
239+
return $rangeValue === $queryValue && $this->rangeParametersMatch($rangeItem, $queryItem);
240+
}
241+
242+
// Media type: type/subtype with wildcards
243+
[$queryType, $querySubtype] = explode('/', $queryValue, 2);
244+
[$rangeType, $rangeSubtype] = explode('/', $rangeValue) + [1 => '*'];
245+
246+
if ('*' !== $rangeType && $rangeType !== $queryType) {
247+
return false;
248+
}
249+
250+
if ('*' !== $rangeSubtype && $rangeSubtype !== $querySubtype) {
251+
return false;
252+
}
253+
254+
// Parameters must match
255+
return $this->rangeParametersMatch($rangeItem, $queryItem);
256+
}
257+
258+
/**
259+
* Checks if the parameters of a range item are satisfied by the query item.
260+
*
261+
* Parameters are case-insensitive; range params must be a subset of query params.
262+
*/
263+
private function rangeParametersMatch(AcceptHeaderItem $rangeItem, AcceptHeaderItem $queryItem): bool
264+
{
265+
$queryAttributes = $this->getMediaParams($queryItem);
266+
$rangeAttributes = $this->getMediaParams($rangeItem);
267+
268+
foreach ($rangeAttributes as $name => $rangeValue) {
269+
if (!\array_key_exists($name, $queryAttributes)) {
270+
return false; // Missing required param
271+
}
272+
273+
$queryValue = $queryAttributes[$name];
274+
275+
if (null === $rangeValue) {
276+
return null === $queryValue; // Both flags or neither
277+
}
278+
279+
if (null === $queryValue || strtolower((string) $queryValue) !== strtolower((string) $rangeValue)) {
280+
return false;
281+
}
282+
}
283+
284+
return true;
285+
}
286+
287+
/**
288+
* Calculates a specificity score for sorting: media precision + param count.
289+
*/
290+
private function getSpecificity(AcceptHeaderItem $item, AcceptHeaderItem $queryItem): int
291+
{
292+
$rangeValue = strtolower($item->getValue());
293+
$queryValue = strtolower($queryItem->getValue());
294+
295+
$paramCount = \count($this->getMediaParams($item));
296+
297+
$isQueryMedia = str_contains($queryValue, '/');
298+
$isRangeMedia = str_contains($rangeValue, '/');
299+
300+
if (!$isQueryMedia && !$isRangeMedia) {
301+
return ('*' !== $rangeValue ? 2000 : 1000) + $paramCount;
302+
}
303+
304+
[$rangeType, $rangeSubtype] = explode('/', $rangeValue) + [1 => '*'];
305+
306+
$specificity = match (true) {
307+
'*' !== $rangeSubtype => 3000, // Exact subtype (text/plain)
308+
'*' !== $rangeType => 2000, // Type wildcard (text/*)
309+
default => 1000, // Full wildcard (*/* or *)
310+
};
311+
312+
return $specificity + $paramCount;
313+
}
314+
315+
/**
316+
* Returns normalized attributes: keys lowercased, excluding 'q'.
317+
*/
318+
private function getMediaParams(AcceptHeaderItem $item): array
319+
{
320+
$attributes = array_change_key_case($item->getAttributes(), \CASE_LOWER);
321+
unset($attributes['q']);
322+
323+
return $attributes;
324+
}
150325
}

‎src/Symfony/Component/HttpFoundation/Tests/AcceptHeaderTest.php‎

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ public static function provideSortingData()
100100
'quality has priority' => ['*;q=0.3,ISO-8859-1,utf-8;q=0.7', ['ISO-8859-1', 'utf-8', '*']],
101101
'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', '*']],
102102
'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', '*']],
103+
'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/*']],
104+
'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/*']],
103105
];
104106
}
105107

@@ -109,7 +111,7 @@ public static function provideSortingData()
109111
public function testDefaultValue($acceptHeader, $value, $expectedQuality)
110112
{
111113
$header = AcceptHeader::fromString($acceptHeader);
112-
$this->assertSame($expectedQuality, $header->get($value)->getQuality());
114+
$this->assertSame($expectedQuality, $header->get($value)?->getQuality());
113115
}
114116

115117
public static function provideDefaultValueData()
@@ -128,5 +130,50 @@ public static function provideDefaultValueData()
128130
yield ['*;q=0.3, ISO-8859-1;q=0.7, utf-8;q=0.7', '*', 0.3];
129131
yield ['*;q=0.3, ISO-8859-1;q=0.7, utf-8;q=0.7', 'utf-8', 0.7];
130132
yield ['*;q=0.3, ISO-8859-1;q=0.7, utf-8;q=0.7', 'SHIFT_JIS', 0.3];
133+
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];
134+
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];
135+
136+
// Edge cases for case-insensitivity
137+
yield 'case-insensitive param names' => ['text/plain;format=flowed;q=0.8, text/plain;Format=fixed', 'text/plain;format=fixed', 1.0];
138+
yield 'case-insensitive charset' => ['text/plain;Charset=utf-8', 'text/plain;charset=utf-8', 1.0];
139+
140+
// Quoted values and specials
141+
yield 'quoted value with space' => ['text/plain;param="value with space"', 'text/plain;param="value with space"', 1.0];
142+
yield 'quoted value with backslash' => ['text/plain;param="value\\with\\backslash"', 'text/plain;param="value\\with\\backslash"', 1.0];
143+
yield 'mismatched quoted' => ['text/plain;param="value with space"', 'text/plain;param=value with space', 1.0];
144+
145+
// Flag params or empty
146+
yield 'flag param' => ['text/plain;flowed;q=0.9', 'text/plain;flowed', 0.9];
147+
yield 'empty param value' => ['text/plain;param=', 'text/plain;param=""', 1.0];
148+
yield 'missing required flag' => ['text/plain;flowed', 'text/plain', null];
149+
150+
// Extra params in query
151+
yield 'extra param in query' => ['text/plain;format=flowed', 'text/plain;format=flowed;charset=utf-8', 1.0];
152+
yield 'missing required param in query' => ['text/plain;format=flowed', 'text/plain;charset=utf-8', null];
153+
yield 'wildcard with param' => ['text/*;format=flowed', 'text/plain;format=flowed', 1.0];
154+
yield 'wildcard missing param' => ['text/*;format=flowed', 'text/plain', null];
155+
156+
// Wildcards and specificity
157+
yield 'specificity priority' => ['*/*;q=0.1, text/*;format=flowed;q=0.5, text/plain;q=0.8', 'text/plain;format=flowed', 0.8];
158+
yield 'wildcard with param match' => ['*/*;param=test', 'text/plain;param=test', 1.0];
159+
yield 'wildcard with param no match' => ['*/*;param=test', 'text/plain', null];
160+
161+
// Non-media types
162+
yield 'charset wildcard' => ['utf-8;q=0.9, *;q=0.5', 'iso-8859-1', 0.5];
163+
yield 'language star' => ['*;q=0.5', 'en-US', 0.5];
164+
yield 'non-media */*' => ['*/*;q=0.5', 'utf-8', 0.5];
165+
166+
// Ties and duplicates
167+
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];
168+
yield 'param count tie' => ['text/plain;q=0.5, text/plain;format=flowed;q=0.5', 'text/plain;format=flowed;extra=foo', 0.5];
169+
170+
// Invalid/malformed
171+
yield 'non-media invalid' => ['text', 'text', 1.0];
172+
yield 'invalid subtype' => ['text/', 'text/plain', null];
173+
yield 'empty header' => ['', 'text/plain', null];
174+
175+
// Mixed case types
176+
yield 'mixed case type' => ['Text/Plain;Format=flowed', 'text/plain;format=flowed', 1.0];
177+
yield 'mixed case charset' => ['UTF-8;q=0.9', 'utf-8', 0.9];
131178
}
132179
}

0 commit comments

Comments
 (0)