Merge pull request #1749 from shlinkio/develop

Release 3.5.4

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*

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[^>]*>(.*?)<\/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';

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: <info>%s</info>', $longUrl),
-                sprintf('Generated short URL: <info>%s</info>', $this->stringifier->stringify($shortUrl)),
+                sprintf('Generated short URL: <info>%s</info>', $this->stringifier->stringify($result->shortUrl)),
             ]);
             return ExitCodes::EXIT_SUCCESS;
         } catch (InvalidUrlException | NonUniqueSlugException $e) {

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];
+    }
 }

module/Core/src/ShortUrl/Model/UrlShorteningResult.php

@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\ShortUrl\Model;
+
+use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
+use Throwable;
+
+final class UrlShorteningResult
+{
+    private function __construct(
+        public readonly ShortUrl $shortUrl,
+        private readonly ?Throwable $errorOnEventDispatching,
+    ) {
+    }
+
+    /**
+     * @param callable(Throwable $errorOnEventDispatching): mixed $handler
+     */
+    public function onEventDispatchingError(callable $handler): void
+    {
+        if ($this->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);
+    }
+}

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;
         });
 
+        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

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

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
     {

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]
+    #[Test, DataProvider('provideDispatchBehavior')]
-    public function urlIsProperlyShortened(): void
+    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

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));
     }
 
     /**

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

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