mirrors/shlink
1
0
Fork 0
mirror of https://github.com/shlinkio/shlink.git synced 2025-03-14 04:00:57 +03:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #1366 from acelaya-forks/feature/fix-autoresolve-titles

Browse source 
Feature/fix autoresolve titles
This commit is contained in:
Alejandro Celaya 2022-02-01 20:00:49 +01:00 • committed by GitHub
commit 87cadce0ac
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 56 additions and 57 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
CHANGELOG.md
View file

@ -4,6 +4,23 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org).
## [Unreleased]
### Added
* *Nothing*
### Changed
* *Nothing*
### Deprecated
* *Nothing*
### Removed
* *Nothing*
### Fixed
* [#1363](https://github.com/shlinkio/shlink/issues/1363) Fixed titles being resolved no matter what when `validateUrl` is not set or is explicity set to true.
## [3.0.0] - 2022-01-28
### Added
* [#767](https://github.com/shlinkio/shlink/issues/767) Added full support to use emojis everywhere, whether it is custom slugs, titles, referrers, etc.

2
composer.json
View file

@ -128,7 +128,7 @@
],
"test:unit": "@php vendor/bin/phpunit --order-by=random --colors=always --coverage-php build/coverage-unit.cov --testdox",
"test:unit:ci": "@test:unit --coverage-xml=build/coverage-unit/coverage-xml --log-junit=build/coverage-unit/junit.xml",
"test:unit:pretty": "@php vendor/bin/phpunit --order-by=random --colors=always --coverage-html build/coverage-unit/coverage-html",
"test:unit:pretty": "@test:unit --coverage-html build/coverage-unit/coverage-html",
"test:db": "@parallel test:db:sqlite:ci test:db:mysql test:db:maria test:db:postgres test:db:ms",
"test:db:sqlite": "APP_ENV=test php vendor/bin/phpunit --order-by=random --colors=always --testdox -c phpunit-db.xml",
"test:db:sqlite:ci": "@test:db:sqlite --coverage-php build/coverage-db.cov --coverage-xml=build/coverage-db/coverage-xml --log-junit=build/coverage-db/junit.xml",

2
config/constants.php
View file

@ -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';

6
module/Core/src/Model/ShortUrlEdit.php
View file

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

6
module/Core/src/Model/ShortUrlMeta.php
View file

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

11
module/Core/src/Options/UrlShortenerOptions.php
View file

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

2
module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php
View file

@ -10,7 +10,7 @@ interface TitleResolutionModelInterface
public function getLongUrl(): string;
public function doValidateUrl(): ?bool;
public function doValidateUrl(): bool;
public function withResolvedTitle(string $title): self;
}

9
module/Core/src/Util/UrlValidator.php
View file

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

4
module/Core/src/Util/UrlValidatorInterface.php
View file

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

4
module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php
View file

@ -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

47
module/Core/test/Util/UrlValidatorTest.php
View file

@ -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
{
@ -121,6 +104,18 @@ class UrlValidatorTest extends TestCase
$request->shouldNotHaveBeenCalled();
}
/** @test */
public function validateUrlWithTitleReturnsNullWhenAutoResolutionIsDisabledAndValidationIsEnabled(): void
{
$request = $this->httpClient->request(Argument::cetera())->willReturn($this->respWithTitle());
$this->options->autoResolveTitles = false;
$result = $this->urlValidator->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', true);
self::assertNull($result);
$request->shouldHaveBeenCalledOnce();
}
/** @test */
public function validateUrlWithTitleResolvesTitleWhenAutoResolutionIsEnabled(): void
{

2
module/Rest/test-api/Action/CreateShortUrlTest.php
View file

@ -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']);

1
module/Rest/test-api/Action/EditShortUrlTest.php
View file

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