From 2b330953922df7f87888525a5fca255800fe5c18 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 20 Jul 2025 11:56:33 +0200 Subject: [PATCH] Add support for more device types in device-specific redirects --- CHANGELOG.md | 10 ++++++++- config/test/constants.php | 10 +++++++-- module/Core/src/Model/DeviceType.php | 22 ++++++++++++++----- .../RedirectRule/Entity/RedirectCondition.php | 4 ++-- module/Core/test-api/Action/RedirectTest.php | 11 ++++++++-- .../Entity/RedirectConditionTest.php | 19 +++++++++++++--- .../ShortUrlRedirectionResolverTest.php | 4 ++-- .../test-api/Action/ListRedirectRulesTest.php | 11 ++++++++++ .../Fixtures/ShortUrlRedirectRulesFixture.php | 8 +++++++ 9 files changed, 82 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5d1339d..35cda1ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,10 +19,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this The new conditions match as soon as a query param exists with any or no value (in the case of `any-value-query-param`), or if a query param exists with no value at all (in the case of `valueless-query-param`). -* [#2387](https://github.com/shlinkio/shlink/issues/2387) Add `TRUSTED_PROXIES` env var and corresponding config option, to configure a comma-separated list of all the proxies in front of Shlink, or simply the amount of trusted proxies in front of Shlink. +* [#2360](https://github.com/shlinkio/shlink/issues/2360) Add `TRUSTED_PROXIES` env var and corresponding config option, to configure a comma-separated list of all the proxies in front of Shlink, or simply the amount of trusted proxies in front of Shlink. This is important to properly detect visitor's IP addresses instead of incorrectly matching one of the proxy's IP address, and if provided, it disables a workaround introduced in https://github.com/shlinkio/shlink/pull/2359. +* [#2274](https://github.com/shlinkio/shlink/issues/2274) Add more supported device types for the `device` redirect condition: + + * `linux`: Will match desktop devices with Linux. + * `windows`: Will match desktop devices with Windows. + * `macos`: Will match desktop devices with MacOS. + * `chromeos`: Will match desktop devices with ChromeOS. + * `mobile`: Will match any mobile devices with either Android or iOS. + ### Changed * [#2406](https://github.com/shlinkio/shlink/issues/2406) Remove references to bootstrap from error templates, and instead inline the very minimum required styles. diff --git a/config/test/constants.php b/config/test/constants.php index bce232f3..f2ad124d 100644 --- a/config/test/constants.php +++ b/config/test/constants.php @@ -11,5 +11,11 @@ const ANDROID_USER_AGENT = 'Mozilla/5.0 (Linux; Android 13) AppleWebKit/537.36 ( . 'Chrome/109.0.5414.86 Mobile Safari/537.36'; const IOS_USER_AGENT = 'Mozilla/5.0 (iPhone; CPU iPhone OS 16_2 like Mac OS X) AppleWebKit/605.1.15 ' . '(KHTML, like Gecko) FxiOS/109.0 Mobile/15E148 Safari/605.1.15'; -const DESKTOP_USER_AGENT = 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like ' - . 'Gecko) Chrome/109.0.0.0 Safari/537.36 Edg/109.0.1518.61'; +const WINDOWS_USER_AGENT = 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) ' + . 'Chrome/138.0.0.0 Safari/537.36 Edg/138.0.3351.95'; +const LINUX_USER_AGENT = 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) ' + . 'HeadlessChrome/81.0.4044.113 Safari/537.36'; +const MACOS_USER_AGENT = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 15_5) AppleWebKit/605.1.15 (KHTML, like Gecko) ' + . 'Version/18.4 Safari/605.1.15'; +const CHROMEOS_USER_AGENT = 'Mozilla/5.0 (X11; CrOS x86_64 16181.61.0) AppleWebKit/537.36 (KHTML, like Gecko) ' + . 'Chrome/134.0.6998.198 Safari/537.36'; diff --git a/module/Core/src/Model/DeviceType.php b/module/Core/src/Model/DeviceType.php index c63b2dff..11b57e02 100644 --- a/module/Core/src/Model/DeviceType.php +++ b/module/Core/src/Model/DeviceType.php @@ -9,18 +9,30 @@ enum DeviceType: string { case ANDROID = 'android'; case IOS = 'ios'; + case MOBILE = 'mobile'; + case WINDOWS = 'windows'; + case MACOS = 'macos'; + case LINUX = 'linux'; + case CHROMEOS = 'chromeos'; case DESKTOP = 'desktop'; - public static function matchFromUserAgent(string $userAgent): self|null + /** + * Determines which device types provided user agent matches. It could be more than one + * @return self[] + */ + public static function matchFromUserAgent(string $userAgent): array { static $uaParser = new UserAgentParser(); $ua = $uaParser->parse($userAgent); return match ($ua->platform()) { - Platforms::IPHONE, Platforms::IPAD => self::IOS, // Detects both iPhone and iPad (except iPadOS 13+) - Platforms::ANDROID => self::ANDROID, // Detects both android phones and android tablets - Platforms::LINUX, Platforms::WINDOWS, Platforms::MACINTOSH, Platforms::CHROME_OS => self::DESKTOP, - default => null, + Platforms::IPHONE, Platforms::IPAD => [self::IOS, self::MOBILE], // iPhone and iPad (except iPadOS 13+) + Platforms::ANDROID => [self::ANDROID, self::MOBILE], // android phones and android tablets + Platforms::LINUX => [self::LINUX, self::DESKTOP], + Platforms::WINDOWS => [self::WINDOWS, self::DESKTOP], + Platforms::MACINTOSH => [self::MACOS, self::DESKTOP], + Platforms::CHROME_OS => [self::CHROMEOS, self::DESKTOP], + default => [], }; } } diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php index dd24ca6d..45e29839 100644 --- a/module/Core/src/RedirectRule/Entity/RedirectCondition.php +++ b/module/Core/src/RedirectRule/Entity/RedirectCondition.php @@ -166,8 +166,8 @@ class RedirectCondition extends AbstractEntity implements JsonSerializable private function matchesDevice(ServerRequestInterface $request): bool { - $device = DeviceType::matchFromUserAgent($request->getHeaderLine('User-Agent')); - return $device !== null && $device->value === $this->matchValue; + $devices = DeviceType::matchFromUserAgent($request->getHeaderLine('User-Agent')); + return some($devices, fn (DeviceType $device) => $device->value === $this->matchValue); } private function matchesRemoteIpAddress(ServerRequestInterface $request): bool diff --git a/module/Core/test-api/Action/RedirectTest.php b/module/Core/test-api/Action/RedirectTest.php index 799851c7..bc2d0c70 100644 --- a/module/Core/test-api/Action/RedirectTest.php +++ b/module/Core/test-api/Action/RedirectTest.php @@ -13,8 +13,9 @@ use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; use function sprintf; use const ShlinkioTest\Shlink\ANDROID_USER_AGENT; -use const ShlinkioTest\Shlink\DESKTOP_USER_AGENT; use const ShlinkioTest\Shlink\IOS_USER_AGENT; +use const ShlinkioTest\Shlink\LINUX_USER_AGENT; +use const ShlinkioTest\Shlink\WINDOWS_USER_AGENT; class RedirectTest extends ApiTestCase { @@ -41,9 +42,15 @@ class RedirectTest extends ApiTestCase ], 'fb://profile/33138223345', ]; + yield 'linux' => [ + [ + RequestOptions::HEADERS => ['User-Agent' => LINUX_USER_AGENT], + ], + 'https://example.com/linux', + ]; yield 'desktop' => [ [ - RequestOptions::HEADERS => ['User-Agent' => DESKTOP_USER_AGENT], + RequestOptions::HEADERS => ['User-Agent' => WINDOWS_USER_AGENT], ], '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 28e3f6b1..cc544353 100644 --- a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php +++ b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php @@ -15,8 +15,11 @@ use Shlinkio\Shlink\IpGeolocation\Model\Location; use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE; use const ShlinkioTest\Shlink\ANDROID_USER_AGENT; -use const ShlinkioTest\Shlink\DESKTOP_USER_AGENT; +use const ShlinkioTest\Shlink\CHROMEOS_USER_AGENT; use const ShlinkioTest\Shlink\IOS_USER_AGENT; +use const ShlinkioTest\Shlink\LINUX_USER_AGENT; +use const ShlinkioTest\Shlink\MACOS_USER_AGENT; +use const ShlinkioTest\Shlink\WINDOWS_USER_AGENT; class RedirectConditionTest extends TestCase { @@ -89,10 +92,20 @@ class RedirectConditionTest extends TestCase #[TestWith([null, DeviceType::ANDROID, false])] #[TestWith(['unknown', DeviceType::ANDROID, false])] #[TestWith([ANDROID_USER_AGENT, DeviceType::ANDROID, true])] - #[TestWith([DESKTOP_USER_AGENT, DeviceType::DESKTOP, true])] + #[TestWith([WINDOWS_USER_AGENT, DeviceType::DESKTOP, true])] + #[TestWith([LINUX_USER_AGENT, DeviceType::DESKTOP, true])] + #[TestWith([MACOS_USER_AGENT, DeviceType::DESKTOP, true])] + #[TestWith([CHROMEOS_USER_AGENT, DeviceType::DESKTOP, true])] + #[TestWith([WINDOWS_USER_AGENT, DeviceType::WINDOWS, true])] + #[TestWith([LINUX_USER_AGENT, DeviceType::LINUX, true])] + #[TestWith([MACOS_USER_AGENT, DeviceType::MACOS, true])] + #[TestWith([CHROMEOS_USER_AGENT, DeviceType::CHROMEOS, true])] #[TestWith([IOS_USER_AGENT, DeviceType::IOS, true])] + #[TestWith([IOS_USER_AGENT, DeviceType::MOBILE, true])] + #[TestWith([ANDROID_USER_AGENT, DeviceType::MOBILE, true])] #[TestWith([IOS_USER_AGENT, DeviceType::ANDROID, false])] - #[TestWith([DESKTOP_USER_AGENT, DeviceType::IOS, false])] + #[TestWith([WINDOWS_USER_AGENT, DeviceType::IOS, false])] + #[TestWith([LINUX_USER_AGENT, DeviceType::WINDOWS, false])] public function matchesDevice(string|null $userAgent, DeviceType $value, bool $expected): void { $request = ServerRequestFactory::fromGlobals(); diff --git a/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php b/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php index 470ff95e..d7948be2 100644 --- a/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php +++ b/module/Core/test/RedirectRule/ShortUrlRedirectionResolverTest.php @@ -19,8 +19,8 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE; use const ShlinkioTest\Shlink\ANDROID_USER_AGENT; -use const ShlinkioTest\Shlink\DESKTOP_USER_AGENT; use const ShlinkioTest\Shlink\IOS_USER_AGENT; +use const ShlinkioTest\Shlink\WINDOWS_USER_AGENT; class ShortUrlRedirectionResolverTest extends TestCase { @@ -68,7 +68,7 @@ class ShortUrlRedirectionResolverTest extends TestCase RedirectCondition::forLanguage('es-ES'), // This condition won't match 'https://example.com/foo/bar', ]; - yield 'desktop user agent' => [$request(DESKTOP_USER_AGENT), null, 'https://example.com/foo/bar']; + yield 'desktop user agent' => [$request(WINDOWS_USER_AGENT), null, 'https://example.com/foo/bar']; yield 'matching android device' => [ $request(ANDROID_USER_AGENT), RedirectCondition::forDevice(DeviceType::ANDROID), diff --git a/module/Rest/test-api/Action/ListRedirectRulesTest.php b/module/Rest/test-api/Action/ListRedirectRulesTest.php index 7909dcfd..b24caa7a 100644 --- a/module/Rest/test-api/Action/ListRedirectRulesTest.php +++ b/module/Rest/test-api/Action/ListRedirectRulesTest.php @@ -98,6 +98,17 @@ class ListRedirectRulesTest extends ApiTestCase ], ], ], + [ + 'longUrl' => 'https://example.com/linux', + 'priority' => 7, + 'conditions' => [ + [ + 'type' => 'device', + 'matchKey' => null, + 'matchValue' => 'linux', + ], + ], + ], ]])] public function returnsListOfRulesForShortUrl(string $shortCode, array $expectedRules): void { diff --git a/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php b/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php index 3aa81315..50b1dae5 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlRedirectRulesFixture.php @@ -78,6 +78,14 @@ class ShortUrlRedirectRulesFixture extends AbstractFixture implements DependentF ); $manager->persist($ipAddressRule); + $linuxRule = new ShortUrlRedirectRule( + shortUrl: $defShortUrl, + priority: 7, + longUrl: 'https://example.com/linux', + conditions: new ArrayCollection([RedirectCondition::forDevice(DeviceType::LINUX)]), + ); + $manager->persist($linuxRule); + $manager->flush(); } }