Added validateUrl optional flag for create/edit short URLs

This commit is contained in:
Alejandro Celaya 2020-09-23 19:19:17 +02:00
parent 1f78f5266a
commit d5eac3b1c3
11 changed files with 71 additions and 29 deletions

View File

@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core;
use Cake\Chronos\Chronos; use Cake\Chronos\Chronos;
use DateTimeInterface; use DateTimeInterface;
use Fig\Http\Message\StatusCodeInterface; use Fig\Http\Message\StatusCodeInterface;
use Laminas\InputFilter\InputFilter;
use PUGX\Shortid\Factory as ShortIdFactory; use PUGX\Shortid\Factory as ShortIdFactory;
use function sprintf; use function sprintf;
@ -62,3 +63,15 @@ function determineTableName(string $tableName, array $emConfig = []): string
return sprintf('%s.%s', $schema, $tableName); return sprintf('%s.%s', $schema, $tableName);
} }
function getOptionalIntFromInputFilter(InputFilter $inputFilter, string $fieldName): ?int
{
$value = $inputFilter->getValue($fieldName);
return $value !== null ? (int) $value : null;
}
function getOptionalBoolFromInputFilter(InputFilter $inputFilter, string $fieldName): ?bool
{
$value = $inputFilter->getValue($fieldName);
return $value !== null ? (bool) $value : null;
}

View File

@ -9,6 +9,8 @@ use Shlinkio\Shlink\Core\Exception\ValidationException;
use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter; use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
use function array_key_exists; use function array_key_exists;
use function Shlinkio\Shlink\Core\getOptionalBoolFromInputFilter;
use function Shlinkio\Shlink\Core\getOptionalIntFromInputFilter;
use function Shlinkio\Shlink\Core\parseDateField; use function Shlinkio\Shlink\Core\parseDateField;
final class ShortUrlEdit final class ShortUrlEdit
@ -21,6 +23,7 @@ final class ShortUrlEdit
private ?Chronos $validUntil = null; private ?Chronos $validUntil = null;
private bool $maxVisitsPropWasProvided = false; private bool $maxVisitsPropWasProvided = false;
private ?int $maxVisits = null; private ?int $maxVisits = null;
private ?bool $validateUrl = null;
// Enforce named constructors // Enforce named constructors
private function __construct() private function __construct()
@ -55,13 +58,8 @@ final class ShortUrlEdit
$this->longUrl = $inputFilter->getValue(ShortUrlMetaInputFilter::LONG_URL); $this->longUrl = $inputFilter->getValue(ShortUrlMetaInputFilter::LONG_URL);
$this->validSince = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_SINCE)); $this->validSince = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_SINCE));
$this->validUntil = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_UNTIL)); $this->validUntil = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_UNTIL));
$this->maxVisits = $this->getOptionalIntFromInputFilter($inputFilter, ShortUrlMetaInputFilter::MAX_VISITS); $this->maxVisits = getOptionalIntFromInputFilter($inputFilter, ShortUrlMetaInputFilter::MAX_VISITS);
} $this->validateUrl = getOptionalBoolFromInputFilter($inputFilter, ShortUrlMetaInputFilter::VALIDATE_URL);
private function getOptionalIntFromInputFilter(ShortUrlMetaInputFilter $inputFilter, string $fieldName): ?int
{
$value = $inputFilter->getValue($fieldName);
return $value !== null ? (int) $value : null;
} }
public function longUrl(): ?string public function longUrl(): ?string
@ -103,4 +101,9 @@ final class ShortUrlEdit
{ {
return $this->maxVisitsPropWasProvided; return $this->maxVisitsPropWasProvided;
} }
public function doValidateUrl(): ?bool
{
return $this->validateUrl;
}
} }

View File

