Skip to content

Commit bebe53d

Browse files
yoeunesyoeunes
committed
[HttpFoundation] Fix AcceptHeader overwrites items with different parameters
1 parent d3bd736 commit bebe53d

File tree

2 files changed

+244
-6
lines changed

2 files changed

+244
-6
lines changed

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

Lines changed: 177 additions & 5 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

@@ -69,19 +69,36 @@ public function __toString(): string
6969
}
7070

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

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

Lines changed: 67 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,69 @@ 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];
178+
}
179+
180+
public function testPerformanceWithLargeHeader()
181+
{
182+
$largeHeader = [];
183+
for ($i = 0; $i < 100; ++$i) {
184+
$largeHeader[] = "text/plain;param{$i}=value;q=0.5";
185+
}
186+
$headerString = implode(', ', $largeHeader);
187+
$header = AcceptHeader::fromString($headerString);
188+
189+
$start = microtime(true);
190+
for ($j = 0; $j < 1000; ++$j) {
191+
$header->get('text/plain;param50=value');
192+
}
193+
$duration = microtime(true) - $start;
194+
195+
// No assert, just for manual check; should be < 1s
196+
$this->assertLessThan(1.0, $duration, 'Performance test for large header');
131197
}
132198
}

0 commit comments

Comments
 (0)