Add API test to verify titles for timing-out long URLs

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

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

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",

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

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,
     ],

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('<title>The title</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',
         ],
     ],

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