diff --git a/UPGRADE.md b/UPGRADE.md index f76be20b..ab6085ad 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -13,6 +13,7 @@ * The short URLs `loosely` mode is no longer supported, as it was a typo. Use `loose` mode instead. * QR codes URLs now work by default, even for short URLs that cannot be visited due to max visits or date range limitations. If you want to keep previous behavior, pass `QR_CODE_FOR_DISABLED_SHORT_URLS=false` or the equivalent configuration option. +* Shlink no longer allows to opt-in for long URL verification. Long URLs are unconditionally considered correct during short URL creation/edition. ### Changes in REST API @@ -21,7 +22,6 @@ * `INVALID_SHORT_URL_DELETION` -> `https://shlink.io/api/error/invalid-short-url-deletion` * `DOMAIN_NOT_FOUND` -> `https://shlink.io/api/error/domain-not-found` * `FORBIDDEN_OPERATION` -> `https://shlink.io/api/error/forbidden-tag-operation` - * `INVALID_URL` -> `https://shlink.io/api/error/invalid-url` * `INVALID_SLUG` -> `https://shlink.io/api/error/non-unique-slug` * `INVALID_SHORTCODE` -> `https://shlink.io/api/error/short-url-not-found` * `TAG_CONFLICT` -> `https://shlink.io/api/error/tag-conflict` diff --git a/docs/swagger/definitions/ShortUrlEdition.json b/docs/swagger/definitions/ShortUrlEdition.json index dda213ca..baef4f52 100644 --- a/docs/swagger/definitions/ShortUrlEdition.json +++ b/docs/swagger/definitions/ShortUrlEdition.json @@ -20,11 +20,6 @@ "description": "The maximum number of allowed visits for this short code", "type": ["number", "null"] }, - "validateUrl": { - "deprecated": true, - "description": "**[DEPRECATED]** Tells if the long URL should or should not be validated as a reachable URL. Defaults to `false`", - "type": "boolean" - }, "tags": { "type": "array", "items": { diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index c226046f..08e08b67 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -388,10 +388,6 @@ ] } }, - "url": { - "type": "string", - "description": "A URL that could not be verified, if the error type is https://shlink.io/api/error/invalid-url" - }, "customSlug": { "type": "string", "description": "Provided custom slug when the error type is https://shlink.io/api/error/non-unique-slug" @@ -408,15 +404,6 @@ "Invalid arguments with API v3 and newer": { "$ref": "../examples/short-url-invalid-args-v3.json" }, - "Invalid long URL with API v3 and newer": { - "value": { - "title": "Invalid URL", - "type": "https://shlink.io/api/error/invalid-url", - "detail": "Provided URL foo is invalid. Try with a different one.", - "status": 400, - "url": "https://invalid-url.com" - } - }, "Non-unique slug with API v3 and newer": { "value": { "title": "Invalid custom slug", @@ -429,15 +416,6 @@ "Invalid arguments previous to API v3": { "$ref": "../examples/short-url-invalid-args-v2.json" }, - "Invalid long URL previous to API v3": { - "value": { - "title": "Invalid URL", - "type": "INVALID_URL", - "detail": "Provided URL foo is invalid. Try with a different one.", - "status": 400, - "url": "https://invalid-url.com" - } - }, "Non-unique slug previous to API v3": { "value": { "title": "Invalid custom slug", diff --git a/docs/swagger/paths/v1_short-urls_shorten.json b/docs/swagger/paths/v1_short-urls_shorten.json index cacb00bb..5c16482c 100644 --- a/docs/swagger/paths/v1_short-urls_shorten.json +++ b/docs/swagger/paths/v1_short-urls_shorten.json @@ -88,49 +88,6 @@ } } }, - "400": { - "description": "The long URL was not provided or is invalid.", - "content": { - "application/problem+json": { - "schema": { - "$ref": "../definitions/Error.json" - }, - "examples": { - "API v3 and newer": { - "value": { - "title": "Invalid URL", - "type": "https://shlink.io/api/error/invalid-url", - "detail": "Provided URL foo is invalid. Try with a different one.", - "status": 400, - "url": "https://invalid-url.com" - } - }, - "Previous to API v3": { - "value": { - "title": "Invalid URL", - "type": "INVALID_URL", - "detail": "Provided URL foo is invalid. Try with a different one.", - "status": 400, - "url": "https://invalid-url.com" - } - } - } - }, - "text/plain": { - "schema": { - "type": "string" - }, - "examples": { - "API v3 and newer": { - "value": "https://shlink.io/api/error/invalid-url" - }, - "Previous to API v3": { - "value": "INVALID_URL" - } - } - } - } - }, "default": { "description": "Unexpected error.", "content": { diff --git a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php index 3377e649..118ad201 100644 --- a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Util\ExitCode; -use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; @@ -95,12 +94,6 @@ class CreateShortUrlCommand extends Command InputOption::VALUE_REQUIRED, 'The length for generated short code (it will be ignored if --custom-slug was provided).', ) - ->addOption( - 'validate-url', - null, - InputOption::VALUE_NONE, - '[DEPRECATED] Makes the URL to be validated as publicly accessible.', - ) ->addOption( 'crawlable', 'r', @@ -148,7 +141,6 @@ class CreateShortUrlCommand extends Command $customSlug = $input->getOption('custom-slug'); $maxVisits = $input->getOption('max-visits'); $shortCodeLength = $input->getOption('short-code-length') ?? $this->options->defaultShortCodesLength; - $doValidateUrl = $input->getOption('validate-url'); try { $result = $this->urlShortener->shorten(ShortUrlCreation::fromRawData([ @@ -160,7 +152,6 @@ class CreateShortUrlCommand extends Command ShortUrlInputFilter::FIND_IF_EXISTS => $input->getOption('find-if-exists'), ShortUrlInputFilter::DOMAIN => $input->getOption('domain'), ShortUrlInputFilter::SHORT_CODE_LENGTH => $shortCodeLength, - ShortUrlInputFilter::VALIDATE_URL => $doValidateUrl, ShortUrlInputFilter::TAGS => $tags, ShortUrlInputFilter::CRAWLABLE => $input->getOption('crawlable'), ShortUrlInputFilter::FORWARD_QUERY => !$input->getOption('no-forward-query'), @@ -176,7 +167,7 @@ class CreateShortUrlCommand extends Command sprintf('Generated short URL: %s', $this->stringifier->stringify($result->shortUrl)), ]); return ExitCode::EXIT_SUCCESS; - } catch (InvalidUrlException | NonUniqueSlugException $e) { + } catch (NonUniqueSlugException $e) { $io->error($e->getMessage()); return ExitCode::EXIT_FAILURE; } diff --git a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php index de0fe26b..33031c6b 100644 --- a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php @@ -12,7 +12,6 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\ShortUrl\CreateShortUrlCommand; use Shlinkio\Shlink\CLI\Util\ExitCode; -use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; @@ -68,22 +67,6 @@ class CreateShortUrlCommandTest extends TestCase self::assertStringNotContainsString('but the real-time updates cannot', $output); } - #[Test] - public function exceptionWhileParsingLongUrlOutputsError(): void - { - $url = 'http://domain.com/invalid'; - $this->urlShortener->expects($this->once())->method('shorten')->withAnyParameters()->willThrowException( - InvalidUrlException::fromUrl($url), - ); - $this->stringifier->method('stringify')->with($this->isInstanceOf(ShortUrl::class))->willReturn(''); - - $this->commandTester->execute(['longUrl' => $url]); - $output = $this->commandTester->getDisplay(); - - self::assertEquals(ExitCode::EXIT_FAILURE, $this->commandTester->getStatusCode()); - self::assertStringContainsString('Provided URL http://domain.com/invalid is invalid.', $output); - } - #[Test] public function providingNonUniqueSlugOutputsError(): void { @@ -148,12 +131,12 @@ class CreateShortUrlCommandTest extends TestCase } #[Test, DataProvider('provideFlags')] - public function urlValidationHasExpectedValueBasedOnProvidedFlags(array $options, ?bool $expectedValidateUrl): void + public function urlValidationHasExpectedValueBasedOnProvidedFlags(array $options, ?bool $expectedCrawlable): void { $shortUrl = ShortUrl::createFake(); $this->urlShortener->expects($this->once())->method('shorten')->with( - $this->callback(function (ShortUrlCreation $meta) use ($expectedValidateUrl) { - Assert::assertEquals($expectedValidateUrl, $meta->doValidateUrl()); + $this->callback(function (ShortUrlCreation $meta) use ($expectedCrawlable) { + Assert::assertEquals($expectedCrawlable, $meta->crawlable); return true; }), )->willReturn(UrlShorteningResult::withoutErrorOnEventDispatching($shortUrl)); @@ -166,7 +149,7 @@ class CreateShortUrlCommandTest extends TestCase public static function provideFlags(): iterable { yield 'no flags' => [[], null]; - yield 'validate-url' => [['--validate-url' => true], true]; + yield 'crawlable' => [['--crawlable' => true], true]; } /** diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 6b6be190..5f9ae565 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -75,7 +75,6 @@ return [ Visit\Entity\Visit::class, ], - Util\UrlValidator::class => ConfigAbstractFactory::class, Util\DoctrineBatchHelper::class => ConfigAbstractFactory::class, Util\RedirectResponseHelper::class => ConfigAbstractFactory::class, @@ -153,7 +152,6 @@ return [ ShortUrl\Helper\ShortCodeUniquenessHelper::class => ['em', Options\UrlShortenerOptions::class], Domain\DomainService::class => ['em', 'config.url_shortener.domain.hostname'], - Util\UrlValidator::class => ['httpClient', Options\UrlShortenerOptions::class], Util\DoctrineBatchHelper::class => ['em'], Util\RedirectResponseHelper::class => [Options\RedirectOptions::class], @@ -180,7 +178,7 @@ return [ Lock\LockFactory::class, ], ShortUrl\Helper\ShortUrlStringifier::class => ['config.url_shortener.domain', 'config.router.base_path'], - ShortUrl\Helper\ShortUrlTitleResolutionHelper::class => [Util\UrlValidator::class], + ShortUrl\Helper\ShortUrlTitleResolutionHelper::class => ['httpClient', Options\UrlShortenerOptions::class], ShortUrl\Helper\ShortUrlRedirectionBuilder::class => [Options\TrackingOptions::class], ShortUrl\Transformer\ShortUrlDataTransformer::class => [ShortUrl\Helper\ShortUrlStringifier::class], ShortUrl\Middleware\ExtraPathRedirectMiddleware::class => [ diff --git a/module/Core/src/Exception/InvalidUrlException.php b/module/Core/src/Exception/InvalidUrlException.php deleted file mode 100644 index 200914c2..00000000 --- a/module/Core/src/Exception/InvalidUrlException.php +++ /dev/null @@ -1,35 +0,0 @@ -detail = $e->getMessage(); - $e->title = self::TITLE; - $e->type = toProblemDetailsType(self::ERROR_CODE); - $e->status = $status; - $e->additional = ['url' => $url]; - - return $e; - } -} diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index e53e9afa..ee2c5920 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -120,7 +120,6 @@ class ShortUrl extends AbstractEntity ?ShortUrlRelationResolverInterface $relationResolver = null, ): self { $meta = [ - ShortUrlInputFilter::VALIDATE_URL => false, ShortUrlInputFilter::LONG_URL => $url->longUrl, ShortUrlInputFilter::DOMAIN => $url->domain, ShortUrlInputFilter::TAGS => $url->tags, diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php index 71963437..6a539f58 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php @@ -4,31 +4,90 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Helper; -use Shlinkio\Shlink\Core\Exception\InvalidUrlException; -use Shlinkio\Shlink\Core\Util\UrlValidatorInterface; +use Fig\Http\Message\RequestMethodInterface; +use GuzzleHttp\ClientInterface; +use GuzzleHttp\RequestOptions; +use Psr\Http\Message\ResponseInterface; +use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; +use Throwable; -class ShortUrlTitleResolutionHelper implements ShortUrlTitleResolutionHelperInterface +use function html_entity_decode; +use function preg_match; +use function str_contains; +use function str_starts_with; +use function strtolower; +use function trim; + +use const Shlinkio\Shlink\TITLE_TAG_VALUE; + +readonly class ShortUrlTitleResolutionHelper implements ShortUrlTitleResolutionHelperInterface { - public function __construct(private readonly UrlValidatorInterface $urlValidator) - { + private const MAX_REDIRECTS = 15; + private const CHROME_USER_AGENT = 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) ' + . 'Chrome/121.0.0.0 Safari/537.36'; + + public function __construct( + private ClientInterface $httpClient, + private UrlShortenerOptions $options, + ) { } /** - * @deprecated TODO Rename to processTitle once URL validation is removed with Shlink 4.0.0 - * Move relevant logic from URL validator here. * @template T of TitleResolutionModelInterface * @param T $data * @return T - * @throws InvalidUrlException */ - public function processTitleAndValidateUrl(TitleResolutionModelInterface $data): TitleResolutionModelInterface + public function processTitle(TitleResolutionModelInterface $data): TitleResolutionModelInterface { - if ($data->hasTitle()) { - $this->urlValidator->validateUrl($data->getLongUrl(), $data->doValidateUrl()); + if (! $this->options->autoResolveTitles || $data->hasTitle()) { return $data; } - $title = $this->urlValidator->validateUrlWithTitle($data->getLongUrl(), $data->doValidateUrl()); - return $title === null ? $data : $data->withResolvedTitle($title); + $response = $this->fetchUrl($data->getLongUrl()); + if ($response === null) { + return $data; + } + + $contentType = strtolower($response->getHeaderLine('Content-Type')); + if (! str_starts_with($contentType, 'text/html')) { + return $data; + } + + $title = $this->tryToResolveTitle($response); + return $title !== null ? $data->withResolvedTitle($title) : $data; + } + + private function fetchUrl(string $url): ?ResponseInterface + { + try { + return $this->httpClient->request(RequestMethodInterface::METHOD_GET, $url, [ + // TODO Add a sensible timeout that prevents hanging here forever + // Prevent potential infinite redirection loops + RequestOptions::ALLOW_REDIRECTS => ['max' => self::MAX_REDIRECTS], + RequestOptions::IDN_CONVERSION => true, + // Making the request with a browser's user agent results in responses closer to a real user + RequestOptions::HEADERS => ['User-Agent' => self::CHROME_USER_AGENT], + RequestOptions::STREAM => true, // This ensures large files are not fully downloaded if not needed + ]); + } catch (Throwable) { + return null; + } + } + + private function tryToResolveTitle(ResponseInterface $response): ?string + { + $collectedBody = ''; + $body = $response->getBody(); + // With streaming enabled, we can walk the body until the tag is found, and then stop + while (! str_contains($collectedBody, '') && ! $body->eof()) { + $collectedBody .= $body->read(1024); + } + preg_match(TITLE_TAG_VALUE, $collectedBody, $matches); + return isset($matches[1]) ? $this->normalizeTitle($matches[1]) : null; + } + + private function normalizeTitle(string $title): string + { + return html_entity_decode(trim($title)); } } diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelperInterface.php b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelperInterface.php index 1861b451..6641460a 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelperInterface.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelperInterface.php @@ -4,16 +4,12 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Helper; -use Shlinkio\Shlink\Core\Exception\InvalidUrlException; - interface ShortUrlTitleResolutionHelperInterface { /** - * @deprecated TODO Rename to processTitle once URL validation is removed with Shlink 4.0.0 * @template T of TitleResolutionModelInterface * @param T $data * @return T - * @throws InvalidUrlException */ - public function processTitleAndValidateUrl(TitleResolutionModelInterface $data): TitleResolutionModelInterface; + public function processTitle(TitleResolutionModelInterface $data): TitleResolutionModelInterface; } diff --git a/module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php b/module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php index 4c56bfc1..cecd83e1 100644 --- a/module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php +++ b/module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php @@ -10,8 +10,5 @@ interface TitleResolutionModelInterface public function getLongUrl(): string; - /** @deprecated */ - public function doValidateUrl(): bool; - public function withResolvedTitle(string $title): static; } diff --git a/module/Core/src/ShortUrl/Model/ShortUrlCreation.php b/module/Core/src/ShortUrl/Model/ShortUrlCreation.php index 43b39874..1a1673ac 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlCreation.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlCreation.php @@ -35,8 +35,6 @@ final class ShortUrlCreation implements TitleResolutionModelInterface public readonly bool $findIfExists = false, public readonly ?string $domain = null, public readonly int $shortCodeLength = 5, - /** @deprecated */ - public readonly bool $validateUrl = false, public readonly ?ApiKey $apiKey = null, public readonly array $tags = [], public readonly ?string $title = null, @@ -75,7 +73,6 @@ final class ShortUrlCreation implements TitleResolutionModelInterface $inputFilter, ShortUrlInputFilter::SHORT_CODE_LENGTH, ) ?? DEFAULT_SHORT_CODES_LENGTH, - validateUrl: getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::VALIDATE_URL) ?? false, apiKey: $inputFilter->getValue(ShortUrlInputFilter::API_KEY), tags: $inputFilter->getValue(ShortUrlInputFilter::TAGS), title: $inputFilter->getValue(ShortUrlInputFilter::TITLE), @@ -97,7 +94,6 @@ final class ShortUrlCreation implements TitleResolutionModelInterface findIfExists: $this->findIfExists, domain: $this->domain, shortCodeLength: $this->shortCodeLength, - validateUrl: $this->validateUrl, apiKey: $this->apiKey, tags: $this->tags, title: $title, @@ -137,12 +133,6 @@ final class ShortUrlCreation implements TitleResolutionModelInterface return $this->domain !== null; } - /** @deprecated */ - public function doValidateUrl(): bool - { - return $this->validateUrl; - } - public function hasTitle(): bool { return $this->title !== null; diff --git a/module/Core/src/ShortUrl/Model/ShortUrlEdition.php b/module/Core/src/ShortUrl/Model/ShortUrlEdition.php index fe92fae8..7a0000de 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlEdition.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlEdition.php @@ -38,8 +38,6 @@ final class ShortUrlEdition implements TitleResolutionModelInterface private readonly bool $titlePropWasProvided = false, public readonly ?string $title = null, public readonly bool $titleWasAutoResolved = false, - /** @deprecated */ - public readonly bool $validateUrl = false, private readonly bool $crawlablePropWasProvided = false, public readonly bool $crawlable = false, private readonly bool $forwardQueryPropWasProvided = false, @@ -76,7 +74,6 @@ final class ShortUrlEdition implements TitleResolutionModelInterface tags: $inputFilter->getValue(ShortUrlInputFilter::TAGS), titlePropWasProvided: array_key_exists(ShortUrlInputFilter::TITLE, $data), title: $inputFilter->getValue(ShortUrlInputFilter::TITLE), - validateUrl: getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::VALIDATE_URL) ?? false, crawlablePropWasProvided: array_key_exists(ShortUrlInputFilter::CRAWLABLE, $data), crawlable: $inputFilter->getValue(ShortUrlInputFilter::CRAWLABLE), forwardQueryPropWasProvided: array_key_exists(ShortUrlInputFilter::FORWARD_QUERY, $data), @@ -102,7 +99,6 @@ final class ShortUrlEdition implements TitleResolutionModelInterface titlePropWasProvided: $this->titlePropWasProvided, title: $title, titleWasAutoResolved: true, - validateUrl: $this->validateUrl, crawlablePropWasProvided: $this->crawlablePropWasProvided, crawlable: $this->crawlable, forwardQueryPropWasProvided: $this->forwardQueryPropWasProvided, @@ -155,12 +151,6 @@ final class ShortUrlEdition implements TitleResolutionModelInterface return $this->titleWasAutoResolved; } - /** @deprecated */ - public function doValidateUrl(): bool - { - return $this->validateUrl; - } - public function crawlableWasProvided(): bool { return $this->crawlablePropWasProvided; diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php index 23ac8a2f..325b400f 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php @@ -36,8 +36,6 @@ class ShortUrlInputFilter extends InputFilter public const SHORT_CODE_LENGTH = 'shortCodeLength'; public const LONG_URL = 'longUrl'; public const DEVICE_LONG_URLS = 'deviceLongUrls'; - /** @deprecated */ - public const VALIDATE_URL = 'validateUrl'; public const API_KEY = 'apiKey'; public const TAGS = 'tags'; public const TITLE = 'title'; @@ -97,9 +95,8 @@ class ShortUrlInputFilter extends InputFilter $this->add($this->createBooleanInput(self::FIND_IF_EXISTS, false)); - // These cannot be defined as a boolean inputs, because they can actually have 3 values: true, false and null. + // This cannot be defined as a boolean inputs, because they can actually have 3 values: true, false and null. // Defining them as boolean will make null fall back to false, which is not the desired behavior. - $this->add($this->createInput(self::VALIDATE_URL, false)); $this->add($this->createInput(self::FORWARD_QUERY, false)); $domain = $this->createInput(self::DOMAIN, false); diff --git a/module/Core/src/ShortUrl/ShortUrlService.php b/module/Core/src/ShortUrl/ShortUrlService.php index 95561fc5..1c3e9295 100644 --- a/module/Core/src/ShortUrl/ShortUrlService.php +++ b/module/Core/src/ShortUrl/ShortUrlService.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl; use Doctrine\ORM; -use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlTitleResolutionHelperInterface; @@ -26,7 +25,6 @@ class ShortUrlService implements ShortUrlServiceInterface /** * @throws ShortUrlNotFoundException - * @throws InvalidUrlException */ public function updateShortUrl( ShortUrlIdentifier $identifier, @@ -34,7 +32,7 @@ class ShortUrlService implements ShortUrlServiceInterface ?ApiKey $apiKey = null, ): ShortUrl { if ($shortUrlEdit->longUrlWasProvided()) { - $shortUrlEdit = $this->titleResolutionHelper->processTitleAndValidateUrl($shortUrlEdit); + $shortUrlEdit = $this->titleResolutionHelper->processTitle($shortUrlEdit); } $shortUrl = $this->urlResolver->resolveShortUrl($identifier, $apiKey); diff --git a/module/Core/src/ShortUrl/ShortUrlServiceInterface.php b/module/Core/src/ShortUrl/ShortUrlServiceInterface.php index 3365374e..c7892f55 100644 --- a/module/Core/src/ShortUrl/ShortUrlServiceInterface.php +++ b/module/Core/src/ShortUrl/ShortUrlServiceInterface.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl; -use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlEdition; @@ -15,7 +14,6 @@ interface ShortUrlServiceInterface { /** * @throws ShortUrlNotFoundException - * @throws InvalidUrlException */ public function updateShortUrl( ShortUrlIdentifier $identifier, diff --git a/module/Core/src/ShortUrl/UrlShortener.php b/module/Core/src/ShortUrl/UrlShortener.php index 0305f936..4a908c78 100644 --- a/module/Core/src/ShortUrl/UrlShortener.php +++ b/module/Core/src/ShortUrl/UrlShortener.php @@ -8,7 +8,6 @@ use Doctrine\ORM\EntityManagerInterface; use Psr\Container\ContainerExceptionInterface; use Psr\EventDispatcher\EventDispatcherInterface; use Shlinkio\Shlink\Core\EventDispatcher\Event\ShortUrlCreated; -use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortCodeUniquenessHelperInterface; @@ -31,7 +30,6 @@ class UrlShortener implements UrlShortenerInterface /** * @throws NonUniqueSlugException - * @throws InvalidUrlException */ public function shorten(ShortUrlCreation $creation): UrlShorteningResult { @@ -41,7 +39,7 @@ class UrlShortener implements UrlShortenerInterface return UrlShorteningResult::withoutErrorOnEventDispatching($existingShortUrl); } - $creation = $this->titleResolutionHelper->processTitleAndValidateUrl($creation); + $creation = $this->titleResolutionHelper->processTitle($creation); /** @var ShortUrl $newShortUrl */ $newShortUrl = $this->em->wrapInTransaction(function () use ($creation): ShortUrl { diff --git a/module/Core/src/ShortUrl/UrlShortenerInterface.php b/module/Core/src/ShortUrl/UrlShortenerInterface.php index 70896ec1..7da0aaef 100644 --- a/module/Core/src/ShortUrl/UrlShortenerInterface.php +++ b/module/Core/src/ShortUrl/UrlShortenerInterface.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl; -use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\UrlShorteningResult; @@ -13,7 +12,6 @@ interface UrlShortenerInterface { /** * @throws NonUniqueSlugException - * @throws InvalidUrlException */ public function shorten(ShortUrlCreation $creation): UrlShorteningResult; } diff --git a/module/Core/src/Util/UrlValidator.php b/module/Core/src/Util/UrlValidator.php deleted file mode 100644 index 1ab3e8f8..00000000 --- a/module/Core/src/Util/UrlValidator.php +++ /dev/null @@ -1,116 +0,0 @@ -validateUrlAndGetResponse($url); - } - - /** - * @deprecated - * @throws InvalidUrlException - */ - public function validateUrlWithTitle(string $url, bool $doValidate): ?string - { - if (! $doValidate && ! $this->options->autoResolveTitles) { - return null; - } - - if (! $this->options->autoResolveTitles) { - $this->validateUrlAndGetResponse($url, self::METHOD_HEAD); - return null; - } - - $response = $doValidate ? $this->validateUrlAndGetResponse($url) : $this->getResponse($url); - if ($response === null) { - return null; - } - - $contentType = strtolower($response->getHeaderLine('Content-Type')); - if (! str_starts_with($contentType, 'text/html')) { - return null; - } - - $collectedBody = ''; - $body = $response->getBody(); - // With streaming enabled, we can walk the body until the tag is found, and then stop - while (! str_contains($collectedBody, '') && ! $body->eof()) { - $collectedBody .= $body->read(1024); - } - preg_match(TITLE_TAG_VALUE, $collectedBody, $matches); - return isset($matches[1]) ? $this->normalizeTitle($matches[1]) : null; - } - - /** - * @param self::METHOD_GET|self::METHOD_HEAD $method - * @throws InvalidUrlException - */ - private function validateUrlAndGetResponse(string $url, string $method = self::METHOD_GET): ResponseInterface - { - try { - return $this->httpClient->request($method, $url, [ - RequestOptions::ALLOW_REDIRECTS => ['max' => self::MAX_REDIRECTS], - RequestOptions::IDN_CONVERSION => true, - // Making the request with a browser's user agent makes the validation closer to a real user - RequestOptions::HEADERS => ['User-Agent' => self::CHROME_USER_AGENT], - RequestOptions::STREAM => true, // This ensures large files are not fully downloaded if not needed - ]); - } catch (GuzzleException $e) { - throw InvalidUrlException::fromUrl($url, $e); - } - } - - private function getResponse(string $url): ?ResponseInterface - { - try { - return $this->validateUrlAndGetResponse($url); - } catch (Throwable) { - return null; - } - } - - private function normalizeTitle(string $title): string - { - return html_entity_decode(trim($title)); - } -} diff --git a/module/Core/src/Util/UrlValidatorInterface.php b/module/Core/src/Util/UrlValidatorInterface.php deleted file mode 100644 index cb38dc42..00000000 --- a/module/Core/src/Util/UrlValidatorInterface.php +++ /dev/null @@ -1,23 +0,0 @@ -getMessage()); - self::assertEquals($expectedMessage, $e->getDetail()); - self::assertEquals('Invalid URL', $e->getTitle()); - self::assertEquals('https://shlink.io/api/error/invalid-url', $e->getType()); - self::assertEquals(['url' => $url], $e->getAdditionalData()); - self::assertEquals(StatusCodeInterface::STATUS_BAD_REQUEST, $e->getCode()); - self::assertEquals(StatusCodeInterface::STATUS_BAD_REQUEST, $e->getStatus()); - self::assertEquals($prev, $e->getPrevious()); - } - - public static function providePrevious(): iterable - { - yield 'null previous' => [null]; - yield 'instance previous' => [new Exception('Previous error', 10)]; - } -} diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php index ae89fa6f..d5c9f833 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php @@ -4,46 +4,131 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\ShortUrl\Helper; -use PHPUnit\Framework\Attributes\DataProvider; +use Exception; +use GuzzleHttp\ClientInterface; +use Laminas\Diactoros\Response; +use Laminas\Diactoros\Response\JsonResponse; +use Laminas\Diactoros\Stream; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlTitleResolutionHelper; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; -use Shlinkio\Shlink\Core\Util\UrlValidatorInterface; class ShortUrlTitleResolutionHelperTest extends TestCase { - private ShortUrlTitleResolutionHelper $helper; - private MockObject & UrlValidatorInterface $urlValidator; + private MockObject & ClientInterface $httpClient; protected function setUp(): void { - $this->urlValidator = $this->createMock(UrlValidatorInterface::class); - $this->helper = new ShortUrlTitleResolutionHelper($this->urlValidator); + $this->httpClient = $this->createMock(ClientInterface::class); } - #[Test, DataProvider('provideTitles')] - public function urlIsProperlyShortened(?string $title, int $validateWithTitleCallsNum, int $validateCallsNum): void + #[Test] + public function dataIsReturnedAsIsWhenResolvingTitlesIsDisabled(): void { - $longUrl = 'http://foobar.com/12345/hello?foo=bar'; - $this->urlValidator->expects($this->exactly($validateWithTitleCallsNum))->method('validateUrlWithTitle')->with( - $longUrl, - $this->isFalse(), - ); - $this->urlValidator->expects($this->exactly($validateCallsNum))->method('validateUrl')->with( - $longUrl, - $this->isFalse(), - ); + $data = ShortUrlCreation::fromRawData(['longUrl' => 'http://foobar.com/12345/hello?foo=bar']); + $this->httpClient->expects($this->never())->method('request'); - $this->helper->processTitleAndValidateUrl( - ShortUrlCreation::fromRawData(['longUrl' => $longUrl, 'title' => $title]), - ); + $result = $this->helper()->processTitle($data); + + self::assertSame($data, $result); } - public static function provideTitles(): iterable + #[Test] + public function dataIsReturnedAsIsWhenItAlreadyHasTitle(): void { - yield 'no title' => [null, 1, 0]; - yield 'title' => ['link title', 0, 1]; + $data = ShortUrlCreation::fromRawData([ + 'longUrl' => 'http://foobar.com/12345/hello?foo=bar', + 'title' => 'foo', + ]); + $this->httpClient->expects($this->never())->method('request'); + + $result = $this->helper(autoResolveTitles: true)->processTitle($data); + + self::assertSame($data, $result); + } + + #[Test] + public function dataIsReturnedAsIsWhenFetchingFails(): void + { + $data = ShortUrlCreation::fromRawData([ + 'longUrl' => 'http://foobar.com/12345/hello?foo=bar', + ]); + $this->httpClient->expects($this->once())->method('request')->willThrowException(new Exception('Error')); + + $result = $this->helper(autoResolveTitles: true)->processTitle($data); + + self::assertSame($data, $result); + } + + #[Test] + public function dataIsReturnedAsIsWhenResponseIsNotHtml(): void + { + $data = ShortUrlCreation::fromRawData([ + 'longUrl' => 'http://foobar.com/12345/hello?foo=bar', + ]); + $this->httpClient->expects($this->once())->method('request')->willReturn(new JsonResponse(['foo' => 'bar'])); + + $result = $this->helper(autoResolveTitles: true)->processTitle($data); + + self::assertSame($data, $result); + } + + #[Test] + public function dataIsReturnedAsIsWhenTitleCannotBeResolvedFromResponse(): void + { + $data = ShortUrlCreation::fromRawData([ + 'longUrl' => 'http://foobar.com/12345/hello?foo=bar', + ]); + $this->httpClient->expects($this->once())->method('request')->willReturn($this->respWithoutTitle()); + + $result = $this->helper(autoResolveTitles: true)->processTitle($data); + + self::assertSame($data, $result); + } + + #[Test] + public function titleIsUpdatedWhenItCanBeResolvedFromResponse(): void + { + $data = ShortUrlCreation::fromRawData([ + 'longUrl' => 'http://foobar.com/12345/hello?foo=bar', + ]); + $this->httpClient->expects($this->once())->method('request')->willReturn($this->respWithTitle()); + + $result = $this->helper(autoResolveTitles: true)->processTitle($data); + + self::assertNotSame($data, $result); + self::assertEquals('Resolved "title"', $result->title); + } + + private function respWithoutTitle(): Response + { + $body = $this->createStreamWithContent('No title'); + return new Response($body, 200, ['Content-Type' => 'text/html']); + } + + private function respWithTitle(): Response + { + $body = $this->createStreamWithContent(' Resolved "title" '); + return new Response($body, 200, ['Content-Type' => 'TEXT/html; charset=utf-8']); + } + + private function createStreamWithContent(string $content): Stream + { + $body = new Stream('php://temp', 'wr'); + $body->write($content); + $body->rewind(); + + return $body; + } + + private function helper(bool $autoResolveTitles = false): ShortUrlTitleResolutionHelper + { + return new ShortUrlTitleResolutionHelper( + $this->httpClient, + new UrlShortenerOptions(autoResolveTitles: $autoResolveTitles), + ); } } diff --git a/module/Core/test/ShortUrl/ShortUrlServiceTest.php b/module/Core/test/ShortUrl/ShortUrlServiceTest.php index 67b10720..dfbf7d75 100644 --- a/module/Core/test/ShortUrl/ShortUrlServiceTest.php +++ b/module/Core/test/ShortUrl/ShortUrlServiceTest.php @@ -63,7 +63,7 @@ class ShortUrlServiceTest extends TestCase )->willReturn($shortUrl); $this->titleResolutionHelper->expects($expectedValidateCalls) - ->method('processTitleAndValidateUrl') + ->method('processTitle') ->with($shortUrlEdit) ->willReturn($shortUrlEdit); @@ -102,10 +102,6 @@ class ShortUrlServiceTest extends TestCase 'maxVisits' => 10, 'longUrl' => 'https://modifiedLongUrl', ]), ApiKey::create()]; - yield 'long URL with validation' => [new InvokedCount(1), ShortUrlEdition::fromRawData([ - 'longUrl' => 'https://modifiedLongUrl', - 'validateUrl' => true, - ]), null]; yield 'device redirects' => [new InvokedCount(0), ShortUrlEdition::fromRawData([ 'deviceLongUrls' => [ DeviceType::IOS->value => 'https://iosLongUrl', diff --git a/module/Core/test/ShortUrl/UrlShortenerTest.php b/module/Core/test/ShortUrl/UrlShortenerTest.php index a442abb3..b332afd2 100644 --- a/module/Core/test/ShortUrl/UrlShortenerTest.php +++ b/module/Core/test/ShortUrl/UrlShortenerTest.php @@ -57,7 +57,7 @@ class UrlShortenerTest extends TestCase { $longUrl = 'http://foobar.com/12345/hello?foo=bar'; $meta = ShortUrlCreation::fromRawData(['longUrl' => $longUrl]); - $this->titleResolutionHelper->expects($this->once())->method('processTitleAndValidateUrl')->with( + $this->titleResolutionHelper->expects($this->once())->method('processTitle')->with( $meta, )->willReturnArgument(0); $this->shortCodeHelper->method('ensureShortCodeUniqueness')->willReturn(true); @@ -90,7 +90,7 @@ class UrlShortenerTest extends TestCase ); $this->shortCodeHelper->expects($this->once())->method('ensureShortCodeUniqueness')->willReturn(false); - $this->titleResolutionHelper->expects($this->once())->method('processTitleAndValidateUrl')->with( + $this->titleResolutionHelper->expects($this->once())->method('processTitle')->with( $meta, )->willReturnArgument(0); @@ -105,7 +105,7 @@ class UrlShortenerTest extends TestCase $repo = $this->createMock(ShortUrlRepository::class); $repo->expects($this->once())->method('findOneMatching')->willReturn($expected); $this->em->expects($this->once())->method('getRepository')->with(ShortUrl::class)->willReturn($repo); - $this->titleResolutionHelper->expects($this->never())->method('processTitleAndValidateUrl'); + $this->titleResolutionHelper->expects($this->never())->method('processTitle'); $this->shortCodeHelper->method('ensureShortCodeUniqueness')->willReturn(true); $result = $this->urlShortener->shorten($meta); diff --git a/module/Core/test/Util/UrlValidatorTest.php b/module/Core/test/Util/UrlValidatorTest.php deleted file mode 100644 index 233d69bd..00000000 --- a/module/Core/test/Util/UrlValidatorTest.php +++ /dev/null @@ -1,176 +0,0 @@ -httpClient = $this->createMock(ClientInterface::class); - } - - #[Test] - public function exceptionIsThrownWhenUrlIsInvalid(): void - { - $this->httpClient->expects($this->once())->method('request')->willThrowException($this->clientException()); - $this->expectException(InvalidUrlException::class); - - $this->urlValidator()->validateUrl('http://foobar.com/12345/hello?foo=bar', true); - } - - #[Test] - public function expectedUrlIsCalledWhenTryingToVerify(): void - { - $expectedUrl = 'http://foobar.com'; - - $this->httpClient->expects($this->once())->method('request')->with( - RequestMethodInterface::METHOD_GET, - $expectedUrl, - $this->callback(function (array $options) { - Assert::assertArrayHasKey(RequestOptions::ALLOW_REDIRECTS, $options); - Assert::assertEquals(['max' => 15], $options[RequestOptions::ALLOW_REDIRECTS]); - Assert::assertArrayHasKey(RequestOptions::IDN_CONVERSION, $options); - Assert::assertTrue($options[RequestOptions::IDN_CONVERSION]); - Assert::assertArrayHasKey(RequestOptions::HEADERS, $options); - Assert::assertArrayHasKey('User-Agent', $options[RequestOptions::HEADERS]); - - return true; - }), - )->willReturn(new Response()); - - $this->urlValidator()->validateUrl($expectedUrl, true); - } - - #[Test] - public function noCheckIsPerformedWhenUrlValidationIsDisabled(): void - { - $this->httpClient->expects($this->never())->method('request'); - $this->urlValidator()->validateUrl('', false); - } - - #[Test] - public function validateUrlWithTitleReturnsNullWhenRequestFailsAndValidationIsDisabled(): void - { - $this->httpClient->expects($this->once())->method('request')->willThrowException($this->clientException()); - - $result = $this->urlValidator(true)->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', false); - - self::assertNull($result); - } - - #[Test] - public function validateUrlWithTitleReturnsNullWhenAutoResolutionIsDisabled(): void - { - $this->httpClient->expects($this->never())->method('request'); - - $result = $this->urlValidator()->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', false); - - self::assertNull($result); - } - - #[Test] - public function validateUrlWithTitleReturnsNullWhenAutoResolutionIsDisabledAndValidationIsEnabled(): void - { - $this->httpClient->expects($this->once())->method('request')->with( - RequestMethodInterface::METHOD_HEAD, - $this->anything(), - $this->anything(), - )->willReturn($this->respWithTitle()); - - $result = $this->urlValidator()->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', true); - - self::assertNull($result); - } - - #[Test] - public function validateUrlWithTitleResolvesTitleWhenAutoResolutionIsEnabled(): void - { - $this->httpClient->expects($this->once())->method('request')->with( - RequestMethodInterface::METHOD_GET, - $this->anything(), - $this->anything(), - )->willReturn($this->respWithTitle()); - - $result = $this->urlValidator(true)->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', true); - - self::assertEquals('Resolved "title"', $result); - } - - #[Test] - public function validateUrlWithTitleReturnsNullWhenAutoResolutionIsEnabledAndReturnedContentTypeIsInvalid(): void - { - $this->httpClient->expects($this->once())->method('request')->with( - RequestMethodInterface::METHOD_GET, - $this->anything(), - $this->anything(), - )->willReturn(new Response('php://memory', 200, ['Content-Type' => 'application/octet-stream'])); - - $result = $this->urlValidator(true)->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', true); - - self::assertNull($result); - } - - #[Test] - public function validateUrlWithTitleReturnsNullWhenAutoResolutionIsEnabledAndBodyDoesNotContainTitle(): void - { - $this->httpClient->expects($this->once())->method('request')->with( - RequestMethodInterface::METHOD_GET, - $this->anything(), - $this->anything(), - )->willReturn( - new Response($this->createStreamWithContent('No title'), 200, ['Content-Type' => 'text/html']), - ); - - $result = $this->urlValidator(true)->validateUrlWithTitle('http://foobar.com/12345/hello?foo=bar', true); - - self::assertNull($result); - } - - private function respWithTitle(): Response - { - $body = $this->createStreamWithContent(' Resolved "title" '); - return new Response($body, 200, ['Content-Type' => 'TEXT/html; charset=utf-8']); - } - - private function createStreamWithContent(string $content): Stream - { - $body = new Stream('php://temp', 'wr'); - $body->write($content); - $body->rewind(); - - return $body; - } - - private function clientException(): ClientException - { - return new ClientException( - '', - new Request(RequestMethodInterface::METHOD_GET, ''), - new Response(), - ); - } - - public function urlValidator(bool $autoResolveTitles = false): UrlValidator - { - return new UrlValidator($this->httpClient, new UrlShortenerOptions(autoResolveTitles: $autoResolveTitles)); - } -} diff --git a/module/Rest/test-api/Action/CreateShortUrlTest.php b/module/Rest/test-api/Action/CreateShortUrlTest.php index efd70666..41a4559b 100644 --- a/module/Rest/test-api/Action/CreateShortUrlTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlTest.php @@ -224,27 +224,6 @@ class CreateShortUrlTest extends ApiTestCase yield ['http://téstb.shlink.io']; // Redirects to http://tést.shlink.io } - #[Test, DataProvider('provideInvalidUrls')] - public function failsToCreateShortUrlWithInvalidLongUrl(string $url, string $version, string $expectedType): void - { - $expectedDetail = sprintf('Provided URL %s is invalid. Try with a different one.', $url); - - [$statusCode, $payload] = $this->createShortUrl(['longUrl' => $url, 'validateUrl' => true], version: $version); - - self::assertEquals(self::STATUS_BAD_REQUEST, $statusCode); - self::assertEquals(self::STATUS_BAD_REQUEST, $payload['status']); - self::assertEquals($expectedType, $payload['type']); - self::assertEquals($expectedDetail, $payload['detail']); - self::assertEquals('Invalid URL', $payload['title']); - self::assertEquals($url, $payload['url']); - } - - public static function provideInvalidUrls(): iterable - { - yield 'API version 2' => ['https://this-has-to-be-invalid.com', '2', 'https://shlink.io/api/error/invalid-url']; - yield 'API version 3' => ['https://this-has-to-be-invalid.com', '3', 'https://shlink.io/api/error/invalid-url']; - } - #[Test, DataProvider('provideInvalidArgumentApiVersions')] public function failsToCreateShortUrlWithoutLongUrl(array $payload, string $version, string $expectedType): void { diff --git a/module/Rest/test-api/Action/EditShortUrlTest.php b/module/Rest/test-api/Action/EditShortUrlTest.php index 89055adb..07fe84d3 100644 --- a/module/Rest/test-api/Action/EditShortUrlTest.php +++ b/module/Rest/test-api/Action/EditShortUrlTest.php @@ -75,28 +75,16 @@ class EditShortUrlTest extends ApiTestCase return $matchingShortUrl['meta'] ?? []; } - #[Test, DataProvider('provideLongUrls')] - public function longUrlCanBeEditedIfItIsValid(string $longUrl, int $expectedStatus, ?string $expectedError): void + public function longUrlCanBeEdited(): void { $shortCode = 'abc123'; $url = sprintf('/short-urls/%s', $shortCode); $resp = $this->callApiWithKey(self::METHOD_PATCH, $url, [RequestOptions::JSON => [ - 'longUrl' => $longUrl, - 'validateUrl' => true, + 'longUrl' => 'https://shlink.io', ]]); - self::assertEquals($expectedStatus, $resp->getStatusCode()); - if ($expectedError !== null) { - $payload = $this->getJsonResponsePayload($resp); - self::assertEquals($expectedError, $payload['type']); - } - } - - public static function provideLongUrls(): iterable - { - yield 'valid URL' => ['https://shlink.io', self::STATUS_OK, null]; - yield 'invalid URL' => ['http://foo', self::STATUS_BAD_REQUEST, 'https://shlink.io/api/error/invalid-url']; + self::assertEquals(self::STATUS_OK, $resp->getStatusCode()); } #[Test, DataProviderExternal(ApiTestDataProviders::class, 'invalidUrlsProvider')]