Skip to content

Commit 50fd750

Browse files
wleeafwleeaf
authored
fix: do not let wildcards select values excluded with q=0 in Negotiate (#10661)
1 parent 8ab9933 commit 50fd750

3 files changed

Lines changed: 248 additions & 6 deletions

File tree

‎system/HTTP/Negotiate.php‎

Lines changed: 128 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -184,13 +184,24 @@ protected function getBestMatch(
184184
}
185185

186186
// if acceptable value is "anything", return the first available
187+
// that the header does not exclude
187188
if ($accept['value'] === '*' || $accept['value'] === '*/*') {
188-
return $supported[0];
189+
foreach ($supported as $available) {
190+
if (! $this->isExcluded($acceptable, $available, $enforceTypes, $matchLocales)) {
191+
return $available;
192+
}
193+
}
194+
195+
continue;
189196
}
190197

191-
// If an acceptable value is supported, return it
198+
// If an acceptable value is supported, return it, unless the header
199+
// excludes it with a more specific value, such as "text/html;q=0"
200+
// for "text/*"
192201
foreach ($supported as $available) {
193-
if ($this->match($accept, $available, $enforceTypes, $matchLocales)) {
202+
if ($this->match($accept, $available, $enforceTypes, $matchLocales)
203+
&& ! $this->isExcluded($acceptable, $available, $enforceTypes, $matchLocales)
204+
) {
194205
return $available;
195206
}
196207
}
@@ -233,8 +244,15 @@ protected function getBestLocaleMatch(array $supportedLocales, ?string $header):
233244
}
234245

235246
// if acceptable value is "anything", return the first available
247+
// that the header does not exclude
236248
if ($accept['value'] === '*') {
237-
return $supportedLocales[0];
249+
foreach ($supportedLocales as $locale) {
250+
if (! $this->isExcluded($acceptable, $locale, false, true)) {
251+
return $locale;
252+
}
253+
}
254+
255+
continue;
238256
}
239257

240258
// look for exact match
@@ -248,13 +266,17 @@ protected function getBestLocaleMatch(array $supportedLocales, ?string $header):
248266

249267
foreach ($fallbackLocales as $fallbackLocale) {
250268
// look for exact match
251-
if (in_array($fallbackLocale, $supportedLocales, true)) {
269+
if (in_array($fallbackLocale, $supportedLocales, true)
270+
&& ! $this->isExcluded($acceptable, $fallbackLocale, false, true)
271+
) {
252272
return $fallbackLocale;
253273
}
254274

255275
// look for regional locale match
256276
foreach ($supportedLocales as $locale) {
257-
if (str_starts_with($locale, $fallbackLocale . '-')) {
277+
if (str_starts_with($locale, $fallbackLocale . '-')
278+
&& ! $this->isExcluded($acceptable, $locale, false, true)
279+
) {
258280
return $locale;
259281
}
260282
}
@@ -263,6 +285,106 @@ protected function getBestLocaleMatch(array $supportedLocales, ?string $header):
263285
return $supportedLocales[0];
264286
}
265287

288+
/**
289+
* Checks whether the header excludes a supported value, that is, whether
290+
* the most specific value in the header that matches it has q=0.
291+
*
292+
* @param list<array{value: string, q: float, params: array<string, string>}> $acceptable
293+
* @param bool $enforceTypes If TRUE, the values are media types.
294+
* @param bool $matchLocales If TRUE, the values are language tags.
295+
*/
296+
private function isExcluded(array $acceptable, string $supported, bool $enforceTypes, bool $matchLocales): bool
297+
{
298+
$supported = $this->parseHeader($supported)[0];
299+
$specificity = -1;
300+
$excluded = false;
301+
302+
// The header is sorted by quality, so of two equally specific values,
303+
// the one with the higher quality is used.
304+
foreach ($acceptable as $accept) {
305+
$acceptSpecificity = $this->getMatchSpecificity($accept, $supported, $enforceTypes, $matchLocales);
306+
307+
if ($acceptSpecificity > $specificity) {
308+
$specificity = $acceptSpecificity;
309+
$excluded = $accept['q'] === 0.0;
310+
}
311+
}
312+
313+
return $excluded;
314+
}
315+
316+
/**
317+
* Returns how specifically a header value matches a supported value, or -1
318+
* if it does not match it. A wildcard is the least specific.
319+
*
320+
* A media range matches the media types of its type and subtype that have
321+
* its parameters. "text/html" is more specific than "text/*", which is more
322+
* specific than a full wildcard, and a range is more specific than one of
323+
* the same kind with fewer parameters, so "text/html;level=1" is more
324+
* specific than "text/html", and "text/*;charset=utf-8" than "text/*"
325+
* (RFC 9110, Section 12.5.1). Types, subtypes, parameter names and charset
326+
* values are compared ignoring case (RFC 9110, Section 8.3).
327+
* A language range matches the tags that equal it or start with it followed
328+
* by "-", ignoring case, and is more specific with more subtags
329+
* (RFC 4647, Section 3.3.1). Other values must be equal, ignoring case.
330+
*
331+
* @param array{value: string, q: float, params: array<string, string>} $accept
332+
* @param array{value: string, q: float, params: array<string, string>} $supported
333+
*/
334+
private function getMatchSpecificity(array $accept, array $supported, bool $enforceTypes, bool $matchLocales): int
335+
{
336+
if ($enforceTypes) {
337+
$range = strtolower($accept['value']);
338+
[$aType, $aSubType] = explode('/', $range, 2) + ['', ''];
339+
[$sType, $sSubType] = explode('/', strtolower($supported['value']), 2) + ['', ''];
340+
341+
if ($range === '*' || $range === '*/*') {
342+
$level = 0;
343+
} elseif ($aType === $sType && $aSubType === '*') {
344+
$level = 1;
345+
} elseif ($aType === $sType && $aSubType === $sSubType) {
346+
$level = 2;
347+
} else {
348+
return -1;
349+
}
350+
351+
$aParams = array_change_key_case($accept['params']);
352+
$sParams = array_change_key_case($supported['params']);
353+
354+
if (isset($aParams['charset'], $sParams['charset'])) {
355+
$aParams['charset'] = strtolower($aParams['charset']);
356+
$sParams['charset'] = strtolower($sParams['charset']);
357+
}
358+
359+
foreach ($aParams as $label => $value) {
360+
if (($sParams[$label] ?? null) !== $value) {
361+
return -1;
362+
}
363+
}
364+
365+
// A range has at most as many parameters as the value it matches,
366+
// so it is more specific than any range of a lower level.
367+
return $level * (count($sParams) + 1) + count($aParams);
368+
}
369+
370+
if ($accept['value'] === '*' || $accept['value'] === '*/*') {
371+
return 0;
372+
}
373+
374+
if ($matchLocales) {
375+
$aTag = strtolower($accept['value']);
376+
$sTag = strtolower($supported['value']);
377+
378+
if ($aTag !== $sTag && ! str_starts_with($sTag, $aTag . '-')) {
379+
return -1;
380+
}
381+
382+
return substr_count($aTag, '-') + 1;
383+
}
384+
385+
return strcasecmp($accept['value'], $supported['value']) === 0 ? 1 : -1;
386+
}
387+
266388
/**
267389
* Parses an Accept* header into it's multiple values.
268390
*

‎tests/system/HTTP/NegotiateTest.php‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,125 @@ public function testBestMatchLowQuality(): void
189189
$this->assertSame('apple/mac', $this->negotiate->media(['popcorn/cheddar', 'apple/mac'], false));
190190
}
191191

192+
public function testWildcardsSkipExcludedValues(): void
193+
{
194+
$this->request->setHeader('Accept', 'application/json;q=0, */*');
195+
$this->assertSame('text/html', $this->negotiate->media(['application/json', 'text/html'], true));
196+
197+
$this->request->setHeader('Accept', 'text/*, text/html;q=0');
198+
$this->assertSame('text/plain', $this->negotiate->media(['text/html', 'text/plain'], true));
199+
200+
$this->request->setHeader('Accept', '*/*, text/html;q=0');
201+
$this->assertSame('', $this->negotiate->media(['text/html'], true));
202+
203+
$this->request->setHeader('Accept-Charset', 'utf-8;q=0, *');
204+
$this->assertSame('iso-8859-1', $this->negotiate->charset(['utf-8', 'iso-8859-1']));
205+
206+
$this->request->setHeader('Accept-Encoding', 'gzip;q=0, *');
207+
$this->assertSame('br', $this->negotiate->encoding(['gzip', 'br']));
208+
209+
$this->request->setHeader('Accept-Language', 'en;q=0, *');
210+
$this->assertSame('fr', $this->negotiate->language(['en', 'fr']));
211+
212+
config(Feature::class)->strictLocaleNegotiation = true;
213+
214+
$this->assertSame('de', $this->negotiate->language(['en', 'de']));
215+
}
216+
217+
public function testMediaRangesExcludeMatchingValues(): void
218+
{
219+
$this->request->setHeader('Accept', 'text/*;q=0, */*');
220+
$this->assertSame('application/json', $this->negotiate->media(['text/html', 'application/json'], true));
221+
222+
$this->request->setHeader('Accept', 'text/html;q=0, */*');
223+
$this->assertSame('text/plain', $this->negotiate->media(['text/html;charset=utf-8', 'text/plain'], true));
224+
225+
$this->request->setHeader('Accept', 'text/html;q=0, text/*');
226+
$this->assertSame('text/css', $this->negotiate->media(['text/html;charset=utf-8', 'text/css'], true));
227+
228+
// a range only excludes the values that have its parameters
229+
$this->request->setHeader('Accept', 'text/html;level=1;q=0, */*');
230+
$this->assertSame('text/html', $this->negotiate->media(['text/html;level=1', 'text/html'], true));
231+
}
232+
233+
public function testRangesExcludeValuesIgnoringCase(): void
234+
{
235+
$this->request->setHeader('Accept', 'text/*;q=0, TEXT/HTML;q=0.5, */*;q=0.1');
236+
$this->assertSame('text/html', $this->negotiate->media(['text/html'], true));
237+
238+
$this->request->setHeader('Accept', 'TEXT/*;q=0, */*');
239+
$this->assertSame('application/json', $this->negotiate->media(['text/html', 'application/json'], true));
240+
241+
$this->request->setHeader('Accept', 'text/html;CHARSET="UTF-8";q=0, */*');
242+
$this->assertSame('text/plain', $this->negotiate->media(['text/html;charset=utf-8', 'text/plain'], true));
243+
244+
$this->request->setHeader('Accept-Charset', 'UTF-8;q=0, *');
245+
$this->assertSame('iso-8859-1', $this->negotiate->charset(['utf-8', 'iso-8859-1']));
246+
247+
$this->request->setHeader('Accept-Encoding', 'GZIP;q=0, *');
248+
$this->assertSame('br', $this->negotiate->encoding(['gzip', 'br']));
249+
}
250+
251+
public function testMediaRangeParametersAddSpecificity(): void
252+
{
253+
$this->request->setHeader('Accept', 'text/*;charset=utf-8;q=0, text/*');
254+
$this->assertSame(
255+
'text/plain;charset=iso-8859-1',
256+
$this->negotiate->media(['text/html;charset=utf-8', 'text/plain;charset=iso-8859-1'], true),
257+
);
258+
259+
$this->request->setHeader('Accept', '*/*;charset=utf-8;q=0, */*');
260+
$this->assertSame('application/json', $this->negotiate->media(['text/html;charset=utf-8', 'application/json'], true));
261+
262+
// a type/subtype range is more specific than a type/* range with more parameters
263+
$this->request->setHeader('Accept', 'text/html;q=0, text/*;charset=utf-8;level=1');
264+
$this->assertSame(
265+
'text/plain;charset=utf-8;level=1',
266+
$this->negotiate->media(['text/html;charset=utf-8;level=1', 'text/plain;charset=utf-8;level=1'], true),
267+
);
268+
}
269+
270+
public function testMoreSpecificRangesTakePrecedence(): void
271+
{
272+
$this->request->setHeader('Accept', 'text/html, text/*;q=0');
273+
$this->assertSame('text/html', $this->negotiate->media(['text/plain', 'text/html'], true));
274+
$this->assertSame('', $this->negotiate->media(['text/plain'], true));
275+
276+
$this->request->setHeader('Accept', 'text/html;q=0, text/html;level=1;q=0.5, */*');
277+
$this->assertSame('text/html;level=1', $this->negotiate->media(['text/html', 'text/html;level=1'], true));
278+
279+
$this->request->setHeader('Accept-Language', 'en;q=0, en-GB;q=0.5, *');
280+
$this->assertSame('en-GB', $this->negotiate->language(['en-US', 'en-GB', 'fr']));
281+
282+
config(Feature::class)->strictLocaleNegotiation = true;
283+
284+
$this->assertSame('en-GB', $this->negotiate->language(['en-US', 'en-GB']));
285+
}
286+
287+
public function testLanguageRangesExcludeRegionalLocales(): void
288+
{
289+
$this->request->setHeader('Accept-Language', 'en;q=0, *');
290+
$this->assertSame('fr', $this->negotiate->language(['en-US', 'fr']));
291+
292+
// language ranges are case-insensitive and end on a subtag boundary
293+
$this->request->setHeader('Accept-Language', 'zh;q=0, *');
294+
$this->assertSame('zha', $this->negotiate->language(['ZH-Hant', 'zha']));
295+
296+
$this->request->setHeader('Accept-Language', 'en-US, en-GB;q=0');
297+
$this->assertSame('en-AU', $this->negotiate->language(['en-GB', 'en-AU']));
298+
299+
config(Feature::class)->strictLocaleNegotiation = true;
300+
301+
$this->request->setHeader('Accept-Language', 'en;q=0, *');
302+
$this->assertSame('fr', $this->negotiate->language(['en-US', 'fr']));
303+
304+
$this->request->setHeader('Accept-Language', 'zh;q=0, *');
305+
$this->assertSame('zha', $this->negotiate->language(['ZH-Hant', 'zha']));
306+
307+
$this->request->setHeader('Accept-Language', 'en-US, en-GB;q=0');
308+
$this->assertSame('en-AU', $this->negotiate->language(['en-GB', 'en-AU']));
309+
}
310+
192311
public function testBestMatchOnlyLowQuality(): void
193312
{
194313
$this->request->setHeader('Accept', 'popcorn/cheddar;q=0');

‎user_guide_src/source/changelogs/v4.7.6.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ Bugs Fixed
4747
- **HTTP:** Fixed a bug where ``IncomingRequest`` did not read the request body when PHP's ``post_max_size`` was set to ``0`` or ``-1`` (unlimited).
4848
- **HTTP:** Fixed a bug where ``IncomingRequest::isSecure()`` did not recognize ``https`` in the ``X-Forwarded-Proto`` header from a trusted proxy when the value contained uppercase letters or surrounding whitespace, or was represented as a single-element array.
4949
- **HTTP:** Fixed a bug where ``Negotiate::media()`` threw an ``ErrorException`` when the ``Accept`` header contained a value without a subtype, such as ``json`` or an empty list element. Such values are now ignored.
50+
- **HTTP:** Fixed a bug where ``Negotiate`` could select a value that the request header excluded with ``q=0`` through a wildcard or a broader range. A ``q=0`` range now excludes every value it matches, such as ``text/html`` and ``text/html;charset=utf-8`` for ``text/*;q=0``, or ``en-US`` for ``en;q=0``, unless a more specific range in the header accepts the value. For example, ``Accept-Encoding: gzip;q=0, *`` no longer selects ``gzip``.
5051
- **HTTP:** Fixed stale ``$_REQUEST`` values after ``SiteURIFactory`` normalizes GET parameters, so ``Validation::withRequest()`` reads the normalized values while unrelated REQUEST entries are preserved.
5152
- **Security:** Fixed a bug where a valid JSON request body that is not an object (for example a JSON array) was corrupted after CSRF verification when the token was sent via the ``X-CSRF-TOKEN`` header.
5253

0 commit comments

Comments
 (0)