@ -8,6 +8,8 @@ use Cake\Chronos\Chronos;
use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Exception\ValidationException;
use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter; use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
use function Shlinkio\Shlink\Core\getOptionalBoolFromInputFilter;
use function Shlinkio\Shlink\Core\getOptionalIntFromInputFilter;
use function Shlinkio\Shlink\Core\parseDateField; use function Shlinkio\Shlink\Core\parseDateField;
use const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH; use const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH;
@ -21,6 +23,7 @@ final class ShortUrlMeta
private ?bool $findIfExists = null; private ?bool $findIfExists = null;
private ?string $domain = null; private ?string $domain = null;
private int $shortCodeLength = 5; private int $shortCodeLength = 5;
private ?bool $validateUrl = null;
// Enforce named constructors // Enforce named constructors
private function __construct() private function __construct()
@ -55,21 +58,16 @@ final class ShortUrlMeta
$this->validSince = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_SINCE)); $this->validSince = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_SINCE));
$this->validUntil = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_UNTIL)); $this->validUntil = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_UNTIL));
$this->customSlug = $inputFilter->getValue(ShortUrlMetaInputFilter::CUSTOM_SLUG); $this->customSlug = $inputFilter->getValue(ShortUrlMetaInputFilter::CUSTOM_SLUG);
$this->maxVisits = $this->getOptionalIntFromInputFilter($inputFilter, ShortUrlMetaInputFilter::MAX_VISITS); $this->maxVisits = getOptionalIntFromInputFilter($inputFilter, ShortUrlMetaInputFilter::MAX_VISITS);
$this->findIfExists = $inputFilter->getValue(ShortUrlMetaInputFilter::FIND_IF_EXISTS); $this->findIfExists = $inputFilter->getValue(ShortUrlMetaInputFilter::FIND_IF_EXISTS);
$this->validateUrl = getOptionalBoolFromInputFilter($inputFilter, ShortUrlMetaInputFilter::VALIDATE_URL);
$this->domain = $inputFilter->getValue(ShortUrlMetaInputFilter::DOMAIN); $this->domain = $inputFilter->getValue(ShortUrlMetaInputFilter::DOMAIN);
$this->shortCodeLength = $this->getOptionalIntFromInputFilter( $this->shortCodeLength = getOptionalIntFromInputFilter(
$inputFilter, $inputFilter,
ShortUrlMetaInputFilter::SHORT_CODE_LENGTH, ShortUrlMetaInputFilter::SHORT_CODE_LENGTH,
) ?? DEFAULT_SHORT_CODES_LENGTH; ) ?? DEFAULT_SHORT_CODES_LENGTH;
} }
private function getOptionalIntFromInputFilter(ShortUrlMetaInputFilter $inputFilter, string $fieldName): ?int
{
$value = $inputFilter->getValue($fieldName);
return $value !== null ? (int) $value : null;
}
public function getValidSince(): ?Chronos public function getValidSince(): ?Chronos
{ {
return $this->validSince; return $this->validSince;
@ -129,4 +127,9 @@ final class ShortUrlMeta
{ {
return $this->shortCodeLength; return $this->shortCodeLength;
} }
public function doValidateUrl(): ?bool
{
return $this->validateUrl;
}
} }

View File

@ -71,7 +71,7 @@ class ShortUrlService implements ShortUrlServiceInterface
public function updateMetadataByShortCode(ShortUrlIdentifier $identifier, ShortUrlEdit $shortUrlEdit): ShortUrl public function updateMetadataByShortCode(ShortUrlIdentifier $identifier, ShortUrlEdit $shortUrlEdit): ShortUrl
{ {
if ($shortUrlEdit->hasLongUrl()) { if ($shortUrlEdit->hasLongUrl()) {
$this->urlValidator->validateUrl($shortUrlEdit->longUrl()); $this->urlValidator->validateUrl($shortUrlEdit->longUrl(), $shortUrlEdit->doValidateUrl());
} }
$shortUrl = $this->urlResolver->resolveShortUrl($identifier); $shortUrl = $this->urlResolver->resolveShortUrl($identifier);

View File

@ -48,7 +48,7 @@ class UrlShortener implements UrlShortenerInterface
return $existingShortUrl; return $existingShortUrl;
} }
$this->urlValidator->validateUrl($url); $this->urlValidator->validateUrl($url, $meta->doValidateUrl());
$this->em->beginTransaction(); $this->em->beginTransaction();
$shortUrl = new ShortUrl($url, $meta, $this->domainResolver); $shortUrl = new ShortUrl($url, $meta, $this->domainResolver);
$shortUrl->setTags($this->tagNamesToEntities($this->em, $tags)); $shortUrl->setTags($this->tagNamesToEntities($this->em, $tags));

View File

@ -27,10 +27,11 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface
/** /**
* @throws InvalidUrlException * @throws InvalidUrlException
*/ */
public function validateUrl(string $url): void public function validateUrl(string $url, ?bool $doValidate): void
{ {
// If the URL validation is not enabled, skip check // If the URL validation is not enabled or it was explicitly set to not validate, skip check
if (! $this->options->isUrlValidationEnabled()) { $doValidate = $doValidate ?? $this->options->isUrlValidationEnabled();
if (! $doValidate) {
return; return;
} }

View File

@ -11,5 +11,5 @@ interface UrlValidatorInterface
/** /**
* @throws InvalidUrlException * @throws InvalidUrlException
*/ */
public function validateUrl(string $url): void; public function validateUrl(string $url, ?bool $doValidate): void;
} }

View File

