Skip to content

Commit c99d747

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

File tree

2 files changed

+245
-4
lines changed

2 files changed

+245
-4
lines changed

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

Lines changed: 177 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,39 @@ public function __toString(): string
7373
*/
7474
public function has(string $value): bool
7575
{
76-
return isset($this->items[$value]);
76+
return isset($this->items[$value]) || null !== $this->get($value);
7777
}
7878

7979
/**
8080
* Returns given value's item, if exists.
8181
*/
8282
public function get(string $value): ?AcceptHeaderItem
8383
{
84-
return $this->items[$value] ?? $this->items[explode('/', $value)[0].'/*'] ?? $this->items['*/*'] ?? $this->items['*'] ?? null;
84+
$queryItem = AcceptHeaderItem::fromString($value.';q=1');
85+
$baseValue = $queryItem->getValue();
86+
$queryAttributes = $queryItem->getAttributes();
87+
$parametersString = $this->buildCanonicalParams($queryAttributes);
88+
$canonicalKey = $baseValue.($parametersString ? ';'.$parametersString : '');
89+
90+
if (isset($this->items[$canonicalKey])) {
91+
return $this->items[$canonicalKey];
92+
}
93+
94+
// Find all matching candidates
95+
$candidates = [];
96+
foreach ($this->items as $item) {
97+
if ($this->matches($item, $queryItem)) {
98+
$candidates[] = $item;
99+
}
100+
}
101+
102+
if (!$candidates) {
103+
return null;
104+
}
105+
106+
$this->sortCandidates($candidates, $queryItem);
107+
108+
return $candidates[0];
85109
}
86110

