Skip to content

Commit 9b1fb5c

Browse files
yoeunesyoeunes
committed
[HttpFoundation] Fix AcceptHeader overwrites items with different parameters
1 parent 3f5c522 commit 9b1fb5c

File tree

3 files changed

+489
-5
lines changed

3 files changed

+489
-5
lines changed

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

Lines changed: 180 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,32 @@ 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+
$candidates = array_filter($this->items, fn (AcceptHeaderItem $item) => $this->matches($item, $queryItem));
95+
if ([] === $candidates) {
96+
return null;
97+
}
98+
99+
$this->sortCandidates($candidates, $queryItem);
100+
101+
return reset($candidates) ?: null;
85102
}
86103

87104
/**
@@ -91,7 +108,7 @@ public function get(string $value): ?AcceptHeaderItem
91108
*/
92109
public function add(AcceptHeaderItem $item): static
93110
{
94-
$this->items[$item->getValue()] = $item;
111+
$this->items[$this->getCanonicalKey($item)] = $item;
95112
$this->sorted = false;
96113

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

‎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)