diff --git a/config/config.php b/config/config.php index a002c329..8fe311a0 100644 --- a/config/config.php +++ b/config/config.php @@ -50,4 +50,5 @@ return (new ConfigAggregator\ConfigAggregator([ ], 'data/cache/app_config.php', [ Core\Config\PostProcessor\BasePathPrefixer::class, Core\Config\PostProcessor\MultiSegmentSlugProcessor::class, + Core\Config\PostProcessor\ShortUrlMethodsProcessor::class, ]))->getMergedConfig(); diff --git a/config/constants.php b/config/constants.php index f6d5e9aa..5c891a34 100644 --- a/config/constants.php +++ b/config/constants.php @@ -9,7 +9,7 @@ use Shlinkio\Shlink\Core\Util\RedirectStatus; const DEFAULT_DELETE_SHORT_URL_THRESHOLD = 15; const DEFAULT_SHORT_CODES_LENGTH = 5; const MIN_SHORT_CODES_LENGTH = 4; -const DEFAULT_REDIRECT_STATUS_CODE = RedirectStatus::STATUS_302; +const DEFAULT_REDIRECT_STATUS_CODE = RedirectStatus::STATUS_302; // Deprecated. Default to 307 for Shlink v4 const DEFAULT_REDIRECT_CACHE_LIFETIME = 30; const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory'; const TITLE_TAG_VALUE = '/]*>(.*?)<\/title>/i'; // Matches the value inside a html title tag diff --git a/module/Core/src/Config/PostProcessor/ShortUrlMethodsProcessor.php b/module/Core/src/Config/PostProcessor/ShortUrlMethodsProcessor.php new file mode 100644 index 00000000..05ecdb6c --- /dev/null +++ b/module/Core/src/Config/PostProcessor/ShortUrlMethodsProcessor.php @@ -0,0 +1,41 @@ + $route['name'] === RedirectAction::class, + ); + if (count($redirectRoutes) === 0) { + return $config; + } + + [$redirectRoute] = array_values($redirectRoutes); + $redirectStatus = RedirectStatus::tryFrom( + $config['redirects']['redirect_status_code'] ?? 0, + ) ?? DEFAULT_REDIRECT_STATUS_CODE; + $redirectRoute['allowed_methods'] = $redirectStatus->isLegacyStatus() + ? [RequestMethodInterface::METHOD_GET] + : Route::HTTP_METHOD_ANY; + + $config['routes'] = [...$rest, $redirectRoute]; + return $config; + } +} diff --git a/module/Core/test/Config/PostProcessor/ShortUrlMethodsProcessorTest.php b/module/Core/test/Config/PostProcessor/ShortUrlMethodsProcessorTest.php new file mode 100644 index 00000000..b73253f7 --- /dev/null +++ b/module/Core/test/Config/PostProcessor/ShortUrlMethodsProcessorTest.php @@ -0,0 +1,105 @@ +processor = new ShortUrlMethodsProcessor(); + } + + /** + * @test + * @dataProvider provideConfigs + */ + public function onlyFirstRouteIdentifiedAsRedirectIsEditedWithProperAllowedMethods( + array $config, + ?array $expectedRoutes, + ): void { + self::assertEquals($expectedRoutes, ($this->processor)($config)['routes'] ?? null); + } + + public function provideConfigs(): iterable + { + $buildConfigWithStatus = static fn (int $status, ?array $expectedAllowedMethods) => [[ + 'routes' => [ + ['name' => 'foo'], + ['name' => 'bar'], + ['name' => RedirectAction::class], + ], + 'redirects' => [ + 'redirect_status_code' => $status, + ], + ], [ + ['name' => 'foo'], + ['name' => 'bar'], + [ + 'name' => RedirectAction::class, + 'allowed_methods' => $expectedAllowedMethods, + ], + ]]; + + yield 'empty config' => [[], null]; + yield 'empty routes' => [['routes' => []], []]; + yield 'no redirects route' => [['routes' => $routes = [ + ['name' => 'foo'], + ['name' => 'bar'], + ]], $routes]; + yield 'one redirects route' => [['routes' => [ + ['name' => 'foo'], + ['name' => 'bar'], + ['name' => RedirectAction::class], + ]], [ + ['name' => 'foo'], + ['name' => 'bar'], + [ + 'name' => RedirectAction::class, + 'allowed_methods' => ['GET'], + ], + ]]; + yield 'one redirects route in different location' => [['routes' => [ + [ + 'name' => RedirectAction::class, + 'allowed_methods' => ['POST'], + ], + ['name' => 'foo'], + ['name' => 'bar'], + ]], [ + ['name' => 'foo'], + ['name' => 'bar'], + [ + 'name' => RedirectAction::class, + 'allowed_methods' => ['GET'], + ], + ]]; + yield 'multiple redirects routes' => [['routes' => [ + ['name' => RedirectAction::class], + ['name' => 'foo'], + ['name' => 'bar'], + ['name' => RedirectAction::class], + ['name' => RedirectAction::class], + ]], [ + ['name' => 'foo'], + ['name' => 'bar'], + [ + 'name' => RedirectAction::class, + 'allowed_methods' => ['GET'], + ], + ]]; + yield 'one redirects route with invalid status code' => $buildConfigWithStatus(500, ['GET']); + yield 'one redirects route with 302 status code' => $buildConfigWithStatus(302, ['GET']); + yield 'one redirects route with 301 status code' => $buildConfigWithStatus(301, ['GET']); + yield 'one redirects route with 307 status code' => $buildConfigWithStatus(307, Route::HTTP_METHOD_ANY); + yield 'one redirects route with 308 status code' => $buildConfigWithStatus(308, Route::HTTP_METHOD_ANY); + } +}