From d5eac3b1c3c2b8da4b6b3fdd4e4f6fb433bc7c41 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 23 Sep 2020 19:19:17 +0200 Subject: [PATCH] Added validateUrl optional flag for create/edit short URLs --- module/Core/functions/functions.php | 13 +++++++++++ module/Core/src/Model/ShortUrlEdit.php | 17 ++++++++------ module/Core/src/Model/ShortUrlMeta.php | 19 +++++++++------- module/Core/src/Service/ShortUrlService.php | 2 +- module/Core/src/Service/UrlShortener.php | 2 +- module/Core/src/Util/UrlValidator.php | 7 +++--- .../Core/src/Util/UrlValidatorInterface.php | 2 +- .../Validation/ShortUrlMetaInputFilter.php | 3 +++ .../Core/test/Service/ShortUrlServiceTest.php | 11 +++++++++- module/Core/test/Service/UrlShortenerTest.php | 2 +- module/Core/test/Util/UrlValidatorTest.php | 22 ++++++++++++++----- 11 files changed, 71 insertions(+), 29 deletions(-) diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 5b6657d1..2f7f86e9 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core; use Cake\Chronos\Chronos; use DateTimeInterface; use Fig\Http\Message\StatusCodeInterface; +use Laminas\InputFilter\InputFilter; use PUGX\Shortid\Factory as ShortIdFactory; use function sprintf; @@ -62,3 +63,15 @@ function determineTableName(string $tableName, array $emConfig = []): string 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; +} diff --git a/module/Core/src/Model/ShortUrlEdit.php b/module/Core/src/Model/ShortUrlEdit.php index 2f3f6919..67300682 100644 --- a/module/Core/src/Model/ShortUrlEdit.php +++ b/module/Core/src/Model/ShortUrlEdit.php @@ -9,6 +9,8 @@ use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter; use function array_key_exists; +use function Shlinkio\Shlink\Core\getOptionalBoolFromInputFilter; +use function Shlinkio\Shlink\Core\getOptionalIntFromInputFilter; use function Shlinkio\Shlink\Core\parseDateField; final class ShortUrlEdit @@ -21,6 +23,7 @@ final class ShortUrlEdit private ?Chronos $validUntil = null; private bool $maxVisitsPropWasProvided = false; private ?int $maxVisits = null; + private ?bool $validateUrl = null; // Enforce named constructors private function __construct() @@ -55,13 +58,8 @@ final class ShortUrlEdit $this->longUrl = $inputFilter->getValue(ShortUrlMetaInputFilter::LONG_URL); $this->validSince = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_SINCE)); $this->validUntil = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_UNTIL)); - $this->maxVisits = $this->getOptionalIntFromInputFilter($inputFilter, ShortUrlMetaInputFilter::MAX_VISITS); - } - - private function getOptionalIntFromInputFilter(ShortUrlMetaInputFilter $inputFilter, string $fieldName): ?int - { - $value = $inputFilter->getValue($fieldName); - return $value !== null ? (int) $value : null; + $this->maxVisits = getOptionalIntFromInputFilter($inputFilter, ShortUrlMetaInputFilter::MAX_VISITS); + $this->validateUrl = getOptionalBoolFromInputFilter($inputFilter, ShortUrlMetaInputFilter::VALIDATE_URL); } public function longUrl(): ?string @@ -103,4 +101,9 @@ final class ShortUrlEdit { return $this->maxVisitsPropWasProvided; } + + public function doValidateUrl(): ?bool + { + return $this->validateUrl; + } } diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php index 76f6d80b..23121d71 100644 --- a/module/Core/src/Model/ShortUrlMeta.php +++ b/module/Core/src/Model/ShortUrlMeta.php @@ -8,6 +8,8 @@ use Cake\Chronos\Chronos; use Shlinkio\Shlink\Core\Exception\ValidationException; 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 const Shlinkio\Shlink\Core\DEFAULT_SHORT_CODES_LENGTH; @@ -21,6 +23,7 @@ final class ShortUrlMeta private ?bool $findIfExists = null; private ?string $domain = null; private int $shortCodeLength = 5; + private ?bool $validateUrl = null; // Enforce named constructors private function __construct() @@ -55,21 +58,16 @@ final class ShortUrlMeta $this->validSince = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_SINCE)); $this->validUntil = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_UNTIL)); $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->validateUrl = getOptionalBoolFromInputFilter($inputFilter, ShortUrlMetaInputFilter::VALIDATE_URL); $this->domain = $inputFilter->getValue(ShortUrlMetaInputFilter::DOMAIN); - $this->shortCodeLength = $this->getOptionalIntFromInputFilter( + $this->shortCodeLength = getOptionalIntFromInputFilter( $inputFilter, ShortUrlMetaInputFilter::SHORT_CODE_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 { return $this->validSince; @@ -129,4 +127,9 @@ final class ShortUrlMeta { return $this->shortCodeLength; } + + public function doValidateUrl(): ?bool + { + return $this->validateUrl; + } } diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php index 456c46ad..9159ef63 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -71,7 +71,7 @@ class ShortUrlService implements ShortUrlServiceInterface public function updateMetadataByShortCode(ShortUrlIdentifier $identifier, ShortUrlEdit $shortUrlEdit): ShortUrl { if ($shortUrlEdit->hasLongUrl()) { - $this->urlValidator->validateUrl($shortUrlEdit->longUrl()); + $this->urlValidator->validateUrl($shortUrlEdit->longUrl(), $shortUrlEdit->doValidateUrl()); } $shortUrl = $this->urlResolver->resolveShortUrl($identifier); diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index cf17d0bd..c6464f41 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -48,7 +48,7 @@ class UrlShortener implements UrlShortenerInterface return $existingShortUrl; } - $this->urlValidator->validateUrl($url); + $this->urlValidator->validateUrl($url, $meta->doValidateUrl()); $this->em->beginTransaction(); $shortUrl = new ShortUrl($url, $meta, $this->domainResolver); $shortUrl->setTags($this->tagNamesToEntities($this->em, $tags)); diff --git a/module/Core/src/Util/UrlValidator.php b/module/Core/src/Util/UrlValidator.php index 01885446..ccf69dd1 100644 --- a/module/Core/src/Util/UrlValidator.php +++ b/module/Core/src/Util/UrlValidator.php @@ -27,10 +27,11 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface /** * @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 (! $this->options->isUrlValidationEnabled()) { + // If the URL validation is not enabled or it was explicitly set to not validate, skip check + $doValidate = $doValidate ?? $this->options->isUrlValidationEnabled(); + if (! $doValidate) { return; } diff --git a/module/Core/src/Util/UrlValidatorInterface.php b/module/Core/src/Util/UrlValidatorInterface.php index 05230605..fdf1e781 100644 --- a/module/Core/src/Util/UrlValidatorInterface.php +++ b/module/Core/src/Util/UrlValidatorInterface.php @@ -11,5 +11,5 @@ interface UrlValidatorInterface /** * @throws InvalidUrlException */ - public function validateUrl(string $url): void; + public function validateUrl(string $url, ?bool $doValidate): void; } diff --git a/module/Core/src/Validation/ShortUrlMetaInputFilter.php b/module/Core/src/Validation/ShortUrlMetaInputFilter.php index 6d0cfffe..5c0030f0 100644 --- a/module/Core/src/Validation/ShortUrlMetaInputFilter.php +++ b/module/Core/src/Validation/ShortUrlMetaInputFilter.php @@ -27,6 +27,7 @@ class ShortUrlMetaInputFilter extends InputFilter public const DOMAIN = 'domain'; public const SHORT_CODE_LENGTH = 'shortCodeLength'; public const LONG_URL = 'longUrl'; + public const VALIDATE_URL = 'validateUrl'; 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->createInput(self::VALIDATE_URL, false)); + $domain = $this->createInput(self::DOMAIN, false); $domain->getValidatorChain()->attach(new Validation\HostAndPortValidator()); $this->add($domain); diff --git a/module/Core/test/Service/ShortUrlServiceTest.php b/module/Core/test/Service/ShortUrlServiceTest.php index 9becdf8b..fcb63ec2 100644 --- a/module/Core/test/Service/ShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrlServiceTest.php @@ -104,7 +104,10 @@ class ShortUrlServiceTest extends TestCase $this->assertEquals($shortUrlEdit->longUrl() ?? $originalLongUrl, $shortUrl->getLongUrl()); $findShortUrl->shouldHaveBeenCalled(); $flush->shouldHaveBeenCalled(); - $this->urlValidator->validateUrl($shortUrlEdit->longUrl())->shouldHaveBeenCalledTimes($expectedValidateCalls); + $this->urlValidator->validateUrl( + $shortUrlEdit->longUrl(), + $shortUrlEdit->doValidateUrl(), + )->shouldHaveBeenCalledTimes($expectedValidateCalls); } public function provideShortUrlEdits(): iterable @@ -123,5 +126,11 @@ class ShortUrlServiceTest extends TestCase 'longUrl' => 'modifiedLongUrl', ], )]; + yield 'long URL with validation' => [1, ShortUrlEdit::fromRawData( + [ + 'longUrl' => 'modifiedLongUrl', + 'validateUrl' => true, + ], + )]; } } diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index 40ed6718..adeda920 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -30,7 +30,7 @@ class UrlShortenerTest extends TestCase public function setUp(): void { $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 { }, ); diff --git a/module/Core/test/Util/UrlValidatorTest.php b/module/Core/test/Util/UrlValidatorTest.php index a20ed693..41541b3c 100644 --- a/module/Core/test/Util/UrlValidatorTest.php +++ b/module/Core/test/Util/UrlValidatorTest.php @@ -37,7 +37,7 @@ class UrlValidatorTest extends TestCase $request->shouldBeCalledOnce(); $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 */ @@ -54,19 +54,29 @@ class UrlValidatorTest extends TestCase ], )->willReturn(new Response()); - $this->urlValidator->validateUrl($expectedUrl); + $this->urlValidator->validateUrl($expectedUrl, null); $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()); - $this->options->validateUrl = false; + $this->options->validateUrl = $validateUrl; - $this->urlValidator->validateUrl(''); + $this->urlValidator->validateUrl('', $doValidate); $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]; + } }