87111
/**
@@ -91,7 +115,16 @@ public function get(string $value): ?AcceptHeaderItem
91115
*/
92116
public function add(AcceptHeaderItem $item): static
93117
{
94-
$this->items[$item->getValue()] = $item;
118+
$baseValue = $item->getValue();
119+
$itemAttributes = $item->getAttributes();
120+
121+
// Canonicalize parameters: sort by name, format as 'name=value' or 'name'
122+
$parametersString = $this->buildCanonicalParams($itemAttributes);
123+
$canonicalKey = $baseValue.($parametersString ? ';'.$parametersString : '');
124+
125+
// Add or overwrite (though duplicates unlikely after canonicalization)
126+
$this->items[$canonicalKey] = $item;
127+
95128
$this->sorted = false;
96129

97130
return $this;
@@ -147,4 +180,145 @@ private function sort(): void
147180
$this->sorted = true;
148181
}
149182
}
183+
184+
/**
185+
* Sorts candidates by specificity, quality, parameter count, and original order.
186+
*
187+
* @param AcceptHeaderItem[] $candidates
188+
*/
189+
private function sortCandidates(array &$candidates, AcceptHeaderItem $queryItem): void
190+
{
191+
usort($candidates, function (AcceptHeaderItem $a, AcceptHeaderItem $b) use ($queryItem) {
192+
$specificityA = $this->getSpecificity($a, $queryItem);
193+
$specificityB = $this->getSpecificity($b, $queryItem);
194+
if ($specificityA !== $specificityB) {
195+
return $specificityB <=> $specificityA;
196+
}
197+
if ($a->getQuality() !== $b->getQuality()) {
198+
return $b->getQuality() <=> $a->getQuality();
199+
}
200+
$parameterCountA = \count($this->getMediaParams($a));
201+
$parameterCountB = \count($this->getMediaParams($b));
202+
if ($parameterCountA !== $parameterCountB) {
203+
return $parameterCountB <=> $parameterCountA;
204+
}
205+
206+
return $a->getIndex() <=> $b->getIndex();
207+
});
208+
}
209+
210+
/**
211+
* Builds a canonical string representation of attributes.
212+
*
213+
* @param array<string, mixed> $attributes
214+
*/
215+
private function buildCanonicalParams(array $attributes): string
216+
{
217+
$normalizedAttributes = [];
218+
foreach ($attributes as $name => $value) {
219+
$lowerName = strtolower($name);
220+
if ('q' === $lowerName) {
221+
continue;
222+
}
223+
$normalizedAttributes[$lowerName] = $value;
224+
}
225+
ksort($normalizedAttributes);
226+
$parameterParts = [];
227+
foreach ($normalizedAttributes as $name => $value) {
228+
if (null === $value) {
229+
$parameterParts[] = $name;
230+
} else {
231+
$quotedValue = \is_string($value) && str_contains($value, ' ') ? '"'.addslashes($value).'"' : $value;
232+
$parameterParts[] = $name.'='.$quotedValue;
233+
}
234+
}
235+
236+
return implode(';', $parameterParts);
237+
}
238+
239+
/**
240+
* Checks if a given header item (range) matches a queried item (value).
241+
*
242+
* @param AcceptHeaderItem $item The item from the Accept header (e.g., text/*;format=flowed)
243+
* @param AcceptHeaderItem $queryItem The item being queried (e.g., text/plain;format=flowed;charset=utf-8)
244+
*/
245+
private function matches(AcceptHeaderItem $item, AcceptHeaderItem $queryItem): bool
246+
{
247+
$requestedValue = $queryItem->getValue();
248+
if (!str_contains($requestedValue, '/')) {
249+
// Charset/Language: exact or *
250+
$rangeValue = $item->getValue();
251+
252+
return strtolower($rangeValue) === strtolower($requestedValue) || '*' === strtolower($rangeValue) || '*/*' === strtolower($rangeValue);
253+
}
254+
// Media type
255+
[$requestedType, $requestedSubtype] = explode('/', $requestedValue, 2);
256+
if (!$requestedSubtype) {
257+
return false;
258+
}
259+
$rangeValue = $item->getValue();
260+
[$rangeType, $rangeSubtype] = explode('/', $rangeValue, 2) + ['', '*'];
261+
if ('*' !== $rangeType && strtolower($rangeType) !== strtolower($requestedType)) {
262+
return false;
263+
}
264+
if ('*' !== $rangeSubtype && strtolower($rangeSubtype) !== strtolower($requestedSubtype)) {
265+
return false;
266+
}
267+
// Check params: range params must be matched by value params
268+
$requestedAttributes = $queryItem->getAttributes();
269+
$normalizedRequestedAttrs = [];
270+
foreach ($requestedAttributes as $name => $val) {
271+
$normalizedRequestedAttrs[strtolower($name)] = $val;
272+
}
273+
$itemAttributes = $item->getAttributes();
274+
foreach ($itemAttributes as $name => $val) {
275+
$lowerName = strtolower($name);
276+
if ('q' === $lowerName) {
277+
continue;
278+
}
279+
if (!\array_key_exists($lowerName, $normalizedRequestedAttrs) || $normalizedRequestedAttrs[$lowerName] !== $val) {
280+
return false;
281+
}
282+
}
283+
284+
return true;
285+
}
286+
287+
/**
288+
* Calculates a specificity score for a given item against a query.
289+
*
290+
* @param AcceptHeaderItem $item The item from the Accept header
291+
* @param AcceptHeaderItem $queryItem The item being queried
292+
*/
293+
private function getSpecificity(AcceptHeaderItem $item, AcceptHeaderItem $queryItem): int
294+
{
295+
$requestedValue = $queryItem->getValue();
296+
if (!str_contains($requestedValue, '/')) {
297+
return strtolower($item->getValue()) === strtolower($requestedValue) ? 2 : 1;
298+
}
299+
[$rangeType, $rangeSubtype] = explode('/', strtolower($item->getValue()), 2) + ['', '*'];
300+
$specificity = 0;
301+
if ('*' !== $rangeSubtype) {
302+
$specificity = 3;
303+
} elseif ('*' !== $rangeType) {
304+
$specificity = 2;
305+
} else {
306+
$specificity = 1;
307+
}
308+
309+
// Add parameter count to specificity (shifted to allow for type/subtype precedence)
310+
$paramCount = \count($this->getMediaParams($item));
311+
312+
return ($specificity * 1000) + $paramCount;
313+
}
314+
315+
/**
316+
* Returns all attributes of an item, excluding the quality ('q') parameter.
317+
*
318+
* @return array<string, mixed>
319+
*/
320+
private function getMediaParams(AcceptHeaderItem $item): array
321+
{
322+
return array_filter($item->getAttributes(), fn ($k) => 'q' !== strtolower($k), \ARRAY_FILTER_USE_KEY);
323+
}
150324
}

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

