Added config post-processor which sets proper allowed methods based on redirect status codes

This commit is contained in:
Alejandro Celaya 2023-01-07 13:51:35 +01:00
parent a06957e9fa
commit 0c1b36d0d4
4 changed files with 148 additions and 1 deletions

View file

@ -50,4 +50,5 @@ return (new ConfigAggregator\ConfigAggregator([
], 'data/cache/app_config.php', [ ], 'data/cache/app_config.php', [
Core\Config\PostProcessor\BasePathPrefixer::class, Core\Config\PostProcessor\BasePathPrefixer::class,
Core\Config\PostProcessor\MultiSegmentSlugProcessor::class, Core\Config\PostProcessor\MultiSegmentSlugProcessor::class,
Core\Config\PostProcessor\ShortUrlMethodsProcessor::class,
]))->getMergedConfig(); ]))->getMergedConfig();

View file

@ -9,7 +9,7 @@ use Shlinkio\Shlink\Core\Util\RedirectStatus;
const DEFAULT_DELETE_SHORT_URL_THRESHOLD = 15; const DEFAULT_DELETE_SHORT_URL_THRESHOLD = 15;
const DEFAULT_SHORT_CODES_LENGTH = 5; const DEFAULT_SHORT_CODES_LENGTH = 5;
const MIN_SHORT_CODES_LENGTH = 4; 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 DEFAULT_REDIRECT_CACHE_LIFETIME = 30;
const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory'; const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory';
const TITLE_TAG_VALUE = '/<title[^>]*>(.*?)<\/title>/i'; // Matches the value inside a html title tag const TITLE_TAG_VALUE = '/<title[^>]*>(.*?)<\/title>/i'; // Matches the value inside a html title tag

View file

@ -0,0 +1,41 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Config\PostProcessor;
use Fig\Http\Message\RequestMethodInterface;
use Mezzio\Router\Route;
use Shlinkio\Shlink\Core\Action\RedirectAction;
use Shlinkio\Shlink\Core\Util\RedirectStatus;
use function array_values;
use function count;
use function Functional\partition;
use const Shlinkio\Shlink\DEFAULT_REDIRECT_STATUS_CODE;
class ShortUrlMethodsProcessor
{
public function __invoke(array $config): array
{
[$redirectRoutes, $rest] = partition(
$config['routes'] ?? [],
static fn (array $route) => $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;
}
}

View file

@ -0,0 +1,105 @@
<?php
declare(strict_types=1);
namespace ShlinkioTest\Shlink\Core\Config\PostProcessor;
use Mezzio\Router\Route;
use PHPUnit\Framework\TestCase;
use Shlinkio\Shlink\Core\Action\RedirectAction;
use Shlinkio\Shlink\Core\Config\PostProcessor\ShortUrlMethodsProcessor;
class ShortUrlMethodsProcessorTest extends TestCase
{
private ShortUrlMethodsProcessor $processor;
protected function setUp(): void
{
$this->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);
}
}