From 78e6fb160cf91aa307e4164c295cbdb0399a4b8c Mon Sep 17 00:00:00 2001 From: Wleeaf Date: Mon, 26 Oct 2026 14:48:15 +0300 Subject: [PATCH] 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. --- system/HTTP/Negotiate.php | 113 ++++++++++++++++++-- tests/system/HTTP/NegotiateTest.php | 82 ++++++++++++++ user_guide_src/source/changelogs/v4.7.6.rst | 1 + 3 files changed, 190 insertions(+), 6 deletions(-) diff --git a/system/HTTP/Negotiate.php b/system/HTTP/Negotiate.php index 82123558a273..4c4cd8767c3b 100644 --- a/system/HTTP/Negotiate.php +++ b/system/HTTP/Negotiate.php @@ -184,13 +184,24 @@ protected function getBestMatch( } // if acceptable value is "anything", return the first available + // that the header does not exclude if ($accept['value'] === '*' || $accept['value'] === '*/*') { - return $supported[0]; + foreach ($supported as $available) { + if (! $this->isExcluded($acceptable, $available, $enforceTypes, $matchLocales)) { + return $available; + } + } + + continue; } - // If an acceptable value is supported, return it + // If an acceptable value is supported, return it, unless the header + // excludes it with a more specific value, such as "text/html;q=0" + // for "text/*" foreach ($supported as $available) { - if ($this->match($accept, $available, $enforceTypes, $matchLocales)) { + if ($this->match($accept, $available, $enforceTypes, $matchLocales) + && ! $this->isExcluded($acceptable, $available, $enforceTypes, $matchLocales) + ) { return $available; } } @@ -233,8 +244,15 @@ protected function getBestLocaleMatch(array $supportedLocales, ?string $header): } // if acceptable value is "anything", return the first available + // that the header does not exclude if ($accept['value'] === '*') { - return $supportedLocales[0]; + foreach ($supportedLocales as $locale) { + if (! $this->isExcluded($acceptable, $locale, false, true)) { + return $locale; + } + } + + continue; } // look for exact match @@ -248,13 +266,17 @@ protected function getBestLocaleMatch(array $supportedLocales, ?string $header): foreach ($fallbackLocales as $fallbackLocale) { // look for exact match - if (in_array($fallbackLocale, $supportedLocales, true)) { + if (in_array($fallbackLocale, $supportedLocales, true) + && ! $this->isExcluded($acceptable, $fallbackLocale, false, true) + ) { return $fallbackLocale; } // look for regional locale match foreach ($supportedLocales as $locale) { - if (str_starts_with($locale, $fallbackLocale . '-')) { + if (str_starts_with($locale, $fallbackLocale . '-') + && ! $this->isExcluded($acceptable, $locale, false, true) + ) { return $locale; } } @@ -263,6 +285,85 @@ protected function getBestLocaleMatch(array $supportedLocales, ?string $header): return $supportedLocales[0]; } + /** + * Checks whether the header excludes a supported value, that is, whether + * the most specific value in the header that matches it has q=0. + * + * @param list}> $acceptable + * @param bool $enforceTypes If TRUE, the values are media types. + * @param bool $matchLocales If TRUE, the values are language tags. + */ + private function isExcluded(array $acceptable, string $supported, bool $enforceTypes, bool $matchLocales): bool + { + $supported = $this->parseHeader($supported)[0]; + $specificity = -1; + $excluded = false; + + // The header is sorted by quality, so of two equally specific values, + // the one with the higher quality is used. + foreach ($acceptable as $accept) { + $acceptSpecificity = $this->getMatchSpecificity($accept, $supported, $enforceTypes, $matchLocales); + + if ($acceptSpecificity > $specificity) { + $specificity = $acceptSpecificity; + $excluded = $accept['q'] === 0.0; + } + } + + return $excluded; + } + + /** + * Returns how specifically a header value matches a supported value, or -1 + * if it does not match it. A wildcard has the lowest specificity, 0. + * + * A media range matches the media types of its type and subtype that have + * its parameters, and "text/html;level=1" is more specific than "text/html", + * which is more specific than "text/*" (RFC 9110, Section 12.5.1). + * A language range matches the tags that equal it or start with it followed + * by "-", ignoring case, and is more specific with more subtags + * (RFC 4647, Section 3.3.1). Other values must be equal. + * + * @param array{value: string, q: float, params: array} $accept + * @param array{value: string, q: float, params: array} $supported + */ + private function getMatchSpecificity(array $accept, array $supported, bool $enforceTypes, bool $matchLocales): int + { + if ($accept['value'] === '*' || $accept['value'] === '*/*') { + return 0; + } + + if ($enforceTypes) { + [$aType, $aSubType] = explode('/', $accept['value'], 2) + ['', '']; + [$sType, $sSubType] = explode('/', $supported['value'], 2) + ['', '']; + + if ($aType !== $sType || ($aSubType !== '*' && $aSubType !== $sSubType)) { + return -1; + } + + foreach ($accept['params'] as $label => $value) { + if (($supported['params'][$label] ?? null) !== $value) { + return -1; + } + } + + return $aSubType === '*' ? 1 : 2 + count($accept['params']); + } + + if ($matchLocales) { + $aTag = strtolower($accept['value']); + $sTag = strtolower($supported['value']); + + if ($aTag !== $sTag && ! str_starts_with($sTag, $aTag . '-')) { + return -1; + } + + return substr_count($aTag, '-') + 1; + } + + return $accept['value'] === $supported['value'] ? 1 : -1; + } + /** * Parses an Accept* header into it's multiple values. * diff --git a/tests/system/HTTP/NegotiateTest.php b/tests/system/HTTP/NegotiateTest.php index 083846c8a1a8..019cc2cf7d3d 100644 --- a/tests/system/HTTP/NegotiateTest.php +++ b/tests/system/HTTP/NegotiateTest.php @@ -189,6 +189,88 @@ public function testBestMatchLowQuality(): void $this->assertSame('apple/mac', $this->negotiate->media(['popcorn/cheddar', 'apple/mac'], false)); } + public function testWildcardsSkipExcludedValues(): void + { + $this->request->setHeader('Accept', 'application/json;q=0, */*'); + $this->assertSame('text/html', $this->negotiate->media(['application/json', 'text/html'], true)); + + $this->request->setHeader('Accept', 'text/*, text/html;q=0'); + $this->assertSame('text/plain', $this->negotiate->media(['text/html', 'text/plain'], true)); + + $this->request->setHeader('Accept', '*/*, text/html;q=0'); + $this->assertSame('', $this->negotiate->media(['text/html'], true)); + + $this->request->setHeader('Accept-Charset', 'utf-8;q=0, *'); + $this->assertSame('iso-8859-1', $this->negotiate->charset(['utf-8', 'iso-8859-1'])); + + $this->request->setHeader('Accept-Encoding', 'gzip;q=0, *'); + $this->assertSame('br', $this->negotiate->encoding(['gzip', 'br'])); + + $this->request->setHeader('Accept-Language', 'en;q=0, *'); + $this->assertSame('fr', $this->negotiate->language(['en', 'fr'])); + + config(Feature::class)->strictLocaleNegotiation = true; + + $this->assertSame('de', $this->negotiate->language(['en', 'de'])); + } + + public function testMediaRangesExcludeMatchingValues(): void + { + $this->request->setHeader('Accept', 'text/*;q=0, */*'); + $this->assertSame('application/json', $this->negotiate->media(['text/html', 'application/json'], true)); + + $this->request->setHeader('Accept', 'text/html;q=0, */*'); + $this->assertSame('text/plain', $this->negotiate->media(['text/html;charset=utf-8', 'text/plain'], true)); + + $this->request->setHeader('Accept', 'text/html;q=0, text/*'); + $this->assertSame('text/css', $this->negotiate->media(['text/html;charset=utf-8', 'text/css'], true)); + + // a range only excludes the values that have its parameters + $this->request->setHeader('Accept', 'text/html;level=1;q=0, */*'); + $this->assertSame('text/html', $this->negotiate->media(['text/html;level=1', 'text/html'], true)); + } + + public function testMoreSpecificRangesTakePrecedence(): void + { + $this->request->setHeader('Accept', 'text/html, text/*;q=0'); + $this->assertSame('text/html', $this->negotiate->media(['text/plain', 'text/html'], true)); + $this->assertSame('', $this->negotiate->media(['text/plain'], true)); + + $this->request->setHeader('Accept', 'text/html;q=0, text/html;level=1;q=0.5, */*'); + $this->assertSame('text/html;level=1', $this->negotiate->media(['text/html', 'text/html;level=1'], true)); + + $this->request->setHeader('Accept-Language', 'en;q=0, en-GB;q=0.5, *'); + $this->assertSame('en-GB', $this->negotiate->language(['en-US', 'en-GB', 'fr'])); + + config(Feature::class)->strictLocaleNegotiation = true; + + $this->assertSame('en-GB', $this->negotiate->language(['en-US', 'en-GB'])); + } + + public function testLanguageRangesExcludeRegionalLocales(): void + { + $this->request->setHeader('Accept-Language', 'en;q=0, *'); + $this->assertSame('fr', $this->negotiate->language(['en-US', 'fr'])); + + // language ranges are case-insensitive and end on a subtag boundary + $this->request->setHeader('Accept-Language', 'zh;q=0, *'); + $this->assertSame('zha', $this->negotiate->language(['ZH-Hant', 'zha'])); + + $this->request->setHeader('Accept-Language', 'en-US, en-GB;q=0'); + $this->assertSame('en-AU', $this->negotiate->language(['en-GB', 'en-AU'])); + + config(Feature::class)->strictLocaleNegotiation = true; + + $this->request->setHeader('Accept-Language', 'en;q=0, *'); + $this->assertSame('fr', $this->negotiate->language(['en-US', 'fr'])); + + $this->request->setHeader('Accept-Language', 'zh;q=0, *'); + $this->assertSame('zha', $this->negotiate->language(['ZH-Hant', 'zha'])); + + $this->request->setHeader('Accept-Language', 'en-US, en-GB;q=0'); + $this->assertSame('en-AU', $this->negotiate->language(['en-GB', 'en-AU'])); + } + public function testBestMatchOnlyLowQuality(): void { $this->request->setHeader('Accept', 'popcorn/cheddar;q=0'); diff --git a/user_guide_src/source/changelogs/v4.7.6.rst b/user_guide_src/source/changelogs/v4.7.6.rst index 4c36d122395f..63d1cbaa3b73 100644 --- a/user_guide_src/source/changelogs/v4.7.6.rst +++ b/user_guide_src/source/changelogs/v4.7.6.rst @@ -42,6 +42,7 @@ Bugs Fixed - **HTTP:** Fixed a bug in ``ContentSecurityPolicy::setDefaultSrc()`` where passing an array of URIs resulted in an invalid ``'Array'`` directive. - **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). - **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. +- **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``. - **HTTP:** Fixed stale ``$_REQUEST`` values after ``SiteURIFactory`` normalizes GET parameters, so ``Validation::withRequest()`` reads the normalized values while unrelated REQUEST entries are preserved. - **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.