Merge pull request #843 from acelaya-forks/feature/runtime-validation-flag

Feature/runtime validation flag

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
 

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"
                             }
                         }
                     }

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"
                             }
                         }
                     }

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;
+    }
 }

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];
+    }
 }

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;
+}

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);
+        $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
@@ -103,4 +101,9 @@ final class ShortUrlEdit
     {
         return $this->maxVisitsPropWasProvided;
     }
+
+    public function doValidateUrl(): ?bool
+    {
+        return $this->validateUrl;
+    }
 }

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;
+    }
 }

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);

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));

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 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;
         }
 

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;
 }

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);

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,
+            ],
+        )];
     }
 }

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 {
             },
         );

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];
+    }
 }