From c65349d265b05de40fe86f9cf83ac7c2a47bc54d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 1 Dec 2024 09:51:00 +0100 Subject: [PATCH] Allow the extra path to be ignored when redirecting --- CHANGELOG.md | 8 +++++- composer.json | 4 +-- module/Core/src/Config/EnvVars.php | 8 ++++-- .../Core/src/Config/Options/ExtraPathMode.php | 13 +++++++++ .../Config/Options/UrlShortenerOptions.php | 17 +++++++++--- .../ExtraPathRedirectMiddleware.php | 9 +++++-- .../ExtraPathRedirectMiddlewareTest.php | 27 ++++++++++++++----- 7 files changed, 68 insertions(+), 18 deletions(-) create mode 100644 module/Core/src/Config/Options/ExtraPathMode.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 19320ac3..fd63464d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this # [Unreleased] ### Added -* *Nothing* +* [#2265](https://github.com/shlinkio/shlink/issues/2265) Add a new `REDIRECT_EXTRA_PATH_MODE` option that accepts three values: + + * `default`: Short URLs only match if the path matches their short code or custom slug. + * `append`: Short URLs are matched as soon as the path starts with the short code or custom slug, and the extra path is appended to the long URL before redirecting. + * `ignore`: Short URLs are matched as soon as the path starts with the short code or custom slug, and the extra path is ignored. + + This option effectively replaces the old `REDIRECT_APPEND_EXTRA_PATH` option, which is now deprecated and will be removed in Shlink 5.0.0 ### Changed * * [#2281](https://github.com/shlinkio/shlink/issues/2281) Update docker image to PHP 8.4 diff --git a/composer.json b/composer.json index fa287271..2138cefb 100644 --- a/composer.json +++ b/composer.json @@ -154,8 +154,8 @@ "@test:cli", "phpcov merge build/coverage-cli --html build/coverage-cli/coverage-html && rm build/coverage-cli/*.cov" ], - "swagger:validate": "php-openapi validate docs/swagger/swagger.json", - "swagger:inline": "php-openapi inline docs/swagger/swagger.json docs/swagger/swagger-inlined.json", + "swagger:validate": "@php -d error_reporting=\"E_ALL & ~E_DEPRECATED & ~E_USER_DEPRECATED\" vendor/bin/php-openapi validate docs/swagger/swagger.json", + "swagger:inline": "@php -d error_reporting=\"E_ALL & ~E_DEPRECATED & ~E_USER_DEPRECATED\" vendor/bin/php-openapi inline docs/swagger/swagger.json docs/swagger/swagger-inlined.json", "clean:dev": "rm -f data/database.sqlite && rm -f config/params/generated_config.php" }, "scripts-descriptions": { diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index 6cdf6297..e2a6d38a 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -84,7 +84,7 @@ enum EnvVars: string case IS_HTTPS_ENABLED = 'IS_HTTPS_ENABLED'; case DEFAULT_DOMAIN = 'DEFAULT_DOMAIN'; case AUTO_RESOLVE_TITLES = 'AUTO_RESOLVE_TITLES'; - case REDIRECT_APPEND_EXTRA_PATH = 'REDIRECT_APPEND_EXTRA_PATH'; + case REDIRECT_EXTRA_PATH_MODE = 'REDIRECT_EXTRA_PATH_MODE'; case MULTI_SEGMENT_SLUGS_ENABLED = 'MULTI_SEGMENT_SLUGS_ENABLED'; case ROBOTS_ALLOW_ALL_SHORT_URLS = 'ROBOTS_ALLOW_ALL_SHORT_URLS'; case ROBOTS_USER_AGENTS = 'ROBOTS_USER_AGENTS'; @@ -92,6 +92,8 @@ enum EnvVars: string case MEMORY_LIMIT = 'MEMORY_LIMIT'; case INITIAL_API_KEY = 'INITIAL_API_KEY'; case SKIP_INITIAL_GEOLITE_DOWNLOAD = 'SKIP_INITIAL_GEOLITE_DOWNLOAD'; + /** @deprecated Use REDIRECT_EXTRA_PATH */ + case REDIRECT_APPEND_EXTRA_PATH = 'REDIRECT_APPEND_EXTRA_PATH'; public function loadFromEnv(): mixed { @@ -125,11 +127,13 @@ enum EnvVars: string self::DEFAULT_SHORT_CODES_LENGTH => DEFAULT_SHORT_CODES_LENGTH, self::SHORT_URL_MODE => ShortUrlMode::STRICT->value, self::IS_HTTPS_ENABLED, self::AUTO_RESOLVE_TITLES => true, - self::REDIRECT_APPEND_EXTRA_PATH, self::MULTI_SEGMENT_SLUGS_ENABLED, self::SHORT_URL_TRAILING_SLASH => false, self::DEFAULT_DOMAIN, self::BASE_PATH => '', self::CACHE_NAMESPACE => 'Shlink', + // Deprecated. In Shlink 5.0.0, add default value for REDIRECT_EXTRA_PATH_MODE + self::REDIRECT_APPEND_EXTRA_PATH => false, + // self::REDIRECT_EXTRA_PATH_MODE => ExtraPathMode::DEFAULT->value, self::REDIS_PUB_SUB_ENABLED, self::MATOMO_ENABLED, diff --git a/module/Core/src/Config/Options/ExtraPathMode.php b/module/Core/src/Config/Options/ExtraPathMode.php new file mode 100644 index 00000000..4904ca45 --- /dev/null +++ b/module/Core/src/Config/Options/ExtraPathMode.php @@ -0,0 +1,13 @@ +loadFromEnv(), MIN_SHORT_CODES_LENGTH, ); - $mode = EnvVars::SHORT_URL_MODE->loadFromEnv(); + + // Deprecated. Initialize extra path from REDIRECT_APPEND_EXTRA_PATH. + $appendExtraPath = EnvVars::REDIRECT_APPEND_EXTRA_PATH->loadFromEnv(); + $extraPathMode = $appendExtraPath ? ExtraPathMode::APPEND : ExtraPathMode::DEFAULT; + + // If REDIRECT_EXTRA_PATH_MODE was explicitly provided, it has precedence + $extraPathModeFromEnv = EnvVars::REDIRECT_EXTRA_PATH_MODE->loadFromEnv(); + if ($extraPathModeFromEnv !== null) { + $extraPathMode = ExtraPathMode::tryFrom($extraPathModeFromEnv) ?? ExtraPathMode::DEFAULT; + } return new self( defaultDomain: EnvVars::DEFAULT_DOMAIN->loadFromEnv(), schema: ((bool) EnvVars::IS_HTTPS_ENABLED->loadFromEnv()) ? 'https' : 'http', defaultShortCodesLength: $shortCodesLength, autoResolveTitles: (bool) EnvVars::AUTO_RESOLVE_TITLES->loadFromEnv(), - appendExtraPath: (bool) EnvVars::REDIRECT_APPEND_EXTRA_PATH->loadFromEnv(), multiSegmentSlugsEnabled: (bool) EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->loadFromEnv(), trailingSlashEnabled: (bool) EnvVars::SHORT_URL_TRAILING_SLASH->loadFromEnv(), - mode: ShortUrlMode::tryFrom($mode) ?? ShortUrlMode::STRICT, + mode: ShortUrlMode::tryFrom(EnvVars::SHORT_URL_MODE->loadFromEnv()) ?? ShortUrlMode::STRICT, + extraPathMode: $extraPathMode, ); } diff --git a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php index 4b013b33..f101774b 100644 --- a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php +++ b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php @@ -9,6 +9,7 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\UriInterface; use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; +use Shlinkio\Shlink\Core\Config\Options\ExtraPathMode; use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; @@ -51,7 +52,7 @@ readonly class ExtraPathRedirectMiddleware implements MiddlewareInterface private function shouldApplyLogic(NotFoundType|null $notFoundType): bool { - if ($notFoundType === null || ! $this->urlShortenerOptions->appendExtraPath) { + if ($notFoundType === null || $this->urlShortenerOptions->extraPathMode === ExtraPathMode::DEFAULT) { return false; } @@ -75,7 +76,11 @@ readonly class ExtraPathRedirectMiddleware implements MiddlewareInterface try { $shortUrl = $this->resolver->resolveEnabledShortUrl($identifier); - $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $request, $extraPath); + $longUrl = $this->redirectionBuilder->buildShortUrlRedirect( + $shortUrl, + $request, + $this->urlShortenerOptions->extraPathMode === ExtraPathMode::APPEND ? $extraPath : null, + ); $this->requestTracker->trackIfApplicable( $shortUrl, $request->withAttribute(REDIRECT_URL_REQUEST_ATTRIBUTE, $longUrl), diff --git a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php index 84ceb790..0578c1f8 100644 --- a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php +++ b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php @@ -11,11 +11,13 @@ use Mezzio\Router\Route; use Mezzio\Router\RouteResult; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Action\RedirectAction; +use Shlinkio\Shlink\Core\Config\Options\ExtraPathMode; use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; @@ -57,8 +59,8 @@ class ExtraPathRedirectMiddlewareTest extends TestCase ServerRequestInterface $request, ): void { $options = new UrlShortenerOptions( - appendExtraPath: $appendExtraPath, multiSegmentSlugsEnabled: $multiSegmentEnabled, + extraPathMode: $appendExtraPath ? ExtraPathMode::APPEND : ExtraPathMode::DEFAULT, ); $this->resolver->expects($this->never())->method('resolveEnabledShortUrl'); $this->requestTracker->expects($this->never())->method('trackIfApplicable'); @@ -102,12 +104,17 @@ class ExtraPathRedirectMiddlewareTest extends TestCase ]; } - #[Test, DataProvider('provideResolves')] + #[Test] + #[TestWith(['multiSegmentEnabled' => false, 'expectedResolveCalls' => 1])] + #[TestWith(['multiSegmentEnabled' => true, 'expectedResolveCalls' => 3])] public function handlerIsCalledWhenNoShortUrlIsFoundAfterExpectedAmountOfIterations( bool $multiSegmentEnabled, int $expectedResolveCalls, ): void { - $options = new UrlShortenerOptions(appendExtraPath: true, multiSegmentSlugsEnabled: $multiSegmentEnabled); + $options = new UrlShortenerOptions( + multiSegmentSlugsEnabled: $multiSegmentEnabled, + extraPathMode: ExtraPathMode::APPEND, + ); $type = $this->createMock(NotFoundType::class); $type->method('isRegularNotFound')->willReturn(true); @@ -127,11 +134,15 @@ class ExtraPathRedirectMiddlewareTest extends TestCase #[Test, DataProvider('provideResolves')] public function visitIsTrackedAndRedirectIsReturnedWhenShortUrlIsFoundAfterExpectedAmountOfIterations( + ExtraPathMode $extraPathMode, bool $multiSegmentEnabled, int $expectedResolveCalls, string|null $expectedExtraPath, ): void { - $options = new UrlShortenerOptions(appendExtraPath: true, multiSegmentSlugsEnabled: $multiSegmentEnabled); + $options = new UrlShortenerOptions( + multiSegmentSlugsEnabled: $multiSegmentEnabled, + extraPathMode: $extraPathMode, + ); $type = $this->createMock(NotFoundType::class); $type->method('isRegularNotFound')->willReturn(true); @@ -171,8 +182,10 @@ class ExtraPathRedirectMiddlewareTest extends TestCase public static function provideResolves(): iterable { - yield [false, 1, '/bar/baz']; - yield [true, 3, null]; + yield [ExtraPathMode::APPEND, false, 1, '/bar/baz']; + yield [ExtraPathMode::APPEND, true, 3, null]; + yield [ExtraPathMode::IGNORE, false, 1, null]; + yield [ExtraPathMode::IGNORE, true, 3, null]; } private function middleware(UrlShortenerOptions|null $options = null): ExtraPathRedirectMiddleware @@ -182,7 +195,7 @@ class ExtraPathRedirectMiddlewareTest extends TestCase $this->requestTracker, $this->redirectionBuilder, $this->redirectResponseHelper, - $options ?? new UrlShortenerOptions(appendExtraPath: true), + $options ?? new UrlShortenerOptions(extraPathMode: ExtraPathMode::APPEND), ); } }