diff --git a/config/constants.php b/config/constants.php index 978964c5..a7bd0bb7 100644 --- a/config/constants.php +++ b/config/constants.php @@ -12,7 +12,7 @@ const MIN_SHORT_CODES_LENGTH = 4; const DEFAULT_REDIRECT_STATUS_CODE = StatusCodeInterface::STATUS_FOUND; const DEFAULT_REDIRECT_CACHE_LIFETIME = 30; const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory'; -const TITLE_TAG_VALUE = '/]*>(.*?)<\/title>/i'; // Matches the value inside an html title tag +const TITLE_TAG_VALUE = '/]*>(.*?)<\/title>/i'; // Matches the value inside a html title tag const DEFAULT_QR_CODE_SIZE = 300; const DEFAULT_QR_CODE_MARGIN = 0; const DEFAULT_QR_CODE_FORMAT = 'png'; diff --git a/module/Core/src/Model/ShortUrlEdit.php b/module/Core/src/Model/ShortUrlEdit.php index 8e187cf6..d27d1fe6 100644 --- a/module/Core/src/Model/ShortUrlEdit.php +++ b/module/Core/src/Model/ShortUrlEdit.php @@ -29,7 +29,7 @@ final class ShortUrlEdit implements TitleResolutionModelInterface private bool $titlePropWasProvided = false; private ?string $title = null; private bool $titleWasAutoResolved = false; - private ?bool $validateUrl = null; + private bool $validateUrl = false; private bool $crawlablePropWasProvided = false; private bool $crawlable = false; private bool $forwardQueryPropWasProvided = false; @@ -72,7 +72,7 @@ final class ShortUrlEdit implements TitleResolutionModelInterface $this->validSince = parseDateField($inputFilter->getValue(ShortUrlInputFilter::VALID_SINCE)); $this->validUntil = parseDateField($inputFilter->getValue(ShortUrlInputFilter::VALID_UNTIL)); $this->maxVisits = getOptionalIntFromInputFilter($inputFilter, ShortUrlInputFilter::MAX_VISITS); - $this->validateUrl = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::VALIDATE_URL); + $this->validateUrl = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::VALIDATE_URL) ?? false; $this->tags = $inputFilter->getValue(ShortUrlInputFilter::TAGS); $this->title = $inputFilter->getValue(ShortUrlInputFilter::TITLE); $this->crawlable = $inputFilter->getValue(ShortUrlInputFilter::CRAWLABLE); @@ -166,7 +166,7 @@ final class ShortUrlEdit implements TitleResolutionModelInterface return $copy; } - public function doValidateUrl(): ?bool + public function doValidateUrl(): bool { return $this->validateUrl; } diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php index 74390281..86f2c9d1 100644 --- a/module/Core/src/Model/ShortUrlMeta.php +++ b/module/Core/src/Model/ShortUrlMeta.php @@ -26,7 +26,7 @@ final class ShortUrlMeta implements TitleResolutionModelInterface private ?bool $findIfExists = null; private ?string $domain = null; private int $shortCodeLength = 5; - private ?bool $validateUrl = null; + private bool $validateUrl = false; private ?ApiKey $apiKey = null; private array $tags = []; private ?string $title = null; @@ -73,7 +73,7 @@ final class ShortUrlMeta implements TitleResolutionModelInterface $this->customSlug = $inputFilter->getValue(ShortUrlInputFilter::CUSTOM_SLUG); $this->maxVisits = getOptionalIntFromInputFilter($inputFilter, ShortUrlInputFilter::MAX_VISITS); $this->findIfExists = $inputFilter->getValue(ShortUrlInputFilter::FIND_IF_EXISTS); - $this->validateUrl = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::VALIDATE_URL); + $this->validateUrl = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::VALIDATE_URL) ?? false; $this->domain = $inputFilter->getValue(ShortUrlInputFilter::DOMAIN); $this->shortCodeLength = getOptionalIntFromInputFilter( $inputFilter, @@ -151,7 +151,7 @@ final class ShortUrlMeta implements TitleResolutionModelInterface return $this->shortCodeLength; } - public function doValidateUrl(): ?bool + public function doValidateUrl(): bool { return $this->validateUrl; } diff --git a/module/Core/src/Options/UrlShortenerOptions.php b/module/Core/src/Options/UrlShortenerOptions.php index 775254ce..57f4bc37 100644 --- a/module/Core/src/Options/UrlShortenerOptions.php +++ b/module/Core/src/Options/UrlShortenerOptions.php @@ -10,20 +10,9 @@ class UrlShortenerOptions extends AbstractOptions { protected $__strictMode__ = false; // phpcs:ignore - private bool $validateUrl = true; private bool $autoResolveTitles = false; private bool $appendExtraPath = false; - public function isUrlValidationEnabled(): bool - { - return $this->validateUrl; - } - - protected function setValidateUrl(bool $validateUrl): void - { - $this->validateUrl = $validateUrl; - } - public function autoResolveTitles(): bool { return $this->autoResolveTitles; diff --git a/module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php b/module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php index e17f04c2..8af28706 100644 --- a/module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php +++ b/module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php @@ -10,7 +10,7 @@ interface TitleResolutionModelInterface public function getLongUrl(): string; - public function doValidateUrl(): ?bool; + public function doValidateUrl(): bool; public function withResolvedTitle(string $title): self; } diff --git a/module/Core/src/Util/UrlValidator.php b/module/Core/src/Util/UrlValidator.php index bd0a5cfb..da061c0d 100644 --- a/module/Core/src/Util/UrlValidator.php +++ b/module/Core/src/Util/UrlValidator.php @@ -30,10 +30,8 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface /** * @throws InvalidUrlException */ - public function validateUrl(string $url, ?bool $doValidate): void + public function validateUrl(string $url, bool $doValidate): void { - // 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; } @@ -41,15 +39,14 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface $this->validateUrlAndGetResponse($url, true); } - public function validateUrlWithTitle(string $url, ?bool $doValidate): ?string + public function validateUrlWithTitle(string $url, bool $doValidate): ?string { - $doValidate = $doValidate ?? $this->options->isUrlValidationEnabled(); if (! $doValidate && ! $this->options->autoResolveTitles()) { return null; } $response = $this->validateUrlAndGetResponse($url, $doValidate); - if ($response === null) { + if ($response === null || ! $this->options->autoResolveTitles()) { return null; } diff --git a/module/Core/src/Util/UrlValidatorInterface.php b/module/Core/src/Util/UrlValidatorInterface.php index f198d301..299bd22a 100644 --- a/module/Core/src/Util/UrlValidatorInterface.php +++ b/module/Core/src/Util/UrlValidatorInterface.php @@ -11,10 +11,10 @@ interface UrlValidatorInterface /** * @throws InvalidUrlException */ - public function validateUrl(string $url, ?bool $doValidate): void; + public function validateUrl(string $url, bool $doValidate): void; /** * @throws InvalidUrlException */ - public function validateUrlWithTitle(string $url, ?bool $doValidate): ?string; + public function validateUrlWithTitle(string $url, bool $doValidate): ?string; } diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php index 6783303c..5a207cfd 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php @@ -35,10 +35,10 @@ class ShortUrlTitleResolutionHelperTest extends TestCase ShortUrlMeta::fromRawData(['longUrl' => $longUrl, 'title' => $title]), ); - $this->urlValidator->validateUrlWithTitle($longUrl, null)->shouldHaveBeenCalledTimes( + $this->urlValidator->validateUrlWithTitle($longUrl, false)->shouldHaveBeenCalledTimes( $validateWithTitleCallsNum, ); - $this->urlValidator->validateUrl($longUrl, null)->shouldHaveBeenCalledTimes($validateCallsNum); + $this->urlValidator->validateUrl($longUrl, false)->shouldHaveBeenCalledTimes($validateCallsNum); } public function provideTitles(): iterable diff --git a/module/Core/test/Util/UrlValidatorTest.php b/module/Core/test/Util/UrlValidatorTest.php index 57a5d3ce..51729743 100644 --- a/module/Core/test/Util/UrlValidatorTest.php +++ b/module/Core/test/Util/UrlValidatorTest.php @@ -42,7 +42,7 @@ class UrlValidatorTest extends TestCase $request->shouldBeCalledOnce(); $this->expectException(InvalidUrlException::class); - $this->urlValidator->validateUrl('http://foobar.com/12345/hello?foo=bar', null); + $this->urlValidator->validateUrl('http://foobar.com/12345/hello?foo=bar', true); } /** @test */ @@ -65,50 +65,33 @@ class UrlValidatorTest extends TestCase }), )->willReturn(new Response()); - $this->urlValidator->validateUrl($expectedUrl, null); + $this->urlValidator->validateUrl($expectedUrl, true); $request->shouldHaveBeenCalledOnce(); } - /** - * @test - * @dataProvider provideDisabledCombinations - */ - public function noCheckIsPerformedWhenUrlValidationIsDisabled(?bool $doValidate, bool $validateUrl): void + /** @test */ + public function noCheckIsPerformedWhenUrlValidationIsDisabled(): void { $request = $this->httpClient->request(Argument::cetera())->willReturn(new Response()); - $this->options->validateUrl = $validateUrl; - $this->urlValidator->validateUrl('', $doValidate); + $this->urlValidator->validateUrl('', false); $request->shouldNotHaveBeenCalled(); } - /** - * @test - * @dataProvider provideDisabledCombinations - */ - public function validateUrlWithTitleReturnsNullWhenRequestFailsAndValidationIsDisabled( - ?bool $doValidate, - bool $validateUrl, - ): void { + /** @test */ + public function validateUrlWithTitleReturnsNullWhenRequestFailsAndValidationIsDisabled(): void + { $request = $this->httpClient->request(Argument::cetera())->willThrow(ClientException::class); - $this->options->validateUrl = $validateUrl; $this->options->autoResolveTitles = true; - $result = $this->urlValidator->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', $doValidate); + $result = $this->urlValidator->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', false); self::assertNull($result); $request->shouldHaveBeenCalledOnce(); } - 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]; - } - /** @test */ public function validateUrlWithTitleReturnsNullWhenAutoResolutionIsDisabled(): void { diff --git a/module/Rest/test-api/Action/CreateShortUrlTest.php b/module/Rest/test-api/Action/CreateShortUrlTest.php index 0abd2021..2fe529a3 100644 --- a/module/Rest/test-api/Action/CreateShortUrlTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlTest.php @@ -230,7 +230,7 @@ class CreateShortUrlTest extends ApiTestCase { $expectedDetail = sprintf('Provided URL %s is invalid. Try with a different one.', $url); - [$statusCode, $payload] = $this->createShortUrl(['longUrl' => $url]); + [$statusCode, $payload] = $this->createShortUrl(['longUrl' => $url, 'validateUrl' => true]); self::assertEquals(self::STATUS_BAD_REQUEST, $statusCode); self::assertEquals(self::STATUS_BAD_REQUEST, $payload['status']); diff --git a/module/Rest/test-api/Action/EditShortUrlTest.php b/module/Rest/test-api/Action/EditShortUrlTest.php index a25ccddd..92f9bbe0 100644 --- a/module/Rest/test-api/Action/EditShortUrlTest.php +++ b/module/Rest/test-api/Action/EditShortUrlTest.php @@ -82,6 +82,7 @@ class EditShortUrlTest extends ApiTestCase $resp = $this->callApiWithKey(self::METHOD_PATCH, $url, [RequestOptions::JSON => [ 'longUrl' => $longUrl, + 'validateUrl' => true, ]]); self::assertEquals($expectedStatus, $resp->getStatusCode());