From 8b259b364d7a1d5db7f08e0d098e005ca909ccae Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 21 Jul 2025 10:02:36 +0200 Subject: [PATCH] Allow redirect cache visibility to be configured --- CHANGELOG.md | 2 + composer.json | 2 +- config/autoload/installer.global.php | 1 + config/constants.php | 1 + module/Core/src/Config/EnvVars.php | 1 + .../src/Config/Options/RedirectOptions.php | 11 ++++- .../Core/src/Util/RedirectResponseHelper.php | 6 ++- .../test/Util/RedirectResponseHelperTest.php | 45 +++++++++++++------ 8 files changed, 51 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35cda1ca..b9671fa9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * `chromeos`: Will match desktop devices with ChromeOS. * `mobile`: Will match any mobile devices with either Android or iOS. +* [#2093](https://github.com/shlinkio/shlink/issues/2093) Add `REDIRECT_CACHE_LIFETIME` env var and corresponding config option, so that it is possible to set the `Cache-Control` visibility directive (`public` or `private`) when the `REDIRECT_STATUS_CODE` has been set to `301` or `308`. + ### 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/composer.json b/composer.json index 720a7089..85ad757b 100644 --- a/composer.json +++ b/composer.json @@ -47,7 +47,7 @@ "shlinkio/shlink-config": "^4.0", "shlinkio/shlink-event-dispatcher": "^4.2", "shlinkio/shlink-importer": "^5.6", - "shlinkio/shlink-installer": "dev-develop#eef3749 as 9.6", + "shlinkio/shlink-installer": "dev-develop#7f9147b as 9.6", "shlinkio/shlink-ip-geolocation": "^4.3", "shlinkio/shlink-json": "^1.2", "spiral/roadrunner": "^2025.1", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 9a7bff04..21d16bb3 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -41,6 +41,7 @@ return [ Option\UrlShortener\GeoLiteLicenseKeyConfigOption::class, Option\UrlShortener\RedirectStatusCodeConfigOption::class, Option\UrlShortener\RedirectCacheLifeTimeConfigOption::class, + Option\UrlShortener\RedirectCacheVisibilityConfigOption::class, Option\UrlShortener\AutoResolveTitlesConfigOption::class, Option\UrlShortener\ExtraPathModeConfigOption::class, Option\UrlShortener\EnableMultiSegmentSlugsConfigOption::class, diff --git a/config/constants.php b/config/constants.php index 28440653..664c8c9f 100644 --- a/config/constants.php +++ b/config/constants.php @@ -11,6 +11,7 @@ const DEFAULT_SHORT_CODES_LENGTH = 5; const MIN_SHORT_CODES_LENGTH = 4; const DEFAULT_REDIRECT_STATUS_CODE = RedirectStatus::STATUS_302; const DEFAULT_REDIRECT_CACHE_LIFETIME = 30; +const DEFAULT_REDIRECT_CACHE_VISIBILITY = 'private'; const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory'; const LOOSE_URI_MATCHER = '/(.+)\:(.+)/i'; // Matches anything starting with a schema. const IP_ADDRESS_REQUEST_ATTRIBUTE = 'remote_address'; diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index 072ee8db..c99635da 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -62,6 +62,7 @@ enum EnvVars: string case DEFAULT_BASE_URL_REDIRECT = 'DEFAULT_BASE_URL_REDIRECT'; case REDIRECT_STATUS_CODE = 'REDIRECT_STATUS_CODE'; case REDIRECT_CACHE_LIFETIME = 'REDIRECT_CACHE_LIFETIME'; + case REDIRECT_CACHE_VISIBILITY = 'REDIRECT_CACHE_VISIBILITY'; case BASE_PATH = 'BASE_PATH'; case SHORT_URL_TRAILING_SLASH = 'SHORT_URL_TRAILING_SLASH'; case SHORT_URL_MODE = 'SHORT_URL_MODE'; diff --git a/module/Core/src/Config/Options/RedirectOptions.php b/module/Core/src/Config/Options/RedirectOptions.php index 75faf097..ffb6dd36 100644 --- a/module/Core/src/Config/Options/RedirectOptions.php +++ b/module/Core/src/Config/Options/RedirectOptions.php @@ -4,26 +4,32 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Config\Options; -use Fig\Http\Message\StatusCodeInterface; use Shlinkio\Shlink\Core\Config\EnvVars; use Shlinkio\Shlink\Core\Util\RedirectStatus; use const Shlinkio\Shlink\DEFAULT_REDIRECT_CACHE_LIFETIME; +use const Shlinkio\Shlink\DEFAULT_REDIRECT_CACHE_VISIBILITY; use const Shlinkio\Shlink\DEFAULT_REDIRECT_STATUS_CODE; final readonly class RedirectOptions { public RedirectStatus $redirectStatusCode; public int $redirectCacheLifetime; + /** @var 'public'|'private' */ + public string $redirectCacheVisibility; public function __construct( - int $redirectStatusCode = StatusCodeInterface::STATUS_FOUND, + int $redirectStatusCode = RedirectStatus::STATUS_302->value, int $redirectCacheLifetime = DEFAULT_REDIRECT_CACHE_LIFETIME, + string|null $redirectCacheVisibility = DEFAULT_REDIRECT_CACHE_VISIBILITY, ) { $this->redirectStatusCode = RedirectStatus::tryFrom($redirectStatusCode) ?? DEFAULT_REDIRECT_STATUS_CODE; $this->redirectCacheLifetime = $redirectCacheLifetime > 0 ? $redirectCacheLifetime : DEFAULT_REDIRECT_CACHE_LIFETIME; + $this->redirectCacheVisibility = $redirectCacheVisibility === 'public' || $redirectCacheVisibility === 'private' + ? $redirectCacheVisibility + : DEFAULT_REDIRECT_CACHE_VISIBILITY; } public static function fromEnv(): self @@ -31,6 +37,7 @@ final readonly class RedirectOptions return new self( redirectStatusCode: (int) EnvVars::REDIRECT_STATUS_CODE->loadFromEnv(), redirectCacheLifetime: (int) EnvVars::REDIRECT_CACHE_LIFETIME->loadFromEnv(), + redirectCacheVisibility: EnvVars::REDIRECT_CACHE_VISIBILITY->loadFromEnv(), ); } } diff --git a/module/Core/src/Util/RedirectResponseHelper.php b/module/Core/src/Util/RedirectResponseHelper.php index 4c4fdd21..31b1d016 100644 --- a/module/Core/src/Util/RedirectResponseHelper.php +++ b/module/Core/src/Util/RedirectResponseHelper.php @@ -20,7 +20,11 @@ readonly class RedirectResponseHelper implements RedirectResponseHelperInterface { $statusCode = $this->options->redirectStatusCode; $headers = ! $statusCode->allowsCache() ? [] : [ - 'Cache-Control' => sprintf('private,max-age=%s', $this->options->redirectCacheLifetime), + 'Cache-Control' => sprintf( + '%s,max-age=%s', + $this->options->redirectCacheVisibility, + $this->options->redirectCacheLifetime, + ), ]; return new RedirectResponse($location, $statusCode->value, $headers); diff --git a/module/Core/test/Util/RedirectResponseHelperTest.php b/module/Core/test/Util/RedirectResponseHelperTest.php index b01333d5..f4e731d9 100644 --- a/module/Core/test/Util/RedirectResponseHelperTest.php +++ b/module/Core/test/Util/RedirectResponseHelperTest.php @@ -15,13 +15,10 @@ class RedirectResponseHelperTest extends TestCase { #[Test, DataProvider('provideRedirectConfigs')] public function expectedStatusCodeAndCacheIsReturnedBasedOnConfig( - int $configuredStatus, - int $configuredLifetime, + RedirectOptions $options, int $expectedStatus, string|null $expectedCacheControl, ): void { - $options = new RedirectOptions($configuredStatus, $configuredLifetime); - $response = $this->helper($options)->buildRedirectResponse('destination'); self::assertInstanceOf(RedirectResponse::class, $response); @@ -34,16 +31,36 @@ class RedirectResponseHelperTest extends TestCase public static function provideRedirectConfigs(): iterable { - yield 'status 302' => [302, 20, 302, null]; - yield 'status 307' => [307, 20, 307, null]; - yield 'status over 308' => [400, 20, 302, null]; - yield 'status below 301' => [201, 20, 302, null]; - yield 'status 301 with valid expiration' => [301, 20, 301, 'private,max-age=20']; - yield 'status 301 with zero expiration' => [301, 0, 301, 'private,max-age=30']; - yield 'status 301 with negative expiration' => [301, -20, 301, 'private,max-age=30']; - yield 'status 308 with valid expiration' => [308, 20, 308, 'private,max-age=20']; - yield 'status 308 with zero expiration' => [308, 0, 308, 'private,max-age=30']; - yield 'status 308 with negative expiration' => [308, -20, 308, 'private,max-age=30']; + yield 'status 302' => [new RedirectOptions(302, 20), 302, null]; + yield 'status 307' => [new RedirectOptions(307, 20), 307, null]; + yield 'status over 308' => [new RedirectOptions(400, 20), 302, null]; + yield 'status below 301' => [new RedirectOptions(201, 20), 302, null]; + yield 'status 301 with valid expiration' => [new RedirectOptions(301, 20), 301, 'private,max-age=20']; + yield 'status 301 with zero expiration' => [new RedirectOptions(301, 0), 301, 'private,max-age=30']; + yield 'status 301 with negative expiration' => [new RedirectOptions(301, -20), 301, 'private,max-age=30']; + yield 'status 308 with valid expiration' => [new RedirectOptions(308, 20), 308, 'private,max-age=20']; + yield 'status 308 with zero expiration' => [new RedirectOptions(308, 0), 308, 'private,max-age=30']; + yield 'status 308 with negative expiration' => [new RedirectOptions(308, -20), 308, 'private,max-age=30']; + yield 'status 301 with public cache' => [ + new RedirectOptions(301, redirectCacheVisibility: 'public'), + 301, + 'public,max-age=30', + ]; + yield 'status 308 with public cache' => [ + new RedirectOptions(308, redirectCacheVisibility: 'public'), + 308, + 'public,max-age=30', + ]; + yield 'status 301 with private cache' => [ + new RedirectOptions(301, redirectCacheVisibility: 'private'), + 301, + 'private,max-age=30', + ]; + yield 'status 301 with invalid cache' => [ + new RedirectOptions(301, redirectCacheVisibility: 'something-else'), + 301, + 'private,max-age=30', + ]; } private function helper(RedirectOptions|null $options = null): RedirectResponseHelper