mirror of
https://github.com/shlinkio/shlink.git
synced 2024-11-27 08:18:24 +03:00
Fixed URL validation still being true by default
This commit is contained in:
parent
48d3ab0cb4
commit
9ea8f3b590
11 changed files with 26 additions and 56 deletions
|
@ -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[^>]*>(.*?)<\/title>/i'; // Matches the value inside an html title tag
|
||||
const TITLE_TAG_VALUE = '/<title[^>]*>(.*?)<\/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';
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
|
|
@ -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;
|
||||
|
|
|
@ -10,7 +10,7 @@ interface TitleResolutionModelInterface
|
|||
|
||||
public function getLongUrl(): string;
|
||||
|
||||
public function doValidateUrl(): ?bool;
|
||||
public function doValidateUrl(): bool;
|
||||
|
||||
public function withResolvedTitle(string $title): self;
|
||||
}
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
{
|
||||
|
|
|
@ -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']);
|
||||
|
|
|
@ -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());
|
||||
|
|
Loading…
Reference in a new issue