Merge pull request #2465 from acelaya-forks/feature/redirect-cache-visibility

Allow redirect cache visibility to be configured

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.
 

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",

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,

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';

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';

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(),
         );
     }
 }

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);

module/Core/test/Util/RedirectResponseHelperTest.php

@@ -15,13 +15,10 @@ class RedirectResponseHelperTest extends TestCase
 {
     #[Test, DataProvider('provideRedirectConfigs')]
     public function expectedStatusCodeAndCacheIsReturnedBasedOnConfig(
-        int $configuredStatus,
+        RedirectOptions $options,
-        int $configuredLifetime,
         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 302' => [new RedirectOptions(302, 20), 302, null];
-        yield 'status 307' => [307, 20, 307, null];
+        yield 'status 307' => [new RedirectOptions(307, 20), 307, null];
-        yield 'status over 308' => [400, 20, 302, null];
+        yield 'status over 308' => [new RedirectOptions(400, 20), 302, null];
-        yield 'status below 301' => [201, 20, 302, null];
+        yield 'status below 301' => [new RedirectOptions(201, 20), 302, null];
-        yield 'status 301 with valid expiration' => [301, 20, 301, 'private,max-age=20'];
+        yield 'status 301 with valid expiration' => [new RedirectOptions(301, 20), 301, 'private,max-age=20'];
-        yield 'status 301 with zero expiration' => [301, 0, 301, 'private,max-age=30'];
+        yield 'status 301 with zero expiration' => [new RedirectOptions(301, 0), 301, 'private,max-age=30'];
-        yield 'status 301 with negative expiration' => [301, -20, 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' => [308, 20, 308, 'private,max-age=20'];
+        yield 'status 308 with valid expiration' => [new RedirectOptions(308, 20), 308, 'private,max-age=20'];
-        yield 'status 308 with zero expiration' => [308, 0, 308, 'private,max-age=30'];
+        yield 'status 308 with zero expiration' => [new RedirectOptions(308, 0), 308, 'private,max-age=30'];
-        yield 'status 308 with negative expiration' => [308, -20, 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