diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 586195da..e910833a 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -16,10 +16,13 @@ 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; use function array_reduce; use function date_default_timezone_get; +use function explode; use function implode; use function is_array; use function print_r; @@ -81,6 +84,33 @@ function normalizeLocale(string $locale): string return trim(strtolower(str_replace('_', '-', $locale))); } +/** + * @param non-empty-string $acceptLanguage + * @return string[]; + */ +function acceptLanguageToLocales(string $acceptLanguage): array +{ + $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 !== '*'); +} + +/** + * Splits a locale into its corresponding language and country codes. + * The country code will be null if not present + * 'es-AR' -> ['es', 'AR'] + * 'fr-FR' -> ['fr', 'FR'] + * 'en' -> ['en', null] + * + * @return array{string, string|null} + */ +function splitLocale(string $locale): array +{ + return array_pad(explode('-', $locale), 2, null); +} + function getOptionalIntFromInputFilter(InputFilter $inputFilter, string $fieldName): ?int { $value = $inputFilter->getValue($fieldName); diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index da235f9e..9b7d3b3d 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -6,10 +6,12 @@ use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Entity\AbstractEntity; use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType; -use function explode; +use function Shlinkio\Shlink\Core\acceptLanguageToLocales; use function Shlinkio\Shlink\Core\ArrayUtils\some; use function Shlinkio\Shlink\Core\normalizeLocale; +use function Shlinkio\Shlink\Core\splitLocale; use function sprintf; +use function trim; class RedirectCondition extends AbstractEntity { @@ -58,17 +60,26 @@ class RedirectCondition extends AbstractEntity private function matchesLanguage(ServerRequestInterface $request): bool { - $acceptLanguage = $request->getHeaderLine('Accept-Language'); + $acceptLanguage = trim($request->getHeaderLine('Accept-Language')); if ($acceptLanguage === '' || $acceptLanguage === '*') { return false; } - $acceptedLanguages = explode(',', $acceptLanguage); - $normalizedLanguage = normalizeLocale($this->matchValue); + $acceptedLanguages = acceptLanguageToLocales($acceptLanguage); + $normalizedLocale = normalizeLocale($this->matchValue); + [$matchLanguage, $matchCountryCode] = splitLocale($normalizedLocale); return some( $acceptedLanguages, - static fn (string $lang) => normalizeLocale($lang) === $normalizedLanguage, + static function (string $lang) use ($matchLanguage, $matchCountryCode): bool { + [$language, $countryCode] = splitLocale($lang); + + if ($matchLanguage !== $language) { + return false; + } + + return $matchCountryCode === null || $matchCountryCode === $countryCode; + }, ); } } diff --git a/module/Core/test-api/Action/RedirectTest.php b/module/Core/test-api/Action/RedirectTest.php index 64c5fd95..cb623edc 100644 --- a/module/Core/test-api/Action/RedirectTest.php +++ b/module/Core/test-api/Action/RedirectTest.php @@ -50,9 +50,7 @@ class RedirectTest extends ApiTestCase ]; yield 'rule: english and foo' => [ [ - RequestOptions::HEADERS => [ - 'Accept-Language' => 'en-UK', - ], + RequestOptions::HEADERS => ['Accept-Language' => 'en-UK'], RequestOptions::QUERY => ['foo' => 'bar'], ], 'https://example.com/english-and-foo-query?foo=bar', @@ -63,11 +61,23 @@ class RedirectTest extends ApiTestCase ], 'https://example.com/multiple-query-params?foo=bar&hello=world', ]; - yield 'rule: english' => [ + yield 'rule: british english' => [ [ RequestOptions::HEADERS => ['Accept-Language' => 'en-UK'], ], 'https://example.com/only-english', ]; + yield 'rule: english' => [ + [ + RequestOptions::HEADERS => ['Accept-Language' => 'en'], + ], + 'https://example.com/only-english', + ]; + yield 'rule: complex matching accept language' => [ + [ + RequestOptions::HEADERS => ['Accept-Language' => 'fr-FR, es;q=08, en;q=0.5, *;q=0.2'], + ], + 'https://example.com/only-english', + ]; } } diff --git a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php index c52988dc..20178068 100644 --- a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php +++ b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php @@ -29,8 +29,12 @@ class RedirectConditionTest extends TestCase #[TestWith(['*', '', false])] // wildcard accept language #[TestWith(['en', 'en', true])] // single language match #[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-UK', 'en-uk', true])] // different casing match + #[TestWith(['en-UK', 'en', true])] // only lang + #[TestWith(['es-AR', 'en', false])] // different only lang + #[TestWith(['fr', 'fr-FR', false])] // less restrictive matching locale public function matchesLanguage(?string $acceptLanguage, string $value, bool $expected): void { $request = ServerRequestFactory::fromGlobals(); diff --git a/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php b/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php index 8f40af7d..6d321a91 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php @@ -24,7 +24,7 @@ class ShortUrlRedirectRulesFixture extends AbstractFixture implements DependentF /** @var ShortUrl $defShortUrl */ $defShortUrl = $this->getReference('def456_short_url'); - $englishCondition = RedirectCondition::forLanguage('en-UK'); + $englishCondition = RedirectCondition::forLanguage('en'); $manager->persist($englishCondition); $fooQueryCondition = RedirectCondition::forQueryParam('foo', 'bar');