Merge pull request #2136 from acelaya-forks/release/4.1.1

Release/4.1.1
2024-12-23 07:33:58 -06:00 · 2024-05-23 09:21:56 +02:00 · 2024-05-23 09:21:56 +02:00 · b2dabf06bf
commit b2dabf06bf
parent 35508e253d 67ae05f473
15 changed files with 112 additions and 58 deletions
--- a/.github/workflows/publish-docker-image.yml
+++ b/.github/workflows/publish-docker-image.yml
@ -15,7 +15,7 @@ jobs:
          - runtime: 'rr'
            tag-suffix: 'roadrunner'
            platforms: 'linux/arm64/v8,linux/amd64'
-    uses: shlinkio/github-actions/.github/workflows/docker-build-and-publish.yml@main
+    uses: shlinkio/github-actions/.github/workflows/docker-publish-image.yml@main
    secrets: inherit
    with:
      image-name: shlinkio/shlink
--- 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).

+## [4.1.1] - 2024-05-23
+### Added
+* *Nothing*
+
+### Changed
+* Use new reusable workflow to publish docker image
+* [#2015](https://github.com/shlinkio/shlink/issues/2015) Update to PHPUnit 11.
+* [#2130](https://github.com/shlinkio/shlink/pull/2130) Replace deprecated `pugx/shortid-php` package with `hidehalo/nanoid-php`.
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* [#2111](https://github.com/shlinkio/shlink/issues/2111) Fix typo in OAS docs examples where redirect rules with `query-param` condition type were defined as `query`.
+* [#2129](https://github.com/shlinkio/shlink/issues/2129) Fix error when resolving title for sites not using UTF-8 charset (detected with Japanese charsets).
+
+
 ## [4.1.0] - 2024-04-14
 ### Added
 * [#1330](https://github.com/shlinkio/shlink/issues/1330) All visit-related endpoints now expose the `visitedUrl` prop for any visit.
@ -824,3 +844,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this

 ### Fixed
 * *Nothing*
+
+
+## Older versions
+* [2.x.x](docs/changelog-archive/CHANGELOG-2.x.md)
+* [1.x.x](docs/changelog-archive/CHANGELOG-1.x.md)
--- a/composer.json
+++ b/composer.json
@ -16,6 +16,7 @@
        "ext-curl": "*",
        "ext-gd": "*",
        "ext-json": "*",
+        "ext-mbstring": "*",
        "ext-pdo": "*",
        "akrabat/ip-address-middleware": "^2.1",
        "cakephp/chronos": "^3.0.2",
@ -26,9 +27,10 @@
        "friendsofphp/proxy-manager-lts": "^1.0",
        "geoip2/geoip2": "^3.0",
        "guzzlehttp/guzzle": "^7.5",
+        "hidehalo/nanoid-php": "^1.1",
        "jaybizzle/crawler-detect": "^1.2.116",
        "laminas/laminas-config": "^3.8",
-        "laminas/laminas-config-aggregator": "^1.13",
+        "laminas/laminas-config-aggregator": "^1.15",
        "laminas/laminas-diactoros": "^3.3",
        "laminas/laminas-inputfilter": "^2.27",
        "laminas/laminas-servicemanager": "^3.21",
@ -40,7 +42,6 @@
        "mlocati/ip-lib": "^1.18",
        "mobiledetect/mobiledetectlib": "^4.8",
        "pagerfanta/core": "^3.8",
-        "pugx/shortid-php": "^1.1",
        "ramsey/uuid": "^4.7",
        "shlinkio/doctrine-specification": "^2.1.1",
        "shlinkio/shlink-common": "^6.1",
@ -63,13 +64,13 @@
    "require-dev": {
        "devizzent/cebe-php-openapi": "^1.0.1",
        "devster/ubench": "^2.1",
-        "phpstan/phpstan": "^1.10",
-        "phpstan/phpstan-doctrine": "^1.3",
-        "phpstan/phpstan-phpunit": "^1.3",
-        "phpstan/phpstan-symfony": "^1.3",
-        "phpunit/php-code-coverage": "^10.1",
-        "phpunit/phpcov": "^9.0",
-        "phpunit/phpunit": "^10.4",
+        "phpstan/phpstan": "^1.11",
+        "phpstan/phpstan-doctrine": "^1.4",
+        "phpstan/phpstan-phpunit": "^1.4",
+        "phpstan/phpstan-symfony": "^1.4",
+        "phpunit/php-code-coverage": "^11.0",
+        "phpunit/phpcov": "^10.0",
+        "phpunit/phpunit": "^11.1",
        "roave/security-advisories": "dev-master",
        "shlinkio/php-coding-standard": "~2.3.0",
        "shlinkio/shlink-test-utils": "^4.1",
--- a/config/constants.php
+++ b/config/constants.php
@ -12,7 +12,6 @@ const MIN_SHORT_CODES_LENGTH = 4;
 const DEFAULT_REDIRECT_STATUS_CODE = RedirectStatus::STATUS_302;
 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 DEFAULT_QR_CODE_SIZE = 300;
 const DEFAULT_QR_CODE_MARGIN = 0;
--- a/docker-compose.yml
+++ b/docker-compose.yml
@ -105,7 +105,7 @@ services:

    shlink_db_ms:
        container_name: shlink_db_ms
-        image: mcr.microsoft.com/mssql/server:2019-latest
+        image: mcr.microsoft.com/mssql/server:2022-latest
        ports:
            - "1433:1433"
        environment:
--- a/docs/swagger/paths/v3_short-urls_{shortCode}_redirect-rules.json
+++ b/docs/swagger/paths/v3_short-urls_{shortCode}_redirect-rules.json
@ -77,12 +77,12 @@
                  "priority": 3,
                  "conditions": [
                    {
-                      "type": "query",
+                      "type": "query-param",
                      "matchKey": "foo",
                      "matchValue": "bar"
                    },
                    {
-                      "type": "query",
+                      "type": "query-param",
                      "matchKey": "hello",
                      "matchValue": "world"
                    }
@ -209,12 +209,12 @@
                "longUrl": "https://example.com/query-foo-bar-hello-world",
                "conditions": [
                  {
-                    "type": "query",
+                    "type": "query-param",
                    "matchKey": "foo",
                    "matchValue": "bar"
                  },
                  {
-                    "type": "query",
+                    "type": "query-param",
                    "matchKey": "hello",
                    "matchValue": "world"
                  }
@ -280,12 +280,12 @@
                  "priority": 3,
                  "conditions": [
                    {
-                      "type": "query",
+                      "type": "query-param",
                      "matchKey": "foo",
                      "matchValue": "bar"
                    },
                    {
-                      "type": "query",
+                      "type": "query-param",
                      "matchKey": "hello",
                      "matchValue": "world"
                    }
--- a/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php
+++ b/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php
@ -34,8 +34,8 @@ class MatomoSendVisitsCommandTest extends TestCase
    }

    #[Test]
-    #[TestWith([true])]
-    #[TestWith([false])]
+    #[TestWith([true], 'interactive')]
+    #[TestWith([false], 'not interactive')]
    public function warningIsOnlyDisplayedInInteractiveMode(bool $interactive): void
    {
        $this->visitSender->method('sendVisitsInDateRange')->willReturn(new SendVisitsResult());
--- a/module/CLI/test/Util/CliTestUtils.php
+++ b/module/CLI/test/Util/CliTestUtils.php
@ -25,6 +25,7 @@ class CliTestUtils
        $command = $generator->testDouble(
            Command::class,
            mockObject: true,
+            markAsMockObject: true,
            callOriginalConstructor: false,
            callOriginalClone: false,
            cloneArguments: false,
--- a/module/Core/functions/functions.php
+++ b/module/Core/functions/functions.php
@ -9,11 +9,11 @@ use Cake\Chronos\Chronos;
 use DateTimeInterface;
 use Doctrine\ORM\Mapping\Builder\FieldBuilder;
 use GuzzleHttp\Psr7\Query;
+use Hidehalo\Nanoid\Client as NanoidClient;
 use Jaybizzle\CrawlerDetect\CrawlerDetect;
 use Laminas\Filter\Word\CamelCaseToSeparator;
 use Laminas\Filter\Word\CamelCaseToUnderscore;
 use Laminas\InputFilter\InputFilter;
-use PUGX\Shortid\Factory as ShortIdFactory;
 use Shlinkio\Shlink\Common\Util\DateRange;
 use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode;

@ -37,15 +37,15 @@ use function ucfirst;

 function generateRandomShortCode(int $length, ShortUrlMode $mode = ShortUrlMode::STRICT): string
 {
-    static $shortIdFactory;
-    if ($shortIdFactory === null) {
-        $shortIdFactory = new ShortIdFactory();
+    static $nanoIdClient;
+    if ($nanoIdClient === null) {
+        $nanoIdClient = new NanoidClient();
    }

    $alphabet = $mode === ShortUrlMode::STRICT
        ? '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'
        : '0123456789abcdefghijklmnopqrstuvwxyz';
-    return $shortIdFactory->generate($length, $alphabet)->serialize();
+    return $nanoIdClient->formattedId($alphabet, $length);
 }

 function parseDateFromQuery(array $query, string $dateName): ?Chronos
--- a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php
+++ b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php
@ -12,20 +12,24 @@ use Shlinkio\Shlink\Core\Options\UrlShortenerOptions;
 use Throwable;

 use function html_entity_decode;
+use function mb_convert_encoding;
 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 const MAX_REDIRECTS = 15;
    public 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';

+    // Matches the value inside a html title tag
+    private const TITLE_TAG_VALUE = '/<title[^>]*>(.*?)<\/title>/i';
+    // Matches the charset inside a Content-Type header
+    private const CHARSET_VALUE = '/charset=([^;]+)/i';
+
    public function __construct(
        private ClientInterface $httpClient,
        private UrlShortenerOptions $options,
@ -53,7 +57,7 @@ readonly class ShortUrlTitleResolutionHelper implements ShortUrlTitleResolutionH
            return $data;
        }

-        $title = $this->tryToResolveTitle($response);
+        $title = $this->tryToResolveTitle($response, $contentType);
        return $title !== null ? $data->withResolvedTitle($title) : $data;
    }

@ -76,7 +80,7 @@ readonly class ShortUrlTitleResolutionHelper implements ShortUrlTitleResolutionH
        }
    }

-    private function tryToResolveTitle(ResponseInterface $response): ?string
+    private function tryToResolveTitle(ResponseInterface $response, string $contentType): ?string
    {
        $collectedBody = '';
        $body = $response->getBody();
@ -84,12 +88,19 @@ readonly class ShortUrlTitleResolutionHelper implements ShortUrlTitleResolutionH
        while (! str_contains($collectedBody, '</title>') && ! $body->eof()) {
            $collectedBody .= $body->read(1024);
        }
-        preg_match(TITLE_TAG_VALUE, $collectedBody, $matches);
-        return isset($matches[1]) ? $this->normalizeTitle($matches[1]) : null;
+
+        // Try to match the title from the <title /> tag
+        preg_match(self::TITLE_TAG_VALUE, $collectedBody, $titleMatches);
+        if (! isset($titleMatches[1])) {
+            return null;
        }

-    private function normalizeTitle(string $title): string
-    {
+        // Get the page's charset from Content-Type header
+        preg_match(self::CHARSET_VALUE, $contentType, $charsetMatches);
+
+        $title = isset($charsetMatches[1])
+            ? mb_convert_encoding($titleMatches[1], 'utf8', $charsetMatches[1])
+            : $titleMatches[1];
        return html_entity_decode(trim($title));
    }
 }
--- a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php
+++ b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php
@ -10,6 +10,7 @@ use PHPUnit\Framework\Assert;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\MockObject\Rule\InvokedCount as InvokedCountMatcher;
 use PHPUnit\Framework\TestCase;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Message\UriInterface;
@ -60,14 +61,19 @@ class NotFoundRedirectHandlerTest extends TestCase

    public static function provideNonRedirectScenarios(): iterable
    {
+        $exactly = static fn (int $expectedCount) => new InvokedCountMatcher($expectedCount);
+        $once = static fn () => $exactly(1);
+
        yield 'no domain' => [function (
            MockObject&DomainServiceInterface $domainService,
            MockObject&NotFoundRedirectResolverInterface $resolver,
+        ) use (
+            $once,
        ): void {
-            $domainService->expects(self::once())->method('findByAuthority')->withAnyParameters()->willReturn(
+            $domainService->expects($once())->method('findByAuthority')->withAnyParameters()->willReturn(
                null,
            );
-            $resolver->expects(self::once())->method('resolveRedirectResponse')->with(
+            $resolver->expects($once())->method('resolveRedirectResponse')->with(
                self::isInstanceOf(NotFoundType::class),
                self::isInstanceOf(NotFoundRedirectOptions::class),
                self::isInstanceOf(UriInterface::class),
@ -76,12 +82,15 @@ class NotFoundRedirectHandlerTest extends TestCase
        yield 'non-redirecting domain' => [function (
            MockObject&DomainServiceInterface $domainService,
            MockObject&NotFoundRedirectResolverInterface $resolver,
+        ) use (
+            $once,
+            $exactly,
        ): void {
-            $domainService->expects(self::once())->method('findByAuthority')->withAnyParameters()->willReturn(
+            $domainService->expects($once())->method('findByAuthority')->withAnyParameters()->willReturn(
                Domain::withAuthority(''),
            );
            $callCount = 0;
-            $resolver->expects(self::exactly(2))->method('resolveRedirectResponse')->willReturnCallback(
+            $resolver->expects($exactly(2))->method('resolveRedirectResponse')->willReturnCallback(
                function (mixed $arg1, mixed $arg2, mixed $arg3) use (&$callCount) {
                    Assert::assertInstanceOf(NotFoundType::class, $arg1);
                    Assert::assertInstanceOf($callCount === 0 ? Domain::class : NotFoundRedirectOptions::class, $arg2);
--- a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php
+++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php
@ -10,6 +10,7 @@ use Exception;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\MockObject\Rule\InvokedCount as InvokedCountMatcher;
 use PHPUnit\Framework\TestCase;
 use Psr\Log\LoggerInterface;
 use RuntimeException;
@ -146,30 +147,34 @@ class NotifyVisitToRabbitMqTest extends TestCase

    public static function providePayloads(): iterable
    {
+        $exactly = static fn (int $expectedCount) => new InvokedCountMatcher($expectedCount);
+        $once = static fn () => $exactly(1);
+        $never = static fn () => $exactly(0);
+
        yield 'non-orphan visit' => [
            Visit::forValidShortUrl(ShortUrl::withLongUrl('https://longUrl'), Visitor::emptyInstance()),
-            function (MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator): void {
+            function (MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator) use ($once, $never): void {
                $update = Update::forTopicAndPayload('', []);
-                $updatesGenerator->expects(self::never())->method('newOrphanVisitUpdate');
-                $updatesGenerator->expects(self::once())->method('newVisitUpdate')->withAnyParameters()->willReturn(
+                $updatesGenerator->expects($never())->method('newOrphanVisitUpdate');
+                $updatesGenerator->expects($once())->method('newVisitUpdate')->withAnyParameters()->willReturn(
                    $update,
                );
-                $updatesGenerator->expects(self::once())->method('newShortUrlVisitUpdate')->willReturn($update);
+                $updatesGenerator->expects($once())->method('newShortUrlVisitUpdate')->willReturn($update);
            },
-            function (MockObject & PublishingHelperInterface $helper): void {
-                $helper->expects(self::exactly(2))->method('publishUpdate')->with(self::isInstanceOf(Update::class));
+            function (MockObject & PublishingHelperInterface $helper) use ($exactly): void {
+                $helper->expects($exactly(2))->method('publishUpdate')->with(self::isInstanceOf(Update::class));
            },
        ];
        yield 'orphan visit' => [
            Visit::forBasePath(Visitor::emptyInstance()),
-            function (MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator): void {
+            function (MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator) use ($once, $never): void {
                $update = Update::forTopicAndPayload('', []);
-                $updatesGenerator->expects(self::once())->method('newOrphanVisitUpdate')->willReturn($update);
-                $updatesGenerator->expects(self::never())->method('newVisitUpdate');
-                $updatesGenerator->expects(self::never())->method('newShortUrlVisitUpdate');
+                $updatesGenerator->expects($once())->method('newOrphanVisitUpdate')->willReturn($update);
+                $updatesGenerator->expects($never())->method('newVisitUpdate');
+                $updatesGenerator->expects($never())->method('newShortUrlVisitUpdate');
            },
-            function (MockObject & PublishingHelperInterface $helper): void {
-                $helper->expects(self::once())->method('publishUpdate')->with(self::isInstanceOf(Update::class));
+            function (MockObject & PublishingHelperInterface $helper) use ($once): void {
+                $helper->expects($once())->method('publishUpdate')->with(self::isInstanceOf(Update::class));
            },
        ];
    }
--- a/module/Core/test/Matomo/MatomoTrackerBuilderTest.php
+++ b/module/Core/test/Matomo/MatomoTrackerBuilderTest.php
@ -37,8 +37,8 @@ class MatomoTrackerBuilderTest extends TestCase
    {
        $tracker = $this->builder()->buildMatomoTracker();

-        self::assertEquals('api_token', $tracker->token_auth); // @phpstan-ignore-line
-        self::assertEquals(5, $tracker->idSite); // @phpstan-ignore-line
+        self::assertEquals('api_token', $tracker->token_auth);
+        self::assertEquals(5, $tracker->idSite);
        self::assertEquals(MatomoTrackerBuilder::MATOMO_DEFAULT_TIMEOUT, $tracker->getRequestTimeout());
        self::assertEquals(MatomoTrackerBuilder::MATOMO_DEFAULT_TIMEOUT, $tracker->getRequestConnectTimeout());
    }
--- a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php
+++ b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php
@ -12,6 +12,7 @@ use Laminas\Diactoros\Response;
 use Laminas\Diactoros\Response\JsonResponse;
 use Laminas\Diactoros\Stream;
 use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\Attributes\TestWith;
 use PHPUnit\Framework\MockObject\Builder\InvocationMocker;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
@ -89,10 +90,12 @@ class ShortUrlTitleResolutionHelperTest extends TestCase
    }

    #[Test]
-    public function titleIsUpdatedWhenItCanBeResolvedFromResponse(): void
+    #[TestWith(['TEXT/html; charset=utf-8'], name: 'charset')]
+    #[TestWith(['TEXT/html'], name: 'no charset')]
+    public function titleIsUpdatedWhenItCanBeResolvedFromResponse(string $contentType): void
    {
        $data = ShortUrlCreation::fromRawData(['longUrl' => self::LONG_URL]);
-        $this->expectRequestToBeCalled()->willReturn($this->respWithTitle());
+        $this->expectRequestToBeCalled()->willReturn($this->respWithTitle($contentType));

        $result = $this->helper(autoResolveTitles: true)->processTitle($data);

@ -122,10 +125,10 @@ class ShortUrlTitleResolutionHelperTest extends TestCase
        return new Response($body, 200, ['Content-Type' => 'text/html']);
    }

-    private function respWithTitle(): Response
+    private function respWithTitle(string $contentType): Response
    {
        $body = $this->createStreamWithContent('<title data-foo="bar">  Resolved &quot;title&quot; </title>');
-        return new Response($body, 200, ['Content-Type' => 'TEXT/html; charset=utf-8']);
+        return new Response($body, 200, ['Content-Type' => $contentType]);
    }

    private function createStreamWithContent(string $content): Stream
--- a/phpstan.neon
+++ b/phpstan.neon
@ -4,8 +4,6 @@ includes:
    - vendor/phpstan/phpstan-phpunit/extension.neon
    - vendor/phpstan/phpstan-phpunit/rules.neon
 parameters:
-    checkMissingIterableValueType: false
-    checkGenericClassInNonGenericObjectType: false
    symfony:
        console_application_loader: 'config/cli-app.php'
    doctrine:
@ -14,3 +12,5 @@ parameters:
    ignoreErrors:
        - '#should return int<0, max> but returns int#'
        - '#expects -1|int<1, max>, int given#'
+        - identifier: missingType.generics
+        - identifier: missingType.iterableValue