From 9ad565f8c8591cbe47fc2f22bdbd077a88785aa1 Mon Sep 17 00:00:00 2001 From: Mark Orlando Zeller Date: Fri, 10 Jan 2025 22:10:51 +0100 Subject: [PATCH 1/6] Add ADDRESS environment vairable to define the listening interface. --- config/roadrunner/.rr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/roadrunner/.rr.yml b/config/roadrunner/.rr.yml index 0c535b77..901291b2 100644 --- a/config/roadrunner/.rr.yml +++ b/config/roadrunner/.rr.yml @@ -7,7 +7,7 @@ server: command: 'php -dopcache.enable_cli=1 -dopcache.validate_timestamps=0 ../../bin/roadrunner-worker.php' http: - address: '0.0.0.0:${PORT:-8080}' + address: '${ADDRESS:-0.0.0.0}:${PORT:-8080}' middleware: ['static'] static: dir: '../../public' From 62fde5a8e298864115aab1253cd85eecb44dd985 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 13 Jan 2025 08:47:19 +0100 Subject: [PATCH 2/6] Update changelog --- CHANGELOG.md | 17 +++++++++++++++++ docker-compose.yml | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 309932c1..557f2704 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,23 @@ 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). +# [Unreleased] +### Added +* [#2331](https://github.com/shlinkio/shlink/issues/2331) Add `ADDRESS` env var which allows to customize the IP address to which RoadRunner binds, when using the official docker image. + +### Changed +* *Nothing* + +### Deprecated +* *Nothing* + +### Removed +* *Nothing* + +### Fixed +* *Nothing* + + # [4.4.0] - 2024-12-27 ### Added * [#2265](https://github.com/shlinkio/shlink/issues/2265) Add a new `REDIRECT_EXTRA_PATH_MODE` option that accepts three values: diff --git a/docker-compose.yml b/docker-compose.yml index e8e69c9d..cc6966f0 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -144,7 +144,7 @@ services: shlink_mercure: container_name: shlink_mercure - image: dunglas/mercure:v0.15 + image: dunglas/mercure:v0.18 ports: - "3080:80" environment: From 1549509eb83343d3eb48510b336ff39f837ac05b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 25 Jan 2025 16:13:40 +0100 Subject: [PATCH 3/6] Update shlink packages --- composer.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/composer.json b/composer.json index f9133a5c..d18802a2 100644 --- a/composer.json +++ b/composer.json @@ -43,12 +43,12 @@ "pagerfanta/core": "^3.8", "ramsey/uuid": "^4.7", "shlinkio/doctrine-specification": "^2.2", - "shlinkio/shlink-common": "^6.6", - "shlinkio/shlink-config": "^3.4", - "shlinkio/shlink-event-dispatcher": "^4.1", - "shlinkio/shlink-importer": "^5.5", - "shlinkio/shlink-installer": "^9.4", - "shlinkio/shlink-ip-geolocation": "^4.2", + "shlinkio/shlink-common": "^7.0", + "shlinkio/shlink-config": "^4.0", + "shlinkio/shlink-event-dispatcher": "^4.2", + "shlinkio/shlink-importer": "^5.6", + "shlinkio/shlink-installer": "^9.5", + "shlinkio/shlink-ip-geolocation": "^4.3", "shlinkio/shlink-json": "^1.2", "spiral/roadrunner": "^2024.3", "spiral/roadrunner-cli": "^2.6", From 3372a2a9c886a23de2e873e0990f554b48e83afe Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 27 Jan 2025 15:40:15 +0100 Subject: [PATCH 4/6] Close connections after every async job that uses the db --- CHANGELOG.md | 2 +- module/Core/config/event_dispatcher.config.php | 6 ++++++ .../src/EventDispatcher/CloseDbConnectionEventListener.php | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 557f2704..d09a1851 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * *Nothing* ### Fixed -* *Nothing* +* [#2341](https://github.com/shlinkio/shlink/issues/2341) Ensure all asynchronous jobs that interact with the database do not leave idle connections open. # [4.4.0] - 2024-12-27 diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index 39efa3cb..d146a516 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -73,6 +73,9 @@ return (static function (): array { ], 'delegators' => [ + EventDispatcher\Matomo\SendVisitToMatomo::class => [ + EventDispatcher\CloseDbConnectionEventListenerDelegator::class, + ], EventDispatcher\Mercure\NotifyVisitToMercure::class => [ EventDispatcher\CloseDbConnectionEventListenerDelegator::class, ], @@ -94,6 +97,9 @@ return (static function (): array { EventDispatcher\LocateUnlocatedVisits::class => [ EventDispatcher\CloseDbConnectionEventListenerDelegator::class, ], + EventDispatcher\UpdateGeoLiteDb::class => [ + EventDispatcher\CloseDbConnectionEventListenerDelegator::class, + ], ], ], diff --git a/module/Core/src/EventDispatcher/CloseDbConnectionEventListener.php b/module/Core/src/EventDispatcher/CloseDbConnectionEventListener.php index 985b13c2..23f9df1b 100644 --- a/module/Core/src/EventDispatcher/CloseDbConnectionEventListener.php +++ b/module/Core/src/EventDispatcher/CloseDbConnectionEventListener.php @@ -11,7 +11,7 @@ class CloseDbConnectionEventListener /** @var callable */ private $wrapped; - public function __construct(private ReopeningEntityManagerInterface $em, callable $wrapped) + public function __construct(private readonly ReopeningEntityManagerInterface $em, callable $wrapped) { $this->wrapped = $wrapped; } From e9fe1ac5d4dc7af84807ef1245c00b9c436d9444 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 28 Jan 2025 10:04:30 +0100 Subject: [PATCH 5/6] Fix error when creating short URL for page with unsupported encoding --- CHANGELOG.md | 1 + module/Core/config/dependencies.config.php | 1 + .../Helper/ShortUrlTitleResolutionHelper.php | 61 +++++++++++++++--- .../ShortUrlTitleResolutionHelperTest.php | 62 +++++++++++++++++-- 4 files changed, 112 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d09a1851..ed878e3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Fixed * [#2341](https://github.com/shlinkio/shlink/issues/2341) Ensure all asynchronous jobs that interact with the database do not leave idle connections open. +* [#2334](https://github.com/shlinkio/shlink/issues/2334) Improve how page titles are encoded to UTF-8, falling back from mbstring to iconv if available, and ultimately using the original title in case of error, but never causing the short URL creation to fail. # [4.4.0] - 2024-12-27 diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index eda556e9..1cd1e961 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -227,6 +227,7 @@ return [ ShortUrl\Helper\ShortUrlTitleResolutionHelper::class => [ 'httpClient', Config\Options\UrlShortenerOptions::class, + 'Logger_Shlink', ], ShortUrl\Helper\ShortUrlRedirectionBuilder::class => [ Config\Options\TrackingOptions::class, diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php index 5af78345..366e18e2 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php @@ -4,14 +4,19 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Helper; +use Closure; use Fig\Http\Message\RequestMethodInterface; use GuzzleHttp\ClientInterface; use GuzzleHttp\RequestOptions; +use Laminas\Stdlib\ErrorHandler; use Psr\Http\Message\ResponseInterface; +use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Throwable; +use function function_exists; use function html_entity_decode; +use function iconv; use function mb_convert_encoding; use function preg_match; use function str_contains; @@ -30,9 +35,14 @@ readonly class ShortUrlTitleResolutionHelper implements ShortUrlTitleResolutionH // Matches the charset inside a Content-Type header private const string CHARSET_VALUE = '/charset=([^;]+)/i'; + /** + * @param (Closure(): bool)|null $isIconvInstalled + */ public function __construct( private ClientInterface $httpClient, private UrlShortenerOptions $options, + private LoggerInterface $logger, + private Closure|null $isIconvInstalled = null, ) { } @@ -58,7 +68,7 @@ readonly class ShortUrlTitleResolutionHelper implements ShortUrlTitleResolutionH } $title = $this->tryToResolveTitle($response, $contentType); - return $title !== null ? $data->withResolvedTitle($title) : $data; + return $title !== null ? $data->withResolvedTitle(html_entity_decode(trim($title))) : $data; } private function fetchUrl(string $url): ResponseInterface|null @@ -84,6 +94,7 @@ readonly class ShortUrlTitleResolutionHelper implements ShortUrlTitleResolutionH { $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); @@ -95,12 +106,48 @@ readonly class ShortUrlTitleResolutionHelper implements ShortUrlTitleResolutionH return null; } - // Get the page's charset from Content-Type header - preg_match(self::CHARSET_VALUE, $contentType, $charsetMatches); + $titleInOriginalEncoding = $titleMatches[1]; - $title = isset($charsetMatches[1]) - ? mb_convert_encoding($titleMatches[1], 'utf8', $charsetMatches[1]) - : $titleMatches[1]; - return html_entity_decode(trim($title)); + // Get the page's charset from Content-Type header, or return title as is if not found + preg_match(self::CHARSET_VALUE, $contentType, $charsetMatches); + if (! isset($charsetMatches[1])) { + return $titleInOriginalEncoding; + } + + $pageCharset = $charsetMatches[1]; + return $this->encodeToUtf8WithMbString($titleInOriginalEncoding, $pageCharset) + ?? $this->encodeToUtf8WithIconv($titleInOriginalEncoding, $pageCharset) + ?? $titleInOriginalEncoding; + } + + private function encodeToUtf8WithMbString(string $titleInOriginalEncoding, string $pageCharset): string|null + { + try { + return mb_convert_encoding($titleInOriginalEncoding, 'utf-8', $pageCharset); + } catch (Throwable $e) { + $this->logger->warning('It was impossible to encode page title in UTF-8 with mb_convert_encoding. {e}', [ + 'e' => $e, + ]); + return null; + } + } + + private function encodeToUtf8WithIconv(string $titleInOriginalEncoding, string $pageCharset): string|null + { + $isIconvInstalled = ($this->isIconvInstalled ?? fn () => function_exists('iconv'))(); + if (! $isIconvInstalled) { + $this->logger->warning('Missing iconv extension. Skipping title encoding'); + return null; + } + + try { + ErrorHandler::start(); + $title = iconv($pageCharset, 'utf-8', $titleInOriginalEncoding); + ErrorHandler::stop(throw: true); + return $title ?: null; + } catch (Throwable $e) { + $this->logger->warning('It was impossible to encode page title in UTF-8 with iconv. {e}', ['e' => $e]); + return null; + } } } diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php index b11a28a3..787ca294 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlTitleResolutionHelperTest.php @@ -16,6 +16,7 @@ use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\MockObject\Builder\InvocationMocker; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlTitleResolutionHelper; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; @@ -25,10 +26,12 @@ class ShortUrlTitleResolutionHelperTest extends TestCase private const string LONG_URL = 'http://foobar.com/12345/hello?foo=bar'; private MockObject & ClientInterface $httpClient; + private MockObject & LoggerInterface $logger; protected function setUp(): void { $this->httpClient = $this->createMock(ClientInterface::class); + $this->logger = $this->createMock(LoggerInterface::class); } #[Test] @@ -90,14 +93,59 @@ class ShortUrlTitleResolutionHelperTest extends TestCase } #[Test] - #[TestWith(['TEXT/html; charset=utf-8'], 'charset')] - #[TestWith(['TEXT/html'], 'no charset')] - public function titleIsUpdatedWhenItCanBeResolvedFromResponse(string $contentType): void + #[TestWith(['TEXT/html', false], 'no charset')] + #[TestWith(['TEXT/html; charset=utf-8', false], 'mbstring-supported charset')] + #[TestWith(['TEXT/html; charset=Windows-1255', true], 'mbstring-unsupported charset')] + public function titleIsUpdatedWhenItCanBeResolvedFromResponse(string $contentType, bool $expectsWarning): void { - $data = ShortUrlCreation::fromRawData(['longUrl' => self::LONG_URL]); $this->expectRequestToBeCalled()->willReturn($this->respWithTitle($contentType)); + if ($expectsWarning) { + $this->logger->expects($this->once())->method('warning')->with( + 'It was impossible to encode page title in UTF-8 with mb_convert_encoding. {e}', + $this->isArray(), + ); + } else { + $this->logger->expects($this->never())->method('warning'); + } - $result = $this->helper(autoResolveTitles: true)->processTitle($data); + $data = ShortUrlCreation::fromRawData(['longUrl' => self::LONG_URL]); + $result = $this->helper(autoResolveTitles: true, iconvEnabled: true)->processTitle($data); + + self::assertNotSame($data, $result); + self::assertEquals('Resolved "title"', $result->title); + } + + #[Test] + #[TestWith([ + 'contentType' => 'text/html; charset=Windows-1255', + 'iconvEnabled' => false, + 'expectedSecondMessage' => 'Missing iconv extension. Skipping title encoding', + ])] + #[TestWith([ + 'contentType' => 'text/html; charset=foo', + 'iconvEnabled' => true, + 'expectedSecondMessage' => 'It was impossible to encode page title in UTF-8 with iconv. {e}', + ])] + public function warningsLoggedWhenTitleCannotBeEncodedToUtf8( + string $contentType, + bool $iconvEnabled, + string $expectedSecondMessage, + ): void { + $this->expectRequestToBeCalled()->willReturn($this->respWithTitle($contentType)); + $callCount = 0; + $this->logger->expects($this->exactly(2))->method('warning')->with($this->callback( + function (string $message) use (&$callCount, $expectedSecondMessage): bool { + $callCount++; + if ($callCount === 1) { + return $message === 'It was impossible to encode page title in UTF-8 with mb_convert_encoding. {e}'; + } + + return $message === $expectedSecondMessage; + }, + )); + + $data = ShortUrlCreation::fromRawData(['longUrl' => self::LONG_URL]); + $result = $this->helper(autoResolveTitles: true, iconvEnabled: $iconvEnabled)->processTitle($data); self::assertNotSame($data, $result); self::assertEquals('Resolved "title"', $result->title); @@ -143,11 +191,13 @@ class ShortUrlTitleResolutionHelperTest extends TestCase return $body; } - private function helper(bool $autoResolveTitles = false): ShortUrlTitleResolutionHelper + private function helper(bool $autoResolveTitles = false, bool $iconvEnabled = false): ShortUrlTitleResolutionHelper { return new ShortUrlTitleResolutionHelper( $this->httpClient, new UrlShortenerOptions(autoResolveTitles: $autoResolveTitles), + $this->logger, + fn () => $iconvEnabled, ); } } From af783dea5728d40482fd4971156e5b806840c518 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 28 Jan 2025 10:12:15 +0100 Subject: [PATCH 6/6] Add v4.4.1 to changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed878e3a..881eeb3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ 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). -# [Unreleased] +# [4.4.1] - 2025-01-28 ### Added * [#2331](https://github.com/shlinkio/shlink/issues/2331) Add `ADDRESS` env var which allows to customize the IP address to which RoadRunner binds, when using the official docker image.