diff --git a/CHANGELOG.md b/CHANGELOG.md index 626e7a03..51f1facc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,26 @@ 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). +## [3.5.4] - 2023-04-12 +### Added +* *Nothing* + +### Changed +* *Nothing* + +### Deprecated +* *Nothing* + +### Removed +* *Nothing* + +### Fixed +* [#1742](https://github.com/shlinkio/shlink/issues/1742) Fix URLs using schemas which do not contain `//`, like `mailto:`, to no longer be considered valid. +* [#1743](https://github.com/shlinkio/shlink/issues/1743) Fix Error when trying to create short URLs from CLI on an openswoole context. + + Unfortunately the reason are real-time updates do not work with openswoole when outside an openswoole request, so the feature has been disabled for that context. + + ## [3.5.3] - 2023-03-31 ### Added * *Nothing* diff --git a/config/constants.php b/config/constants.php index 77f81fdd..35d4c56e 100644 --- a/config/constants.php +++ b/config/constants.php @@ -13,7 +13,7 @@ const DEFAULT_REDIRECT_STATUS_CODE = RedirectStatus::STATUS_302; // Deprecated. const DEFAULT_REDIRECT_CACHE_LIFETIME = 30; const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory'; const TITLE_TAG_VALUE = '/]*>(.*?)<\/title>/i'; // Matches the value inside a html title tag -const LOOSE_URI_MATCHER = '/(.+)\:\/\/(.+)/i'; // Matches anything starting with a schema. +const LOOSE_URI_MATCHER = '/(.+)\:(.+)/i'; // Matches anything starting with a schema. const DEFAULT_QR_CODE_SIZE = 300; const DEFAULT_QR_CODE_MARGIN = 0; const DEFAULT_QR_CODE_FORMAT = 'png'; diff --git a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php index 6fb1001b..bb332d82 100644 --- a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -161,7 +161,7 @@ class CreateShortUrlCommand extends Command $doValidateUrl = $input->getOption('validate-url'); try { - $shortUrl = $this->urlShortener->shorten(ShortUrlCreation::fromRawData([ + $result = $this->urlShortener->shorten(ShortUrlCreation::fromRawData([ ShortUrlInputFilter::LONG_URL => $longUrl, ShortUrlInputFilter::VALID_SINCE => $input->getOption('valid-since'), ShortUrlInputFilter::VALID_UNTIL => $input->getOption('valid-until'), @@ -176,9 +176,14 @@ class CreateShortUrlCommand extends Command ShortUrlInputFilter::FORWARD_QUERY => !$input->getOption('no-forward-query'), ], $this->options)); + $result->onEventDispatchingError(static fn () => $io->isVerbose() && $io->warning( + 'Short URL properly created, but the real-time updates cannot be notified when generating the ' + . 'short URL from the command line. Migrate to roadrunner in order to bypass this limitation.', + )); + $io->writeln([ sprintf('Processed long URL: %s', $longUrl), - sprintf('Generated short URL: %s', $this->stringifier->stringify($shortUrl)), + sprintf('Generated short URL: %s', $this->stringifier->stringify($result->shortUrl)), ]); return ExitCodes::EXIT_SUCCESS; } catch (InvalidUrlException | NonUniqueSlugException $e) { diff --git a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php index 48f740dc..fd474007 100644 --- a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\CLI\Command\ShortUrl; +use Laminas\ServiceManager\Exception\ServiceNotFoundException; use PHPUnit\Framework\Assert; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; @@ -17,8 +18,10 @@ use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; +use Shlinkio\Shlink\Core\ShortUrl\Model\UrlShorteningResult; use Shlinkio\Shlink\Core\ShortUrl\UrlShortenerInterface; use ShlinkioTest\Shlink\CLI\CliTestUtilsTrait; +use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Tester\CommandTester; class CreateShortUrlCommandTest extends TestCase @@ -51,7 +54,9 @@ class CreateShortUrlCommandTest extends TestCase public function properShortCodeIsCreatedIfLongUrlIsCorrect(): void { $shortUrl = ShortUrl::createFake(); - $this->urlShortener->expects($this->once())->method('shorten')->withAnyParameters()->willReturn($shortUrl); + $this->urlShortener->expects($this->once())->method('shorten')->withAnyParameters()->willReturn( + UrlShorteningResult::withoutErrorOnEventDispatching($shortUrl), + ); $this->stringifier->expects($this->once())->method('stringify')->with($shortUrl)->willReturn( 'stringified_short_url', ); @@ -59,11 +64,12 @@ class CreateShortUrlCommandTest extends TestCase $this->commandTester->execute([ 'longUrl' => 'http://domain.com/foo/bar', '--max-visits' => '3', - ]); + ], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); $output = $this->commandTester->getDisplay(); self::assertEquals(ExitCodes::EXIT_SUCCESS, $this->commandTester->getStatusCode()); self::assertStringContainsString('stringified_short_url', $output); + self::assertStringNotContainsString('but the real-time updates cannot', $output); } #[Test] @@ -106,7 +112,7 @@ class CreateShortUrlCommandTest extends TestCase Assert::assertEquals(['foo', 'bar', 'baz', 'boo', 'zar'], $creation->tags); return true; }), - )->willReturn($shortUrl); + )->willReturn(UrlShorteningResult::withoutErrorOnEventDispatching($shortUrl)); $this->stringifier->expects($this->once())->method('stringify')->with($shortUrl)->willReturn( 'stringified_short_url', ); @@ -129,7 +135,7 @@ class CreateShortUrlCommandTest extends TestCase Assert::assertEquals($expectedDomain, $meta->domain); return true; }), - )->willReturn(ShortUrl::createFake()); + )->willReturn(UrlShorteningResult::withoutErrorOnEventDispatching(ShortUrl::createFake())); $this->stringifier->method('stringify')->with($this->isInstanceOf(ShortUrl::class))->willReturn(''); $input['longUrl'] = 'http://domain.com/foo/bar'; @@ -155,7 +161,7 @@ class CreateShortUrlCommandTest extends TestCase Assert::assertEquals($expectedValidateUrl, $meta->doValidateUrl()); return true; }), - )->willReturn($shortUrl); + )->willReturn(UrlShorteningResult::withoutErrorOnEventDispatching($shortUrl)); $this->stringifier->method('stringify')->with($this->isInstanceOf(ShortUrl::class))->willReturn(''); $options['longUrl'] = 'http://domain.com/foo/bar'; @@ -167,4 +173,40 @@ class CreateShortUrlCommandTest extends TestCase yield 'no flags' => [[], null]; yield 'validate-url' => [['--validate-url' => true], true]; } + + /** + * @param callable(string $output): void $assert + */ + #[Test, DataProvider('provideDispatchBehavior')] + public function warningIsPrintedInVerboseModeWhenDispatchErrors(int $verbosity, callable $assert): void + { + $shortUrl = ShortUrl::createFake(); + $this->urlShortener->expects($this->once())->method('shorten')->withAnyParameters()->willReturn( + UrlShorteningResult::withErrorOnEventDispatching($shortUrl, new ServiceNotFoundException()), + ); + $this->stringifier->method('stringify')->willReturn('stringified_short_url'); + + $this->commandTester->execute(['longUrl' => 'http://domain.com/foo/bar'], ['verbosity' => $verbosity]); + $output = $this->commandTester->getDisplay(); + + $assert($output); + } + + public static function provideDispatchBehavior(): iterable + { + $containsAssertion = static fn (string $output) => self::assertStringContainsString( + 'but the real-time updates cannot', + $output, + ); + $doesNotContainAssertion = static fn (string $output) => self::assertStringNotContainsString( + 'but the real-time updates cannot', + $output, + ); + + yield 'quiet' => [OutputInterface::VERBOSITY_QUIET, $doesNotContainAssertion]; + yield 'normal' => [OutputInterface::VERBOSITY_NORMAL, $doesNotContainAssertion]; + yield 'verbose' => [OutputInterface::VERBOSITY_VERBOSE, $containsAssertion]; + yield 'very verbose' => [OutputInterface::VERBOSITY_VERY_VERBOSE, $containsAssertion]; + yield 'debug' => [OutputInterface::VERBOSITY_DEBUG, $containsAssertion]; + } } diff --git a/module/Core/src/ShortUrl/Model/UrlShorteningResult.php b/module/Core/src/ShortUrl/Model/UrlShorteningResult.php new file mode 100644 index 00000000..b9d4f993 --- /dev/null +++ b/module/Core/src/ShortUrl/Model/UrlShorteningResult.php @@ -0,0 +1,37 @@ +errorOnEventDispatching !== null) { + $handler($this->errorOnEventDispatching); + } + } + + public static function withoutErrorOnEventDispatching(ShortUrl $shortUrl): self + { + return new self($shortUrl, null); + } + + public static function withErrorOnEventDispatching(ShortUrl $shortUrl, Throwable $errorOnEventDispatching): self + { + return new self($shortUrl, $errorOnEventDispatching); + } +} diff --git a/module/Core/src/ShortUrl/UrlShortener.php b/module/Core/src/ShortUrl/UrlShortener.php index 7477052f..7bb74ba6 100644 --- a/module/Core/src/ShortUrl/UrlShortener.php +++ b/module/Core/src/ShortUrl/UrlShortener.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl; 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; @@ -13,6 +14,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortCodeUniquenessHelperInterface; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlTitleResolutionHelperInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; +use Shlinkio\Shlink\Core\ShortUrl\Model\UrlShorteningResult; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlRepositoryInterface; use Shlinkio\Shlink\Core\ShortUrl\Resolver\ShortUrlRelationResolverInterface; @@ -31,12 +33,12 @@ class UrlShortener implements UrlShortenerInterface * @throws NonUniqueSlugException * @throws InvalidUrlException */ - public function shorten(ShortUrlCreation $creation): ShortUrl + public function shorten(ShortUrlCreation $creation): UrlShorteningResult { // First, check if a short URL exists for all provided params $existingShortUrl = $this->findExistingShortUrlIfExists($creation); if ($existingShortUrl !== null) { - return $existingShortUrl; + return UrlShorteningResult::withoutErrorOnEventDispatching($existingShortUrl); } $creation = $this->titleResolutionHelper->processTitleAndValidateUrl($creation); @@ -51,9 +53,17 @@ class UrlShortener implements UrlShortenerInterface return $shortUrl; }); - $this->eventDispatcher->dispatch(new ShortUrlCreated($newShortUrl->getId())); + try { + $this->eventDispatcher->dispatch(new ShortUrlCreated($newShortUrl->getId())); + } catch (ContainerExceptionInterface $e) { + // Ignore container errors when dispatching the event. + // When using openswoole, this event will try to enqueue a task, which cannot be done outside an HTTP + // request. + // If the short URL is created from CLI, the event dispatching will fail. + return UrlShorteningResult::withErrorOnEventDispatching($newShortUrl, $e); + } - return $newShortUrl; + return UrlShorteningResult::withoutErrorOnEventDispatching($newShortUrl); } private function findExistingShortUrlIfExists(ShortUrlCreation $creation): ?ShortUrl diff --git a/module/Core/src/ShortUrl/UrlShortenerInterface.php b/module/Core/src/ShortUrl/UrlShortenerInterface.php index d7ae45f0..70896ec1 100644 --- a/module/Core/src/ShortUrl/UrlShortenerInterface.php +++ b/module/Core/src/ShortUrl/UrlShortenerInterface.php @@ -6,8 +6,8 @@ namespace Shlinkio\Shlink\Core\ShortUrl; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; -use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; +use Shlinkio\Shlink\Core\ShortUrl\Model\UrlShorteningResult; interface UrlShortenerInterface { @@ -15,5 +15,5 @@ interface UrlShortenerInterface * @throws NonUniqueSlugException * @throws InvalidUrlException */ - public function shorten(ShortUrlCreation $creation): ShortUrl; + public function shorten(ShortUrlCreation $creation): UrlShorteningResult; } diff --git a/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php b/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php index bb82ee8e..401a3a31 100644 --- a/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php +++ b/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php @@ -165,6 +165,21 @@ class ShortUrlCreationTest extends TestCase yield ['гугл', 'гугл']; } + #[Test, DataProvider('provideValidLongUrls')] + public function supportsDifferentTypesOfSchemas(string $longUrl): void + { + $creation = ShortUrlCreation::fromRawData(['longUrl' => $longUrl]); + self::assertEquals($longUrl, $creation->longUrl); + } + + public static function provideValidLongUrls(): iterable + { + yield 'mailto' => ['mailto:foo@example.com']; + yield 'file' => ['file:///foo/bar']; + yield 'https' => ['https://example.com']; + yield 'deeplink' => ['shlink://some/path']; + } + #[Test, DataProvider('provideTitles')] public function titleIsCroppedIfTooLong(?string $title, ?string $expectedTitle): void { diff --git a/module/Core/test/ShortUrl/UrlShortenerTest.php b/module/Core/test/ShortUrl/UrlShortenerTest.php index 713fee95..a442abb3 100644 --- a/module/Core/test/ShortUrl/UrlShortenerTest.php +++ b/module/Core/test/ShortUrl/UrlShortenerTest.php @@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\Core\ShortUrl; use Cake\Chronos\Chronos; use Doctrine\ORM\EntityManager; +use Laminas\ServiceManager\Exception\ServiceNotFoundException; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; @@ -26,6 +27,7 @@ class UrlShortenerTest extends TestCase private MockObject & EntityManager $em; private MockObject & ShortUrlTitleResolutionHelperInterface $titleResolutionHelper; private MockObject & ShortCodeUniquenessHelperInterface $shortCodeHelper; + private MockObject & EventDispatcherInterface $dispatcher; protected function setUp(): void { @@ -39,17 +41,19 @@ class UrlShortenerTest extends TestCase fn (callable $callback) => $callback(), ); + $this->dispatcher = $this->createMock(EventDispatcherInterface::class); + $this->urlShortener = new UrlShortener( $this->titleResolutionHelper, $this->em, new SimpleShortUrlRelationResolver(), $this->shortCodeHelper, - $this->createMock(EventDispatcherInterface::class), + $this->dispatcher, ); } - #[Test] - public function urlIsProperlyShortened(): void + #[Test, DataProvider('provideDispatchBehavior')] + public function urlIsProperlyShortened(bool $expectDispatchError, callable $dispatchBehavior): void { $longUrl = 'http://foobar.com/12345/hello?foo=bar'; $meta = ShortUrlCreation::fromRawData(['longUrl' => $longUrl]); @@ -57,10 +61,25 @@ class UrlShortenerTest extends TestCase $meta, )->willReturnArgument(0); $this->shortCodeHelper->method('ensureShortCodeUniqueness')->willReturn(true); + $this->dispatcher->expects($this->once())->method('dispatch')->willReturnCallback($dispatchBehavior); - $shortUrl = $this->urlShortener->shorten($meta); + $result = $this->urlShortener->shorten($meta); + $thereIsError = false; + $result->onEventDispatchingError(function () use (&$thereIsError): void { + $thereIsError = true; + }); - self::assertEquals($longUrl, $shortUrl->getLongUrl()); + self::assertEquals($longUrl, $result->shortUrl->getLongUrl()); + self::assertEquals($expectDispatchError, $thereIsError); + } + + public static function provideDispatchBehavior(): iterable + { + yield 'no dispatch error' => [false, static function (): void { + }]; + yield 'dispatch error' => [true, static function (): void { + throw new ServiceNotFoundException(); + }]; } #[Test] @@ -91,7 +110,7 @@ class UrlShortenerTest extends TestCase $result = $this->urlShortener->shorten($meta); - self::assertSame($expected, $result); + self::assertSame($expected, $result->shortUrl); } public static function provideExistingShortUrls(): iterable diff --git a/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php index 288033ed..914efde5 100644 --- a/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/AbstractCreateShortUrlAction.php @@ -26,9 +26,9 @@ abstract class AbstractCreateShortUrlAction extends AbstractRestAction public function handle(Request $request): Response { $shortUrlMeta = $this->buildShortUrlData($request); - $shortUrl = $this->urlShortener->shorten($shortUrlMeta); + $result = $this->urlShortener->shorten($shortUrlMeta); - return new JsonResponse($this->transformer->transform($shortUrl)); + return new JsonResponse($this->transformer->transform($result->shortUrl)); } /** diff --git a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php index acaf8df0..c79eab86 100644 --- a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php @@ -17,6 +17,7 @@ use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; +use Shlinkio\Shlink\Core\ShortUrl\Model\UrlShorteningResult; use Shlinkio\Shlink\Core\ShortUrl\UrlShortener; use Shlinkio\Shlink\Rest\Action\ShortUrl\CreateShortUrlAction; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -53,7 +54,7 @@ class CreateShortUrlActionTest extends TestCase $this->urlShortener->expects($this->once())->method('shorten')->with( ShortUrlCreation::fromRawData($expectedMeta), - )->willReturn($shortUrl); + )->willReturn(UrlShorteningResult::withoutErrorOnEventDispatching($shortUrl)); $this->transformer->expects($this->once())->method('transform')->with($shortUrl)->willReturn( ['shortUrl' => 'stringified_short_url'], ); diff --git a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php index d343a3cd..c1ac6544 100644 --- a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php @@ -12,6 +12,7 @@ use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; +use Shlinkio\Shlink\Core\ShortUrl\Model\UrlShorteningResult; use Shlinkio\Shlink\Core\ShortUrl\UrlShortenerInterface; use Shlinkio\Shlink\Rest\Action\ShortUrl\SingleStepCreateShortUrlAction; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -44,7 +45,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase ])->withAttribute(ApiKey::class, $apiKey); $this->urlShortener->expects($this->once())->method('shorten')->with( ShortUrlCreation::fromRawData(['apiKey' => $apiKey, 'longUrl' => 'http://foobar.com']), - )->willReturn(ShortUrl::createFake()); + )->willReturn(UrlShorteningResult::withoutErrorOnEventDispatching(ShortUrl::createFake())); $resp = $this->action->handle($request);