Skip to content

Commit 78e6fb1

Browse files
wleeafwleeaf
authored
fix: do not let wildcards select values excluded with q=0 in Negotiate
A wildcard or a broader range could select a supported value even when the header excluded that value with q=0. For example, `Accept-Encoding: gzip;q=0, *` selected gzip, `Accept: text/*;q=0, */*` selected text/html, `Accept: text/html;q=0, */*` selected text/html;charset=utf-8, and `Accept-Language: en;q=0, *` selected en-US, with and without strict locale negotiation. A supported value is now excluded when the most specific value in the header that matches it has q=0 (RFC 9110, Section 12.5.1), so `text/html, text/*;q=0` still selects text/html. A media range matches the media types of its type and subtype that have its parameters, and a language range matches the tags that equal it or start with it followed by "-", ignoring case (RFC 4647, Section 3.3.1). Wildcards, broader matches and the strict locale fallbacks skip excluded values. Without a match, the fallback to the first supported value when `$strictMatch` is false is unchanged.
1 parent 345262a commit 78e6fb1

3 files changed

Lines changed: 190 additions & 6 deletions

File tree

‎system/HTTP/Negotiate.php‎

Lines changed: 107 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,85 @@ 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 has the lowest specificity, 0.
319+
*
320+
* A media range matches the media types of its type and subtype that have
321+
* its parameters, and "text/html;level=1" is more specific than "text/html",
322+
* which is more specific than "text/*" (RFC 9110, Section 12.5.1).
323+
* A language range matches the tags that equal it or start with it followed
324+
* by "-", ignoring case, and is more specific with more subtags
325+
* (RFC 4647, Section 3.3.1). Other values must be equal.
326+
*
327+
* @param array{value: string, q: float, params: array<string, string>} $accept
328+
* @param array{value: string, q: float, params: array<string, string>} $supported
329+
*/
330+
private function getMatchSpecificity(array $accept, array $supported, bool $enforceTypes, bool $matchLocales): int
331+
{
332+
if ($accept['value'] === '*' || $accept['value'] === '*/*') {
333+
return 0;
334+
}
335+
336+
if ($enforceTypes) {
337+
[$aType, $aSubType] = explode('/', $accept['value'], 2) + ['', ''];
338+
[$sType, $sSubType] = explode('/', $supported['value'], 2) + ['', ''];
339+
340+
if ($aType !== $sType || ($aSubType !== '*' && $aSubType !== $sSubType)) {
341+
return -1;
342+
}
343+
344+
foreach ($accept['params'] as $label => $value) {
345+
if (($supported['params'][$label] ?? null) !== $value) {
346+
return -1;
347+
}
348+
}
349+
350+
return $aSubType === '*' ? 1 : 2 + count($accept['params']);
351+
}
352+
353+
if ($matchLocales) {
354+
$aTag = strtolower($accept['value']);
355+
$sTag = strtolower($supported['value']);
356+
357+
if ($aTag !== $sTag && ! str_starts_with($sTag, $aTag . '-')) {
358+
return -1;
359+
}
360+
361+
return substr_count($aTag, '-') + 1;
362+
}
363+
364+
return $accept['value'] === $supported['value'] ? 1 : -1;
365+
}
366+
266367
/**
267368
* Parses an Accept* header into it's multiple values.
268369
*

‎tests/system/HTTP/NegotiateTest.php‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,88 @@ 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 testMoreSpecificRangesTakePrecedence(): void
234+
{
235+
$this->request->setHeader('Accept', 'text/html, text/*;q=0');
236+
$this->assertSame('text/html', $this->negotiate->media(['text/plain', 'text/html'], true));
237+
$this->assertSame('', $this->negotiate->media(['text/plain'], true));
238+
239+
$this->request->setHeader('Accept', 'text/html;q=0, text/html;level=1;q=0.5, */*');
240+
$this->assertSame('text/html;level=1', $this->negotiate->media(['text/html', 'text/html;level=1'], true));
241+
242+
$this->request->setHeader('Accept-Language', 'en;q=0, en-GB;q=0.5, *');
243+
$this->assertSame('en-GB', $this->negotiate->language(['en-US', 'en-GB', 'fr']));
244+
245+
config(Feature::class)->strictLocaleNegotiation = true;
246+
247+
$this->assertSame('en-GB', $this->negotiate->language(['en-US', 'en-GB']));
248+
}
249+
250+
public function testLanguageRangesExcludeRegionalLocales(): void
251+
{
252+
$this->request->setHeader('Accept-Language', 'en;q=0, *');
253+
$this->assertSame('fr', $this->negotiate->language(['en-US', 'fr']));
254+
255+
// language ranges are case-insensitive and end on a subtag boundary
256+
$this->request->setHeader('Accept-Language', 'zh;q=0, *');
257+
$this->assertSame('zha', $this->negotiate->language(['ZH-Hant', 'zha']));
258+
259+
$this->request->setHeader('Accept-Language', 'en-US, en-GB;q=0');
260+
$this->assertSame('en-AU', $this->negotiate->language(['en-GB', 'en-AU']));
261+
262+
config(Feature::class)->strictLocaleNegotiation = true;
263+
264+
$this->request->setHeader('Accept-Language', 'en;q=0, *');
265+
$this->assertSame('fr', $this->negotiate->language(['en-US', 'fr']));
266+
267+
$this->request->setHeader('Accept-Language', 'zh;q=0, *');
268+
$this->assertSame('zha', $this->negotiate->language(['ZH-Hant', 'zha']));
269+
270+
$this->request->setHeader('Accept-Language', 'en-US, en-GB;q=0');
271+
$this->assertSame('en-AU', $this->negotiate->language(['en-GB', 'en-AU']));
272+
}
273+
192274
public function testBestMatchOnlyLowQuality(): void
193275
{
194276
$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
@@ -42,6 +42,7 @@ Bugs Fixed
4242
- **HTTP:** Fixed a bug in ``ContentSecurityPolicy::setDefaultSrc()`` where passing an array of URIs resulted in an invalid ``'Array'`` directive.
4343
- **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).
4444
- **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.
45+
- **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``.
4546
- **HTTP:** Fixed stale ``$_REQUEST`` values after ``SiteURIFactory`` normalizes GET parameters, so ``Validation::withRequest()`` reads the normalized values while unrelated REQUEST entries are preserved.
4647
- **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.
4748

0 commit comments

Comments
 (0)