diff --git a/CHANGELOG.md b/CHANGELOG.md index dfd3ef05..1ace111c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this #### Added * [#829](https://github.com/shlinkio/shlink/issues/829) Added support for QR codes in SVG format, by passing `?format=svg` to the QR code URL. +* [#820](https://github.com/shlinkio/shlink/issues/820) Added new option to force enabling or disabling URL validation on a per-URL basis. + + Currently, there's a global config that tells if long URLs should be validated (by ensuring they are publicly accessible and return a 2xx status). However, this is either always applied or never applied. + + Now, it is possible to enforce validation or enforce disabling validation when a new short URL is created or edited: + + * On the `POST /short-url` and `PATCH /short-url/{shortCode}` endpoints, you can now pass `validateUrl: true/false` in order to enforce enabling or disabling validation, ignoring the global config. If the value is not provided, the global config is still normally applied. + * On the `short-url:generate` CLI command, you can pass `--validate-url` or `--no-validate-url` flags, in order to enforce enabling or disabling validation. If none of them is provided, the global config is still normally applied. #### Changed diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index 2467de24..a89dd187 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -251,6 +251,10 @@ "shortCodeLength": { "description": "The length for generated short code. It has to be at least 4 and defaults to 5. It will be ignored when customSlug is provided", "type": "number" + }, + "validateUrl": { + "description": "Tells if the long URL should or should not be validated as a reachable URL. If not provided, it will fall back to app-level config", + "type": "boolean" } } } diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}.json b/docs/swagger/paths/v1_short-urls_{shortCode}.json index 71a6a427..c7e7dc8a 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}.json @@ -127,6 +127,10 @@ "maxVisits": { "description": "The maximum number of allowed visits for this short code", "type": "number" + }, + "validateUrl": { + "description": "Tells if the long URL (if provided) should or should not be validated as a reachable URL. If not provided, it will fall back to app-level config", + "type": "boolean" } } } diff --git a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php index 06cdd274..36293377 100644 --- a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php @@ -21,7 +21,9 @@ use function array_map; use function Functional\curry; use function Functional\flatten; use function Functional\unique; +use function method_exists; use function sprintf; +use function strpos; class GenerateShortUrlCommand extends Command { @@ -94,6 +96,18 @@ class GenerateShortUrlCommand extends Command 'l', InputOption::VALUE_REQUIRED, 'The length for generated short code (it will be ignored if --customSlug was provided).', + ) + ->addOption( + 'validate-url', + null, + InputOption::VALUE_NONE, + 'Forces the long URL to be validated, regardless what is globally configured.', + ) + ->addOption( + 'no-validate-url', + null, + InputOption::VALUE_NONE, + 'Forces the long URL to not be validated, regardless what is globally configured.', ); } @@ -125,6 +139,7 @@ class GenerateShortUrlCommand extends Command $customSlug = $input->getOption('customSlug'); $maxVisits = $input->getOption('maxVisits'); $shortCodeLength = $input->getOption('shortCodeLength') ?? $this->defaultShortCodeLength; + $doValidateUrl = $this->doValidateUrl($input); try { $shortUrl = $this->urlShortener->urlToShortCode($longUrl, $tags, ShortUrlMeta::fromRawData([ @@ -135,6 +150,7 @@ class GenerateShortUrlCommand extends Command ShortUrlMetaInputFilter::FIND_IF_EXISTS => $input->getOption('findIfExists'), ShortUrlMetaInputFilter::DOMAIN => $input->getOption('domain'), ShortUrlMetaInputFilter::SHORT_CODE_LENGTH => $shortCodeLength, + ShortUrlMetaInputFilter::VALIDATE_URL => $doValidateUrl, ])); $io->writeln([ @@ -147,4 +163,18 @@ class GenerateShortUrlCommand extends Command return ExitCodes::EXIT_FAILURE; } } + + private function doValidateUrl(InputInterface $input): ?bool + { + $rawInput = method_exists($input, '__toString') ? $input->__toString() : ''; + + if (strpos($rawInput, '--no-validate-url') !== false) { + return false; + } + if (strpos($rawInput, '--validate-url') !== false) { + return true; + } + + return null; + } } diff --git a/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php index 689a5e7c..eee57b81 100644 --- a/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GenerateShortUrlCommandTest.php @@ -13,6 +13,7 @@ use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; +use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Service\UrlShortener; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; @@ -105,4 +106,34 @@ class GenerateShortUrlCommandTest extends TestCase $this->assertStringContainsString($shortUrl->toString(self::DOMAIN_CONFIG), $output); $urlToShortCode->shouldHaveBeenCalledOnce(); } + + /** + * @test + * @dataProvider provideFlags + */ + public function urlValidationHasExpectedValueBasedOnProvidedTags(array $options, ?bool $expectedValidateUrl): void + { + $shortUrl = new ShortUrl(''); + $urlToShortCode = $this->urlShortener->urlToShortCode( + Argument::type('string'), + Argument::type('array'), + Argument::that(function (ShortUrlMeta $meta) use ($expectedValidateUrl) { + Assert::assertEquals($expectedValidateUrl, $meta->doValidateUrl()); + return $meta; + }), + )->willReturn($shortUrl); + + $options['longUrl'] = 'http://domain.com/foo/bar'; + $this->commandTester->execute($options); + + $urlToShortCode->shouldHaveBeenCalledOnce(); + } + + public function provideFlags(): iterable + { + yield 'no flags' => [[], null]; + yield 'no-validate-url only' => [['--no-validate-url' => true], false]; + yield 'validate-url' => [['--validate-url' => true], true]; + yield 'both flags' => [['--validate-url' => true, '--no-validate-url' => true], false]; + } } 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]; + } }