@ -27,6 +27,7 @@ class ShortUrlMetaInputFilter extends InputFilter
public const DOMAIN = 'domain'; public const DOMAIN = 'domain';
public const SHORT_CODE_LENGTH = 'shortCodeLength'; public const SHORT_CODE_LENGTH = 'shortCodeLength';
public const LONG_URL = 'longUrl'; public const LONG_URL = 'longUrl';
public const VALIDATE_URL = 'validateUrl';
public function __construct(array $data) public function __construct(array $data)
{ {
@ -64,6 +65,8 @@ class ShortUrlMetaInputFilter extends InputFilter
$this->add($this->createBooleanInput(self::FIND_IF_EXISTS, false)); $this->add($this->createBooleanInput(self::FIND_IF_EXISTS, false));
$this->add($this->createInput(self::VALIDATE_URL, false));
$domain = $this->createInput(self::DOMAIN, false); $domain = $this->createInput(self::DOMAIN, false);
$domain->getValidatorChain()->attach(new Validation\HostAndPortValidator()); $domain->getValidatorChain()->attach(new Validation\HostAndPortValidator());
$this->add($domain); $this->add($domain);

View File

@ -104,7 +104,10 @@ class ShortUrlServiceTest extends TestCase
$this->assertEquals($shortUrlEdit->longUrl() ?? $originalLongUrl, $shortUrl->getLongUrl()); $this->assertEquals($shortUrlEdit->longUrl() ?? $originalLongUrl, $shortUrl->getLongUrl());
$findShortUrl->shouldHaveBeenCalled(); $findShortUrl->shouldHaveBeenCalled();
$flush->shouldHaveBeenCalled(); $flush->shouldHaveBeenCalled();
$this->urlValidator->validateUrl($shortUrlEdit->longUrl())->shouldHaveBeenCalledTimes($expectedValidateCalls); $this->urlValidator->validateUrl(
$shortUrlEdit->longUrl(),
$shortUrlEdit->doValidateUrl(),
)->shouldHaveBeenCalledTimes($expectedValidateCalls);
} }
public function provideShortUrlEdits(): iterable public function provideShortUrlEdits(): iterable
@ -123,5 +126,11 @@ class ShortUrlServiceTest extends TestCase
'longUrl' => 'modifiedLongUrl', 'longUrl' => 'modifiedLongUrl',
], ],
)]; )];
yield 'long URL with validation' => [1, ShortUrlEdit::fromRawData(
[
'longUrl' => 'modifiedLongUrl',
'validateUrl' => true,
],
)];
} }
} }

View File

@ -30,7 +30,7 @@ class UrlShortenerTest extends TestCase
public function setUp(): void public function setUp(): void
{ {
$this->urlValidator = $this->prophesize(UrlValidatorInterface::class); $this->urlValidator = $this->prophesize(UrlValidatorInterface::class);
$this->urlValidator->validateUrl('http://foobar.com/12345/hello?foo=bar')->will( $this->urlValidator->validateUrl('http://foobar.com/12345/hello?foo=bar', null)->will(
function (): void { function (): void {
}, },
); );

View File

@ -37,7 +37,7 @@ class UrlValidatorTest extends TestCase
$request->shouldBeCalledOnce(); $request->shouldBeCalledOnce();
$this->expectException(InvalidUrlException::class); $this->expectException(InvalidUrlException::class);
$this->urlValidator->validateUrl('http://foobar.com/12345/hello?foo=bar'); $this->urlValidator->validateUrl('http://foobar.com/12345/hello?foo=bar', null);
} }
/** @test */ /** @test */
@ -54,19 +54,29 @@ class UrlValidatorTest extends TestCase
], ],
)->willReturn(new Response()); )->willReturn(new Response());
$this->urlValidator->validateUrl($expectedUrl); $this->urlValidator->validateUrl($expectedUrl, null);
$request->shouldHaveBeenCalledOnce(); $request->shouldHaveBeenCalledOnce();
} }
/** @test */ /**
public function noCheckIsPerformedWhenUrlValidationIsDisabled(): void * @test
* @dataProvider provideDisabledCombinations
*/
public function noCheckIsPerformedWhenUrlValidationIsDisabled(?bool $doValidate, bool $validateUrl): void
{ {
$request = $this->httpClient->request(Argument::cetera())->willReturn(new Response()); $request = $this->httpClient->request(Argument::cetera())->willReturn(new Response());
$this->options->validateUrl = false; $this->options->validateUrl = $validateUrl;
$this->urlValidator->validateUrl(''); $this->urlValidator->validateUrl('', $doValidate);
$request->shouldNotHaveBeenCalled(); $request->shouldNotHaveBeenCalled();
} }
public function provideDisabledCombinations(): iterable
{
yield 'config is disabled and no runtime option is provided' => [null, false];
yield 'config is enabled but runtime option is disabled' => [false, true];
yield 'both config and runtime option are disabled' => [false, false];
}
} }