Ensured multi-segment feature flag affects how append_extra_path is checked

This commit is contained in:
Alejandro Celaya 2022-08-04 16:10:54 +02:00
parent a3de3e15cb
commit 3d5ddce621
4 changed files with 47 additions and 9 deletions

View File

@ -23,6 +23,7 @@ return (static function (): array {
'default_short_codes_length' => $shortCodesLength,
'auto_resolve_titles' => (bool) EnvVars::AUTO_RESOLVE_TITLES->loadFromEnv(false),
'append_extra_path' => (bool) EnvVars::REDIRECT_APPEND_EXTRA_PATH->loadFromEnv(false),
'multi_segment_slugs_enabled' => (bool) EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->loadFromEnv(false),
],
];

View File

@ -12,6 +12,7 @@ class UrlShortenerOptions extends AbstractOptions
private bool $autoResolveTitles = false;
private bool $appendExtraPath = false;
private bool $multiSegmentSlugsEnabled = false;
public function autoResolveTitles(): bool
{
@ -32,4 +33,14 @@ class UrlShortenerOptions extends AbstractOptions
{
$this->appendExtraPath = $appendExtraPath;
}
public function multiSegmentSlugsEnabled(): bool
{
return $this->multiSegmentSlugsEnabled;
}
protected function setMultiSegmentSlugsEnabled(bool $multiSegmentSlugsEnabled): void
{
$this->multiSegmentSlugsEnabled = $multiSegmentSlugsEnabled;
}
}

View File

@ -38,9 +38,7 @@ class ExtraPathRedirectMiddleware implements MiddlewareInterface
{
/** @var NotFoundType|null $notFoundType */
$notFoundType = $request->getAttribute(NotFoundType::class);
// This logic is applied only if actively opted in and current URL is potentially /{shortCode}/[...]
if (! $notFoundType?->isRegularNotFound() || ! $this->urlShortenerOptions->appendExtraPath()) {
if (! $this->shouldApplyLogic($notFoundType)) {
return $handler->handle($request);
}
@ -61,6 +59,21 @@ class ExtraPathRedirectMiddleware implements MiddlewareInterface
}
}
private function shouldApplyLogic(?NotFoundType $notFoundType): bool
{
if ($notFoundType === null || ! $this->urlShortenerOptions->appendExtraPath()) {
return false;
}
return (
// If multi-segment slugs are enabled, the appropriate not-found type is "invalid_short_url"
$this->urlShortenerOptions->multiSegmentSlugsEnabled() && $notFoundType->isInvalidShortUrl()
) || (
// If multi-segment slugs are disabled, the appropriate not-found type is "regular_404"
! $this->urlShortenerOptions->multiSegmentSlugsEnabled() && $notFoundType->isRegularNotFound()
);
}
/**
* @return array{0: string, 1: string|null}
*/

View File

@ -16,6 +16,7 @@ use Prophecy\Prophecy\ObjectProphecy;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;
use Shlinkio\Shlink\Core\Action\RedirectAction;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\ErrorHandler\Model\NotFoundType;
use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
@ -65,9 +66,11 @@ class ExtraPathRedirectMiddlewareTest extends TestCase
*/
public function handlerIsCalledWhenConfigPreventsRedirectWithExtraPath(
bool $appendExtraPath,
bool $multiSegmentEnabled,
ServerRequestInterface $request,
): void {
$this->options->appendExtraPath = $appendExtraPath;
$this->options->multiSegmentSlugsEnabled = $multiSegmentEnabled;
$this->middleware->process($request, $this->handler->reveal());
@ -83,20 +86,30 @@ class ExtraPathRedirectMiddlewareTest extends TestCase
$buildReq = static fn (?NotFoundType $type): ServerRequestInterface =>
$baseReq->withAttribute(NotFoundType::class, $type);
yield 'disabled option' => [false, $buildReq(NotFoundType::fromRequest($baseReq, '/foo/bar'))];
yield 'base_url error' => [true, $buildReq(NotFoundType::fromRequest($baseReq, ''))];
yield 'disabled option' => [false, false, $buildReq(NotFoundType::fromRequest($baseReq, '/foo/bar'))];
yield 'no error type' => [true, false, $buildReq(null)];
yield 'base_url error' => [true, false, $buildReq(NotFoundType::fromRequest($baseReq, ''))];
yield 'invalid_short_url error' => [
true,
$buildReq(NotFoundType::fromRequest($baseReq, ''))->withAttribute(
false,
$buildReq(NotFoundType::fromRequest($baseReq->withUri(new Uri('/foo'))->withAttribute(
RouteResult::class,
RouteResult::fromRoute(new Route(
'',
'/foo',
$this->prophesize(MiddlewareInterface::class)->reveal(),
['GET'],
RedirectAction::class,
)),
),
), '')),
];
yield 'regular_404 error with multi-segment slugs' => [
true,
true,
$buildReq(NotFoundType::fromRequest($baseReq->withUri(new Uri('/foo'))->withAttribute(
RouteResult::class,
RouteResult::fromRouteFailure(['GET']),
), '')),
];
yield 'no error type' => [true, $buildReq(null)];
}
/** @test */