diff --git a/CHANGELOG.md b/CHANGELOG.md index de9d6b1b..3bb731dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#1908](https://github.com/shlinkio/shlink/issues/1908) Remove support for openswoole (and swoole). ### Fixed -* *Nothing* +* [#2000](https://github.com/shlinkio/shlink/issues/2000) Fix short URL creation/edition getting stuck when trying to resolve the title of a long URL which never returns a response. ## [3.7.3] - 2024-01-04 diff --git a/UPGRADE.md b/UPGRADE.md index ab6085ad..aa235473 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -13,6 +13,7 @@ * The short URLs `loosely` mode is no longer supported, as it was a typo. Use `loose` mode instead. * QR codes URLs now work by default, even for short URLs that cannot be visited due to max visits or date range limitations. If you want to keep previous behavior, pass `QR_CODE_FOR_DISABLED_SHORT_URLS=false` or the equivalent configuration option. +* Long URL title resolution is now enabled by default. You can still disable it by passing `AUTO_RESOLVE_TITLES=false` or the equivalent configuration option. * Shlink no longer allows to opt-in for long URL verification. Long URLs are unconditionally considered correct during short URL creation/edition. ### Changes in REST API diff --git a/composer.json b/composer.json index 8a883325..3ce4f63a 100644 --- a/composer.json +++ b/composer.json @@ -46,7 +46,7 @@ "shlinkio/shlink-config": "dev-main#a43b380 as 3.0", "shlinkio/shlink-event-dispatcher": "dev-main#aa9023c as 4.0", "shlinkio/shlink-importer": "dev-main#65a9a30 as 5.3", - "shlinkio/shlink-installer": "dev-develop#3e8d7d7 as 9.0", + "shlinkio/shlink-installer": "dev-develop#b314455 as 9.0", "shlinkio/shlink-ip-geolocation": "dev-main#a807668 as 3.5", "shlinkio/shlink-json": "^1.1", "spiral/roadrunner": "^2023.3", diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 2816577d..43bd5a74 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -24,7 +24,7 @@ return (static function (): array { 'hostname' => EnvVars::DEFAULT_DOMAIN->loadFromEnv(''), ], 'default_short_codes_length' => $shortCodesLength, - 'auto_resolve_titles' => (bool) EnvVars::AUTO_RESOLVE_TITLES->loadFromEnv(false), + 'auto_resolve_titles' => (bool) EnvVars::AUTO_RESOLVE_TITLES->loadFromEnv(true), 'append_extra_path' => (bool) EnvVars::REDIRECT_APPEND_EXTRA_PATH->loadFromEnv(false), 'multi_segment_slugs_enabled' => (bool) EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->loadFromEnv(false), 'trailing_slash_enabled' => (bool) EnvVars::SHORT_URL_TRAILING_SLASH->loadFromEnv(false), diff --git a/config/autoload/url-shortener.local.php.dist b/config/autoload/url-shortener.local.php.dist index 0b2e9db1..715d2822 100644 --- a/config/autoload/url-shortener.local.php.dist +++ b/config/autoload/url-shortener.local.php.dist @@ -14,7 +14,6 @@ return [ default => '8000', }), ], - 'auto_resolve_titles' => true, // 'multi_segment_slugs_enabled' => true, // 'trailing_slash_enabled' => true, ], diff --git a/config/test/test_config.global.php b/config/test/test_config.global.php index 94d723b6..d86700b9 100644 --- a/config/test/test_config.global.php +++ b/config/test/test_config.global.php @@ -7,7 +7,9 @@ namespace Shlinkio\Shlink; use GuzzleHttp\Client; use Laminas\ConfigAggregator\ConfigAggregator; use Laminas\Diactoros\Response\EmptyResponse; +use Laminas\Diactoros\Response\HtmlResponse; use Laminas\ServiceManager\Factory\InvokableFactory; +use Mezzio\Router\FastRouteRouter; use Monolog\Level; use Shlinkio\Shlink\Common\Logger\LoggerType; use Shlinkio\Shlink\TestUtils\ApiTest\CoverageMiddleware; @@ -18,6 +20,7 @@ use Symfony\Component\Console\Application; use function Laminas\Stratigility\middleware; use function Shlinkio\Shlink\Config\env; use function Shlinkio\Shlink\Core\ArrayUtils\contains; +use function sleep; use function sprintf; use const ShlinkioTest\Shlink\API_TESTS_HOST; @@ -86,6 +89,7 @@ return [ 'debug' => true, ConfigAggregator::ENABLE_CACHE => false, + FastRouteRouter::CONFIG_CACHE_ENABLED => false, 'url_shortener' => [ 'domain' => [ @@ -94,10 +98,12 @@ return [ ], ], - 'routes' => !$isApiTest ? [] : [ + 'routes' => [ + // This route is invoked at the end of API tests, in order to dump coverage collected so far [ 'name' => 'dump_coverage', 'path' => '/api-tests/stop-coverage', + 'allowed_methods' => ['GET'], 'middleware' => middleware(static function () use ($coverage, $coverageType) { // TODO I have tried moving this block to a register_shutdown_function here, which internally checks if // RR_MODE === 'http', but this seems to be false in CI, causing the coverage to not be generated @@ -108,7 +114,17 @@ return [ ); return new EmptyResponse(); }), + ], + + // This route is used to test that title resolution is skipped if the long URL times out + [ + 'name' => 'long_url_with_timeout', + 'path' => '/api-tests/long-url-with-timeout', 'allowed_methods' => ['GET'], + 'middleware' => middleware(static function () { + sleep(5); // Title resolution times out at 3 seconds + return new HtmlResponse('The title'); + }), ], ], @@ -119,6 +135,7 @@ return [ ], ], + // Disable mercure integration during E2E tests 'mercure' => [ 'public_hub_url' => null, 'internal_hub_url' => null, @@ -153,7 +170,7 @@ return [ 'data_fixtures' => [ 'paths' => [ - // TODO These are used for CLI tests too, so maybe should be somewhere else + // TODO These are used for other module's tests, so maybe should be somewhere else __DIR__ . '/../../module/Rest/test-api/Fixtures', ], ], diff --git a/module/Rest/test-api/Action/CreateShortUrlTest.php b/module/Rest/test-api/Action/CreateShortUrlTest.php index 41a4559b..f612f628 100644 --- a/module/Rest/test-api/Action/CreateShortUrlTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlTest.php @@ -328,6 +328,17 @@ class CreateShortUrlTest extends ApiTestCase self::assertEquals('https://github.com/shlinkio/shlink/android', $payload['deviceLongUrls']['android'] ?? null); } + #[Test] + public function titleIsIgnoredIfLongUrlTimesOut(): void + { + [$statusCode, $payload] = $this->createShortUrl([ + 'longUrl' => 'http://127.0.0.1:9999/api-tests/long-url-with-timeout', + ]); + + self::assertEquals(self::STATUS_OK, $statusCode); + self::assertNull($payload['title']); + } + /** * @return array{int, array} */