diff --git a/.github/workflows/publish-docker-image.yml b/.github/workflows/publish-docker-image.yml index a57ebe41c..32bbcda7d 100644 --- 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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 08738f515..cbbccc47e 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). +## [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) diff --git a/composer.json b/composer.json index 517caf0ca..fe2644ce2 100644 --- 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", diff --git a/config/constants.php b/config/constants.php index 51ee04762..20c64f192 100644 --- a/config/constants.php +++ b/config/constants.php @@ -12,7 +12,6 @@ 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>/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; diff --git a/docker-compose.yml b/docker-compose.yml index 5bccfd482..414d7adba 100644 --- 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: diff --git a/docs/swagger/paths/v3_short-urls_{shortCode}_redirect-rules.json b/docs/swagger/paths/v3_short-urls_{shortCode}_redirect-rules.json index b87e26cb9..ecb806936 100644 --- 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" } diff --git a/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php b/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php index e3a527332..78d2f8285 100644 --- a/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php +++ b/module/CLI/test/Command/Integration/MatomoSendVisitsCommandTest.php @@ -34,8 +34,8 @@ public function warningDisplayedIfIntegrationIsNotEnabled(): void } #[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()); diff --git a/module/CLI/test/Util/CliTestUtils.php b/module/CLI/test/Util/CliTestUtils.php index 9c92f882c..c18186ba1 100644 --- a/module/CLI/test/Util/CliTestUtils.php +++ b/module/CLI/test/Util/CliTestUtils.php @@ -25,6 +25,7 @@ public static function createCommandMock(string $name): MockObject & Command $command = $generator->testDouble( Command::class, mockObject: true, + markAsMockObject: true, callOriginalConstructor: false, callOriginalClone: false, cloneArguments: false, diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 2e238e997..d9fc2b5ee 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -9,11 +9,11 @@ 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 @@ 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 diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php index e91b1ff11..3f9b62256 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php @@ -12,20 +12,24 @@ 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>/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 @@ public function processTitle(TitleResolutionModelInterface $data): TitleResoluti return $data; } - $title = $this->tryToResolveTitle($response); + $title = $this->tryToResolveTitle($response, $contentType); return $title !== null ? $data->withResolvedTitle($title) : $data; } @@ -76,7 +80,7 @@ private function fetchUrl(string $url): ?ResponseInterface } } - private function tryToResolveTitle(ResponseInterface $response): ?string + private function tryToResolveTitle(ResponseInterface $response, string $contentType): ?string { $collectedBody = ''; $body = $response->getBody(); @@ -84,12 +88,19 @@ private function tryToResolveTitle(ResponseInterface $response): ?string 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 - { + // Try to match the title from the tag + preg_match(self::TITLE_TAG_VALUE, $collectedBody, $titleMatches); + if (! isset($titleMatches[1])) { + return null; + } + + // 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)); } } diff --git a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php index d8a0390d1..53642f3a2 100644 --- a/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php +++ b/module/Core/test/ErrorHandler/NotFoundRedirectHandlerTest.php @@ -10,6 +10,7 @@ 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 @@ public function nextIsCalledWhenNoRedirectIsResolved(callable $setUp): void 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 @@ public static function provideNonRedirectScenarios(): iterable 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); diff --git a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php index 7386169ff..2251d3011 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php @@ -10,6 +10,7 @@ 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 @@ public function expectedPayloadIsPublishedDependingOnConfig( 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)); }, ]; } diff --git a/module/Core/test/Matomo/MatomoTrackerBuilderTest.php b/module/Core/test/Matomo/MatomoTrackerBuilderTest.php index 5a4e6ab0f..1b55405ee 100644 --- a/module/Core/test/Matomo/MatomoTrackerBuilderTest.php +++ b/module/Core/test/Matomo/MatomoTrackerBuilderTest.php @@ -37,8 +37,8 @@ public function trackerIsCreated(): void { $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()); } diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php index 06b47f8c3..92fac8eb2 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php @@ -12,6 +12,7 @@ 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 @@ public function dataIsReturnedAsIsWhenTitleCannotBeResolvedFromResponse(): void } #[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 @@ private function respWithoutTitle(): Response 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 "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 diff --git a/phpstan.neon b/phpstan.neon index eee4853b3..9ebaec6e0 100644 --- 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