diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c2caa10..9e823265 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ## [Unreleased] ### Added -* [#2041](https://github.com/shlinkio/shlink/issues/2041) Document `color` and `bgColor` params for the QR code route in the OAS docs. +* *Nothing* ### Changed * *Nothing* @@ -18,7 +18,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * *Nothing* ### Fixed -* *Nothing* +* [#2041](https://github.com/shlinkio/shlink/issues/2041) Document missing `color` and `bgColor` params for the QR code route in the OAS docs. +* [#2043](https://github.com/shlinkio/shlink/issues/2043) Fix language redirect conditions matching too low quality accepted languages. ## [4.0.0] - 2024-03-03 diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index e910833a..5ba45ac2 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -8,6 +8,7 @@ use BackedEnum; use Cake\Chronos\Chronos; use DateTimeInterface; use Doctrine\ORM\Mapping\Builder\FieldBuilder; +use GuzzleHttp\Psr7\Query; use Jaybizzle\CrawlerDetect\CrawlerDetect; use Laminas\Filter\Word\CamelCaseToSeparator; use Laminas\Filter\Word\CamelCaseToUnderscore; @@ -16,7 +17,6 @@ use PUGX\Shortid\Factory as ShortIdFactory; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode; -use function array_filter; use function array_keys; use function array_map; use function array_pad; @@ -27,6 +27,7 @@ use function implode; use function is_array; use function print_r; use function Shlinkio\Shlink\Common\buildDateRange; +use function Shlinkio\Shlink\Core\ArrayUtils\map; use function sprintf; use function str_repeat; use function str_replace; @@ -85,16 +86,30 @@ function normalizeLocale(string $locale): string } /** + * Parse an accept-language-like pattern into a list of locales, optionally filtering out those which do not match a + * minimum quality + * * @param non-empty-string $acceptLanguage - * @return string[]; + * @param float<0, 1> $minQuality + * @return iterable; */ -function acceptLanguageToLocales(string $acceptLanguage): array +function acceptLanguageToLocales(string $acceptLanguage, float $minQuality = 0): iterable { - $acceptLanguagesList = array_map(function (string $lang): string { - [$lang] = explode(';', $lang); // Discard everything after the semicolon (en-US;q=0.7) - return normalizeLocale($lang); - }, explode(',', $acceptLanguage)); - return array_filter($acceptLanguagesList, static fn (string $lang) => $lang !== '*'); + /** @var array{string, float|null}[] $acceptLanguagesList */ + $acceptLanguagesList = map(explode(',', $acceptLanguage), static function (string $lang): array { + // Split locale/language and quality (en-US;q=0.7) -> [en-US, q=0.7] + [$lang, $qualityString] = array_pad(explode(';', $lang), length: 2, value: ''); + $normalizedLang = normalizeLocale($lang); + $quality = Query::parse(trim($qualityString))['q'] ?? 1; + + return [$normalizedLang, (float) $quality]; + }); + + foreach ($acceptLanguagesList as [$lang, $quality]) { + if ($lang !== '*' && $quality >= $minQuality) { + yield $lang; + } + } } /** @@ -108,7 +123,7 @@ function acceptLanguageToLocales(string $acceptLanguage): array */ function splitLocale(string $locale): array { - return array_pad(explode('-', $locale), 2, null); + return array_pad(explode('-', $locale), length: 2, value: null); } function getOptionalIntFromInputFilter(InputFilter $inputFilter, string $fieldName): ?int diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index 29123733..5e6df494 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -77,7 +77,7 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable return false; } - $acceptedLanguages = acceptLanguageToLocales($acceptLanguage); + $acceptedLanguages = acceptLanguageToLocales($acceptLanguage, minQuality: 0.9); [$matchLanguage, $matchCountryCode] = splitLocale(normalizeLocale($this->matchValue)); return some( diff --git a/module/Core/test-api/Action/RedirectTest.php b/module/Core/test-api/Action/RedirectTest.php index cb623edc..e8cf211a 100644 --- a/module/Core/test-api/Action/RedirectTest.php +++ b/module/Core/test-api/Action/RedirectTest.php @@ -75,9 +75,15 @@ class RedirectTest extends ApiTestCase ]; yield 'rule: complex matching accept language' => [ [ - RequestOptions::HEADERS => ['Accept-Language' => 'fr-FR, es;q=08, en;q=0.5, *;q=0.2'], + RequestOptions::HEADERS => ['Accept-Language' => 'fr-FR, es;q=0.9, en;q=0.9, *;q=0.2'], ], 'https://example.com/only-english', ]; + yield 'rule: too low quality accept language' => [ + [ + RequestOptions::HEADERS => ['Accept-Language' => 'fr-FR, es;q=0.8, en;q=0.5, *;q=0.2'], + ], + 'https://blog.alejandrocelaya.com/2017/12/09/acmailer-7-0-the-most-important-release-in-a-long-time/', + ]; } } diff --git a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php index eaea4c25..a8ab2a16 100644 --- a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php +++ b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php @@ -35,6 +35,8 @@ class RedirectConditionTest extends TestCase #[TestWith(['es, en,fr', 'en', true])] // multiple languages match #[TestWith(['es, en-US,fr', 'EN', true])] // multiple locales match #[TestWith(['es_ES', 'es-ES', true])] // single locale match + #[TestWith(['en-US,es-ES;q=0.6', 'es-ES', false])] // too low quality + #[TestWith(['en-US,es-ES;q=0.9', 'es-ES', true])] // quality high enough #[TestWith(['en-UK', 'en-uk', true])] // different casing match #[TestWith(['en-UK', 'en', true])] // only lang #[TestWith(['es-AR', 'en', false])] // different only lang