Lines changed: 68 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,70 @@ 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+
yield 'case-sensitive value no match' => ['text/plain;charset=UTF-8', 'text/plain;charset=utf-8', null];
140+
141+
// Quoted values and specials
142+
yield 'quoted value with space' => ['text/plain;param="value with space"', 'text/plain;param="value with space"', 1.0];
143+
yield 'quoted value with backslash' => ['text/plain;param="value\\with\\backslash"', 'text/plain;param="value\\with\\backslash"', 1.0];
144+
yield 'mismatched quoted' => ['text/plain;param="value with space"', 'text/plain;param=value with space', 1.0];
145+
146+
// Flag params or empty
147+
yield 'flag param' => ['text/plain;flowed;q=0.9', 'text/plain;flowed', 0.9];
148+
yield 'empty param value' => ['text/plain;param=', 'text/plain;param=""', 1.0];
149+
yield 'missing required flag' => ['text/plain;flowed', 'text/plain', null];
150+
151+
// Extra params in query
152+
yield 'extra param in query' => ['text/plain;format=flowed', 'text/plain;format=flowed;charset=utf-8', 1.0];
153+
yield 'missing required param in query' => ['text/plain;format=flowed', 'text/plain;charset=utf-8', null];
154+
yield 'wildcard with param' => ['text/*;format=flowed', 'text/plain;format=flowed', 1.0];
155+
yield 'wildcard missing param' => ['text/*;format=flowed', 'text/plain', null];
156+
157+
// Wildcards and specificity
158+
yield 'specificity priority' => ['*/*;q=0.1, text/*;format=flowed;q=0.5, text/plain;q=0.8', 'text/plain;format=flowed', 0.8];
159+
yield 'wildcard with param match' => ['*/*;param=test', 'text/plain;param=test', 1.0];
160+
yield 'wildcard with param no match' => ['*/*;param=test', 'text/plain', null];
161+
162+
// Non-media types
163+
yield 'charset wildcard' => ['utf-8;q=0.9, *;q=0.5', 'iso-8859-1', 0.5];
164+
yield 'language star' => ['*;q=0.5', 'en-US', 0.5];
165+
yield 'non-media */*' => ['*/*;q=0.5', 'utf-8', 0.5];
166+
167+
// Ties and duplicates
168+
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];
169+
yield 'param count tie' => ['text/plain;q=0.5, text/plain;format=flowed;q=0.5', 'text/plain;format=flowed;extra=foo', 0.5];
170+
171+
// Invalid/malformed
172+
yield 'non-media invalid' => ['text', 'text', 1.0];
173+
yield 'invalid subtype' => ['text/', 'text/plain', null];
174+
yield 'empty header' => ['', 'text/plain', null];
175+
176+
// Mixed case types
177+
yield 'mixed case type' => ['Text/Plain;Format=flowed', 'text/plain;format=flowed', 1.0];
178+
yield 'mixed case charset' => ['UTF-8;q=0.9', 'utf-8', 0.9];
179+
}
180+
181+
public function testPerformanceWithLargeHeader()
182+
{
183+
$largeHeader = [];
184+
for ($i = 0; $i < 100; ++$i) {
185+
$largeHeader[] = "text/plain;param{$i}=value;q=0.5";
186+
}
187+
$headerString = implode(', ', $largeHeader);
188+
$header = AcceptHeader::fromString($headerString);
189+
190+
$start = microtime(true);
191+
for ($j = 0; $j < 1000; ++$j) {
192+
$header->get('text/plain;param50=value');
193+
}
194+
$duration = microtime(true) - $start;
195+
196+
// No assert, just for manual check; should be < 1s
197+
$this->assertLessThan(1.0, $duration, 'Performance test for large header');
131198
}
132199
}

0 commit comments

Comments
 (0)