diff --git a/CHANGELOG.md b/CHANGELOG.md index 6993aeb7..b2358670 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this #### Added -* *Nothing* +* [#746](https://github.com/shlinkio/shlink/issues/746) Allowed to configure the kind of redirect you want to use for your short URLs. You can either set: + + * `302` redirects: Default behavior. Visitors always hit the server. + * `301` redirects: Better for SEO. Visitors hit the server the first time and then cache the redirect. + + When selecting 301 redirects, you can also configure the time redirects are cached, to mitigate deviations in stats. + +* [#709](https://github.com/shlinkio/shlink/issues/709) Added multi-architecture builds for the docker image. #### Changed @@ -31,7 +38,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this #### Added -* [#709](https://github.com/shlinkio/shlink/issues/709) Added multi-architecture builds for the docker image. +* *Nothing* #### Changed diff --git a/composer.json b/composer.json index 777463b2..997f8558 100644 --- a/composer.json +++ b/composer.json @@ -52,7 +52,7 @@ "shlinkio/shlink-common": "^3.1.0", "shlinkio/shlink-config": "^1.0", "shlinkio/shlink-event-dispatcher": "^1.4", - "shlinkio/shlink-installer": "^5.0.0", + "shlinkio/shlink-installer": "^5.1.0", "shlinkio/shlink-ip-geolocation": "^1.4", "symfony/console": "^5.1", "symfony/filesystem": "^5.1", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index db1914db..ba0b8332 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -37,6 +37,8 @@ return [ Option\Mercure\MercureJwtSecretConfigOption::class, Option\UrlShortener\GeoLiteLicenseKeyConfigOption::class, Option\UrlShortener\IpAnonymizationConfigOption::class, + Option\UrlShortener\RedirectStatusCodeConfigOption::class, + Option\UrlShortener\RedirectCacheLifeTimeConfigOption::class, ], 'installation_commands' => [ diff --git a/config/autoload/swoole.local.php.dist b/config/autoload/swoole.local.php.dist index 0c485690..f30b3610 100644 --- a/config/autoload/swoole.local.php.dist +++ b/config/autoload/swoole.local.php.dist @@ -2,9 +2,6 @@ declare(strict_types=1); -use Laminas\ServiceManager\Factory\InvokableFactory; -use Mezzio\Swoole\HotCodeReload\FileWatcher\InotifyFileWatcher; - return [ 'mezzio-swoole' => [ @@ -13,10 +10,4 @@ return [ ], ], - 'dependencies' => [ - 'factories' => [ - InotifyFileWatcher::class => InvokableFactory::class, - ], - ], - ]; diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 5ad66bea..f27210af 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -2,6 +2,8 @@ declare(strict_types=1); +use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_CACHE_LIFETIME; +use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_STATUS_CODE; use const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH; return [ @@ -15,6 +17,8 @@ return [ 'anonymize_remote_addr' => true, 'visits_webhooks' => [], 'default_short_codes_length' => DEFAULT_SHORT_CODES_LENGTH, + 'redirect_status_code' => DEFAULT_REDIRECT_STATUS_CODE, + 'redirect_cache_lifetime' => DEFAULT_REDIRECT_CACHE_LIFETIME, ], ]; diff --git a/config/cli-config.php b/config/cli-config.php index c0e80687..71c7a75e 100644 --- a/config/cli-config.php +++ b/config/cli-config.php @@ -4,11 +4,10 @@ declare(strict_types=1); use Doctrine\ORM\EntityManager; use Doctrine\ORM\Tools\Console\ConsoleRunner; -use Laminas\ServiceManager\ServiceManager; use Psr\Container\ContainerInterface; return (function () { - /** @var ContainerInterface|ServiceManager $container */ + /** @var ContainerInterface $container */ $container = include __DIR__ . '/container.php'; $em = $container->get(EntityManager::class); diff --git a/docker/README.md b/docker/README.md index 199dc6e2..561bdba0 100644 --- a/docker/README.md +++ b/docker/README.md @@ -174,6 +174,8 @@ This is the complete list of supported env vars: * `MERCURE_INTERNAL_HUB_URL`: An internal URL for a mercure hub. Will be used only when publishing updates to mercure, and does not need to be public. If this is not provided but `MERCURE_PUBLIC_HUB_URL` was, the former one will be used to publish updates. * `MERCURE_JWT_SECRET`: The secret key that was provided to the mercure hub server, in order to be able to generate valid JWTs for publishing/subscribing to that server. * `ANONYMIZE_REMOTE_ADDR`: Tells if IP addresses from visitors should be obfuscated before storing them in the database. Default value is `true`. **Careful!** Setting this to `false` will make your Shlink instance no longer be in compliance with the GDPR and other similar data protection regulations. +* `REDIRECT_STATUS_CODE`: Either **301** or **302**. Used to determine if redirects from short to long URLs should be used with a 301 or 302 status. Defaults to 302. +* `REDIRECT_CACHE_LIFETIME`: Allows to set the amount of seconds that redirects should be cached when redirect status is 301. Default values is 30. An example using all env vars could look like this: @@ -206,6 +208,8 @@ docker run \ -e "MERCURE_INTERNAL_HUB_URL=http://my-mercure-hub.prod.svc.cluster.local" \ -e MERCURE_JWT_SECRET=super_secret_key \ -e ANONYMIZE_REMOTE_ADDR=false \ + -e REDIRECT_STATUS_CODE=301 \ + -e REDIRECT_CACHE_LIFETIME=90 \ shlinkio/shlink:stable ``` @@ -251,7 +255,9 @@ The whole configuration should have this format, but it can be split into multip "mercure_public_hub_url": "https://example.com", "mercure_internal_hub_url": "http://my-mercure-hub.prod.svc.cluster.local", "mercure_jwt_secret": "super_secret_key", - "anonymize_remote_addr": false + "anonymize_remote_addr": false, + "redirect_status_code": 301, + "redirect_cache_lifetime": 90 } ``` diff --git a/docker/config/shlink_in_docker.local.php b/docker/config/shlink_in_docker.local.php index b870ccd7..8d3c55fa 100644 --- a/docker/config/shlink_in_docker.local.php +++ b/docker/config/shlink_in_docker.local.php @@ -11,6 +11,9 @@ use function explode; use function Functional\contains; use function Shlinkio\Shlink\Common\env; +use const Shlinkio\Shlink\Core\DEFAULT_DELETE_SHORT_URL_THRESHOLD; +use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_CACHE_LIFETIME; +use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_STATUS_CODE; use const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH; use const Shlinkio\Shlink\Core\MIN_SHORT_CODES_LENGTH; @@ -104,7 +107,7 @@ return [ 'delete_short_urls' => [ 'check_visits_threshold' => true, - 'visits_threshold' => (int) env('DELETE_SHORT_URL_THRESHOLD', 15), + 'visits_threshold' => (int) env('DELETE_SHORT_URL_THRESHOLD', DEFAULT_DELETE_SHORT_URL_THRESHOLD), ], 'entity_manager' => [ @@ -120,6 +123,8 @@ return [ 'anonymize_remote_addr' => (bool) env('ANONYMIZE_REMOTE_ADDR', true), 'visits_webhooks' => $helper->getVisitsWebhooks(), 'default_short_codes_length' => $helper->getDefaultShortCodesLength(), + 'redirect_status_code' => (int) env('REDIRECT_STATUS_CODE', DEFAULT_REDIRECT_STATUS_CODE), + 'redirect_cache_lifetime' => (int) env('REDIRECT_CACHE_LIFETIME', DEFAULT_REDIRECT_CACHE_LIFETIME), ], 'not_found_redirects' => $helper->getNotFoundRedirectsConfig(), diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index debf021f..46bf1735 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -76,6 +76,7 @@ return [ Service\ShortUrl\ShortUrlResolver::class, Service\VisitsTracker::class, Options\AppOptions::class, + Options\UrlShortenerOptions::class, 'Logger_Shlink', ], Action\PixelAction::class => [ diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 3016b18c..deea5f5f 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -6,12 +6,16 @@ namespace Shlinkio\Shlink\Core; use Cake\Chronos\Chronos; use DateTimeInterface; +use Fig\Http\Message\StatusCodeInterface; use PUGX\Shortid\Factory as ShortIdFactory; use function sprintf; +const DEFAULT_DELETE_SHORT_URL_THRESHOLD = 15; const DEFAULT_SHORT_CODES_LENGTH = 5; const MIN_SHORT_CODES_LENGTH = 4; +const DEFAULT_REDIRECT_STATUS_CODE = StatusCodeInterface::STATUS_FOUND; +const DEFAULT_REDIRECT_CACHE_LIFETIME = 30; const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory'; function generateRandomShortCode(int $length): string diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index e6e1ec4c..80fef898 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -4,18 +4,41 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Action; +use Fig\Http\Message\StatusCodeInterface; use Laminas\Diactoros\Response\RedirectResponse; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; +use Psr\Log\LoggerInterface; +use Shlinkio\Shlink\Core\Options; +use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; +use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; -class RedirectAction extends AbstractTrackingAction +use function sprintf; + +class RedirectAction extends AbstractTrackingAction implements StatusCodeInterface { + private Options\UrlShortenerOptions $urlShortenerOptions; + + public function __construct( + ShortUrlResolverInterface $urlResolver, + VisitsTrackerInterface $visitTracker, + Options\AppOptions $appOptions, + Options\UrlShortenerOptions $urlShortenerOptions, + ?LoggerInterface $logger = null + ) { + parent::__construct($urlResolver, $visitTracker, $appOptions, $logger); + $this->urlShortenerOptions = $urlShortenerOptions; + } + protected function createSuccessResp(string $longUrl): Response { - // Return a redirect response to the long URL. - // Use a temporary redirect to make sure browsers always hit the server for analytics purposes - return new RedirectResponse($longUrl); + $statusCode = $this->urlShortenerOptions->redirectStatusCode(); + $headers = $statusCode === self::STATUS_FOUND ? [] : [ + 'Cache-Control' => sprintf('private,max-age=%s', $this->urlShortenerOptions->redirectCacheLifetime()), + ]; + + return new RedirectResponse($longUrl, $statusCode, $headers); } protected function createErrorResp(ServerRequestInterface $request, RequestHandlerInterface $handler): Response diff --git a/module/Core/src/Config/SimplifiedConfigParser.php b/module/Core/src/Config/SimplifiedConfigParser.php index 81f05d14..38fbdea3 100644 --- a/module/Core/src/Config/SimplifiedConfigParser.php +++ b/module/Core/src/Config/SimplifiedConfigParser.php @@ -38,6 +38,8 @@ class SimplifiedConfigParser 'mercure_internal_hub_url' => ['mercure', 'internal_hub_url'], 'mercure_jwt_secret' => ['mercure', 'jwt_secret'], 'anonymize_remote_addr' => ['url_shortener', 'anonymize_remote_addr'], + 'redirect_status_code' => ['url_shortener', 'redirect_status_code'], + 'redirect_cache_lifetime' => ['url_shortener', 'redirect_cache_lifetime'], ]; private const SIMPLIFIED_CONFIG_SIDE_EFFECTS = [ 'delete_short_url_threshold' => [ diff --git a/module/Core/src/Options/DeleteShortUrlsOptions.php b/module/Core/src/Options/DeleteShortUrlsOptions.php index ee7d9dee..9fa7fcf0 100644 --- a/module/Core/src/Options/DeleteShortUrlsOptions.php +++ b/module/Core/src/Options/DeleteShortUrlsOptions.php @@ -6,9 +6,11 @@ namespace Shlinkio\Shlink\Core\Options; use Laminas\Stdlib\AbstractOptions; +use const Shlinkio\Shlink\Core\DEFAULT_DELETE_SHORT_URL_THRESHOLD; + class DeleteShortUrlsOptions extends AbstractOptions { - private int $visitsThreshold = 15; + private int $visitsThreshold = DEFAULT_DELETE_SHORT_URL_THRESHOLD; private bool $checkVisitsThreshold = true; public function getVisitsThreshold(): int diff --git a/module/Core/src/Options/UrlShortenerOptions.php b/module/Core/src/Options/UrlShortenerOptions.php index 19267ea1..92bb7d07 100644 --- a/module/Core/src/Options/UrlShortenerOptions.php +++ b/module/Core/src/Options/UrlShortenerOptions.php @@ -6,20 +6,53 @@ namespace Shlinkio\Shlink\Core\Options; use Laminas\Stdlib\AbstractOptions; +use function Functional\contains; + +use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_CACHE_LIFETIME; +use const Shlinkio\Shlink\Core\DEFAULT_REDIRECT_STATUS_CODE; + class UrlShortenerOptions extends AbstractOptions { protected $__strictMode__ = false; // phpcs:ignore private bool $validateUrl = true; + private int $redirectStatusCode = DEFAULT_REDIRECT_STATUS_CODE; + private int $redirectCacheLifetime = DEFAULT_REDIRECT_CACHE_LIFETIME; public function isUrlValidationEnabled(): bool { return $this->validateUrl; } - protected function setValidateUrl(bool $validateUrl): self + protected function setValidateUrl(bool $validateUrl): void { $this->validateUrl = $validateUrl; - return $this; + } + + public function redirectStatusCode(): int + { + return $this->redirectStatusCode; + } + + protected function setRedirectStatusCode(int $redirectStatusCode): void + { + $this->redirectStatusCode = $this->normalizeRedirectStatusCode($redirectStatusCode); + } + + private function normalizeRedirectStatusCode(int $statusCode): int + { + return contains([301, 302], $statusCode) ? $statusCode : DEFAULT_REDIRECT_STATUS_CODE; + } + + public function redirectCacheLifetime(): int + { + return $this->redirectCacheLifetime; + } + + protected function setRedirectCacheLifetime(int $redirectCacheLifetime): void + { + $this->redirectCacheLifetime = $redirectCacheLifetime > 0 + ? $redirectCacheLifetime + : DEFAULT_REDIRECT_CACHE_LIFETIME; } } diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index f4de05c5..1a6bb617 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -27,16 +27,19 @@ class RedirectActionTest extends TestCase private RedirectAction $action; private ObjectProphecy $urlResolver; private ObjectProphecy $visitTracker; + private Options\UrlShortenerOptions $shortenerOpts; public function setUp(): void { $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class); $this->visitTracker = $this->prophesize(VisitsTrackerInterface::class); + $this->shortenerOpts = new Options\UrlShortenerOptions(); $this->action = new RedirectAction( $this->urlResolver->reveal(), $this->visitTracker->reveal(), new Options\AppOptions(['disableTrackParam' => 'foobar']), + $this->shortenerOpts, ); } @@ -48,8 +51,9 @@ class RedirectActionTest extends TestCase { $shortCode = 'abc123'; $shortUrl = new ShortUrl('http://domain.com/foo/bar?some=thing'); - $shortCodeToUrl = $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, '')) - ->willReturn($shortUrl); + $shortCodeToUrl = $this->urlResolver->resolveEnabledShortUrl( + new ShortUrlIdentifier($shortCode, ''), + )->willReturn($shortUrl); $track = $this->visitTracker->track(Argument::cetera())->will(function (): void { }); @@ -110,4 +114,40 @@ class RedirectActionTest extends TestCase $track->shouldNotHaveBeenCalled(); } + + /** + * @test + * @dataProvider provideRedirectConfigs + */ + public function expectedStatusCodeAndCacheIsReturnedBasedOnConfig( + int $configuredStatus, + int $configuredLifetime, + int $expectedStatus, + ?string $expectedCacheControl + ): void { + $this->shortenerOpts->redirectStatusCode = $configuredStatus; + $this->shortenerOpts->redirectCacheLifetime = $configuredLifetime; + + $shortUrl = new ShortUrl('http://domain.com/foo/bar'); + $shortCode = $shortUrl->getShortCode(); + $this->urlResolver->resolveEnabledShortUrl(Argument::cetera())->willReturn($shortUrl); + + $request = (new ServerRequest())->withAttribute('shortCode', $shortCode); + $response = $this->action->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal()); + + $this->assertInstanceOf(Response\RedirectResponse::class, $response); + $this->assertEquals($expectedStatus, $response->getStatusCode()); + $this->assertEquals($response->hasHeader('Cache-Control'), $expectedCacheControl !== null); + $this->assertEquals($response->getHeaderLine('Cache-Control'), $expectedCacheControl ?? ''); + } + + public function provideRedirectConfigs(): iterable + { + yield 'status 302' => [302, 20, 302, null]; + yield 'status over 302' => [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']; + } } diff --git a/module/Core/test/Config/SimplifiedConfigParserTest.php b/module/Core/test/Config/SimplifiedConfigParserTest.php index 3700b042..9d4bca69 100644 --- a/module/Core/test/Config/SimplifiedConfigParserTest.php +++ b/module/Core/test/Config/SimplifiedConfigParserTest.php @@ -65,6 +65,8 @@ class SimplifiedConfigParserTest extends TestCase 'mercure_internal_hub_url' => 'internal_url', 'mercure_jwt_secret' => 'super_secret_value', 'anonymize_remote_addr' => false, + 'redirect_status_code' => 301, + 'redirect_cache_lifetime' => 90, ]; $expected = [ 'app_options' => [ @@ -94,6 +96,8 @@ class SimplifiedConfigParserTest extends TestCase ], 'default_short_codes_length' => 8, 'anonymize_remote_addr' => false, + 'redirect_status_code' => 301, + 'redirect_cache_lifetime' => 90, ], 'delete_short_urls' => [