Merge pull request #2298 from acelaya-forks/feature/ignore-extra-path

Allow the extra path to be ignored when redirecting
shlinkio · Dec 1, 2024 · bfaab6c · bfaab6c
2 parents e74ee79 + d83081f
commit bfaab6c
Show file tree

Hide file tree

Showing 8 changed files with 70 additions and 20 deletions.
diff --git 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
@@ -47,7 +47,7 @@
         "shlinkio/shlink-config": "^3.4",
         "shlinkio/shlink-event-dispatcher": "^4.1",
         "shlinkio/shlink-importer": "^5.3.2",
-        "shlinkio/shlink-installer": "^9.3",
+        "shlinkio/shlink-installer": "dev-develop#957db97 as 9.4",
         "shlinkio/shlink-ip-geolocation": "^4.2",
         "shlinkio/shlink-json": "^1.1",
         "spiral/roadrunner": "^2024.1",
@@ -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/config/autoload/installer.global.php b/config/autoload/installer.global.php
@@ -41,7 +41,7 @@
             Option\UrlShortener\RedirectStatusCodeConfigOption::class,
             Option\UrlShortener\RedirectCacheLifeTimeConfigOption::class,
             Option\UrlShortener\AutoResolveTitlesConfigOption::class,
-            Option\UrlShortener\AppendExtraPathConfigOption::class,
+            Option\UrlShortener\ExtraPathModeConfigOption::class,
             Option\UrlShortener\EnableMultiSegmentSlugsConfigOption::class,
             Option\UrlShortener\EnableTrailingSlashConfigOption::class,
             Option\UrlShortener\ShortUrlModeConfigOption::class,

diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php
@@ -84,14 +84,16 @@ 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';
     case TIMEZONE = 'TIMEZONE';
     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 @@ private function defaultValue(): string|int|bool|null
             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
@@ -0,0 +1,13 @@
+<?php
+
+namespace Shlinkio\Shlink\Core\Config\Options;
+
+enum ExtraPathMode: string
+{
+    /** URLs with extra path will not match a short URL */
+    case DEFAULT = 'default';
+    /** The extra path will be appended to the long URL */
+    case APPEND = 'append';
+    /** The extra path will be ignored */
+    case IGNORE = 'ignore';
+}
diff --git a/module/Core/src/Config/Options/UrlShortenerOptions.php b/module/Core/src/Config/Options/UrlShortenerOptions.php
@@ -22,10 +22,10 @@ public function __construct(
         public string $schema = 'http',
         public int $defaultShortCodesLength = DEFAULT_SHORT_CODES_LENGTH,
         public bool $autoResolveTitles = false,
-        public bool $appendExtraPath = false,
         public bool $multiSegmentSlugsEnabled = false,
         public bool $trailingSlashEnabled = false,
         public ShortUrlMode $mode = ShortUrlMode::STRICT,
+        public ExtraPathMode $extraPathMode = ExtraPathMode::DEFAULT,
     ) {
     }
 
@@ -35,17 +35,26 @@ public static function fromEnv(): self
             (int) EnvVars::DEFAULT_SHORT_CODES_LENGTH->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
@@ -9,6 +9,7 @@
 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 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
 
     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 @@ private function tryToResolveRedirect(
 
         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
@@ -11,11 +11,13 @@
 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 @@ public function handlerIsCalledWhenConfigPreventsRedirectWithExtraPath(
         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 @@ public static function provideNonRedirectingRequests(): iterable
         ];
     }
 
-    #[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 @@ public function handlerIsCalledWhenNoShortUrlIsFoundAfterExpectedAmountOfIterati
 
     #[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 @@ function () use ($shortUrl, &$currentIteration, $expectedResolveCalls): ShortUrl
 
     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 @@ private function middleware(UrlShortenerOptions|null $options = null): ExtraPath
             $this->requestTracker,
             $this->redirectionBuilder,
             $this->redirectResponseHelper,
-            $options ?? new UrlShortenerOptions(appendExtraPath: true),
+            $options ?? new UrlShortenerOptions(extraPathMode: ExtraPathMode::APPEND),
         );
     }
 }