Merge pull request #1035 from shlinkio/develop

Release 2.6.1

CHANGELOG.md

@@ -4,6 +4,25 @@ 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).
 
+## [2.6.1] - 2021-02-22
+### Added
+* *Nothing*
+
+### Changed
+* [#1026](https://github.com/shlinkio/shlink/issues/1026) Removed non-inclusive terms from source code.
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* [#1024](https://github.com/shlinkio/shlink/issues/1024) Fixed migration that is incorrectly skipped due to the wrong condition being used to check it.
+* [#1031](https://github.com/shlinkio/shlink/issues/1031) Fixed shortening of twitter URLs with URL validation enabled.
+* [#1034](https://github.com/shlinkio/shlink/issues/1034) Fixed warning displayed when shlink is stopped while running it with swoole.
+
+
 ## [2.6.0] - 2021-02-13
 ### Added
 * [#856](https://github.com/shlinkio/shlink/issues/856) Added PHP 8.0 support.
@@ -22,8 +41,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 
     This new orphan visits can be consumed in these ways:
 
-      * The `https://shlink.io/new-orphan-visit` mercure topic, which gets notified when an orphan visit occurs.
-      * The `GET /visits/orphan` REST endpoint, which behaves like the short URL visits and tags visits endpoints, but returns only orphan visits.
+    * The `https://shlink.io/new-orphan-visit` mercure topic, which gets notified when an orphan visit occurs.
+    * The `GET /visits/orphan` REST endpoint, which behaves like the short URL visits and tags visits endpoints, but returns only orphan visits.
 
 ### Changed
 * [#977](https://github.com/shlinkio/shlink/issues/977) Migrated from `laminas/laminas-paginator` to `pagerfanta/core` to handle pagination.

README.md

@@ -33,7 +33,8 @@ The idea is that you can just generate a container using the image and provide t
 
 First, make sure the host where you are going to run shlink fulfills these requirements:
 
-* PHP 7.4 with JSON, curl, PDO, intl and gd extensions enabled (PHP 8.0 support is coming).
+* PHP 7.4 or 8.0
+* The next PHP extensions: json, curl, pdo, intl, gd and gmp.
     * apcu extension is recommended if you don't plan to use swoole.
     * xml extension is required if you want to generate QR codes in svg format.
 * MySQL, MariaDB, PostgreSQL, Microsoft SQL Server or SQLite.

composer.json

@@ -20,7 +20,7 @@
         "cocur/slugify": "^4.0",
         "doctrine/cache": "^1.9",
         "doctrine/migrations": "^3.0.2",
-        "doctrine/orm": "^2.8",
+        "doctrine/orm": "2.8.1 || ^2.8.3",
         "endroid/qr-code": "dev-master#0f1613a as 3.10",
         "geoip2/geoip2": "^2.9",
         "guzzlehttp/guzzle": "^7.0",
@@ -37,7 +37,7 @@
         "mezzio/mezzio": "^3.3",
         "mezzio/mezzio-fastroute": "^3.1",
         "mezzio/mezzio-problem-details": "^1.3",
-        "mezzio/mezzio-swoole": "^3.1",
+        "mezzio/mezzio-swoole": "^3.3",
         "monolog/monolog": "^2.0",
         "nikolaposa/monolog-factory": "^3.1",
         "ocramius/proxy-manager": "^2.11",

data/migrations/Version20210207100807.php

@@ -15,10 +15,9 @@ final class Version20210207100807 extends AbstractMigration
     public function up(Schema $schema): void
     {
         $visits = $schema->getTable('visits');
-        $shortUrlId = $visits->getColumn('short_url_id');
-
-        $this->skipIf(! $shortUrlId->getNotnull());
+        $this->skipIf($visits->hasColumn('visited_url'));
 
+        $shortUrlId = $visits->getColumn('short_url_id');
         $shortUrlId->setNotnull(false);
 
         $visits->addColumn('visited_url', Types::STRING, [
@@ -34,10 +33,9 @@ final class Version20210207100807 extends AbstractMigration
     public function down(Schema $schema): void
     {
         $visits = $schema->getTable('visits');
-        $shortUrlId = $visits->getColumn('short_url_id');
-
-        $this->skipIf($shortUrlId->getNotnull());
+        $this->skipIf(! $visits->hasColumn('visited_url'));
 
+        $shortUrlId = $visits->getColumn('short_url_id');
         $shortUrlId->setNotnull(true);
         $visits->dropColumn('visited_url');
         $visits->dropColumn('type');

module/Core/src/Util/UrlValidator.php

@@ -20,6 +20,8 @@ use const Shlinkio\Shlink\Core\TITLE_TAG_VALUE;
 class UrlValidator implements UrlValidatorInterface, RequestMethodInterface
 {
     private const MAX_REDIRECTS = 15;
+    private const CHROME_USER_AGENT = 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) '
+        . 'Chrome/51.0.2704.103 Safari/537.36';
 
     private ClientInterface $httpClient;
     private UrlShortenerOptions $options;
@@ -67,6 +69,8 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface
             return $this->httpClient->request(self::METHOD_GET, $url, [
                 RequestOptions::ALLOW_REDIRECTS => ['max' => self::MAX_REDIRECTS],
                 RequestOptions::IDN_CONVERSION => true,
+                // Making the request with a browser's user agent makes the validation closer to a real user
+                RequestOptions::HEADERS => ['User-Agent' => self::CHROME_USER_AGENT],
             ]);
         } catch (GuzzleException $e) {
             if ($throwOnError) {

module/Core/test/Util/UrlValidatorTest.php

@@ -10,6 +10,7 @@ use GuzzleHttp\Exception\ClientException;
 use GuzzleHttp\RequestOptions;
 use Laminas\Diactoros\Response;
 use Laminas\Diactoros\Stream;
+use PHPUnit\Framework\Assert;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\PhpUnit\ProphecyTrait;
@@ -52,10 +53,16 @@ class UrlValidatorTest extends TestCase
         $request = $this->httpClient->request(
             RequestMethodInterface::METHOD_GET,
             $expectedUrl,
-            [
-                RequestOptions::ALLOW_REDIRECTS => ['max' => 15],
-                RequestOptions::IDN_CONVERSION => true,
-            ],
+            Argument::that(function (array $options) {
+                Assert::assertArrayHasKey(RequestOptions::ALLOW_REDIRECTS, $options);
+                Assert::assertEquals(['max' => 15], $options[RequestOptions::ALLOW_REDIRECTS]);
+                Assert::assertArrayHasKey(RequestOptions::IDN_CONVERSION, $options);
+                Assert::assertTrue($options[RequestOptions::IDN_CONVERSION]);
+                Assert::assertArrayHasKey(RequestOptions::HEADERS, $options);
+                Assert::assertArrayHasKey('User-Agent', $options[RequestOptions::HEADERS]);
+
+                return true;
+            }),
         )->willReturn(new Response());
 
         $this->urlValidator->validateUrl($expectedUrl, null);

module/Rest/config/auth.config.php

@@ -9,7 +9,7 @@ use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory;
 return [
 
     'auth' => [
-        'routes_whitelist' => [
+        'routes_without_api_key' => [
             Action\HealthAction::class,
             ConfigProvider::UNVERSIONED_HEALTH_ENDPOINT_NAME,
         ],
@@ -28,7 +28,7 @@ return [
     ConfigAbstractFactory::class => [
         Middleware\AuthenticationMiddleware::class => [
             Service\ApiKeyService::class,
-            'config.auth.routes_whitelist',
+            'config.auth.routes_without_api_key',
             'config.auth.routes_with_query_api_key',
         ],
     ],

module/Rest/src/Middleware/AuthenticationMiddleware.php

@@ -24,16 +24,16 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa
     public const API_KEY_HEADER = 'X-Api-Key';
 
     private ApiKeyServiceInterface $apiKeyService;
-    private array $routesWhitelist;
+    private array $routesWithoutApiKey;
     private array $routesWithQueryApiKey;
 
     public function __construct(
         ApiKeyServiceInterface $apiKeyService,
-        array $routesWhitelist,
+        array $routesWithoutApiKey,
         array $routesWithQueryApiKey
     ) {
         $this->apiKeyService = $apiKeyService;
-        $this->routesWhitelist = $routesWhitelist;
+        $this->routesWithoutApiKey = $routesWithoutApiKey;
         $this->routesWithQueryApiKey = $routesWithQueryApiKey;
     }
 
@@ -45,7 +45,7 @@ class AuthenticationMiddleware implements MiddlewareInterface, StatusCodeInterfa
             $routeResult === null
             || $routeResult->isFailure()
             || $request->getMethod() === self::METHOD_OPTIONS
-            || contains($this->routesWhitelist, $routeResult->getMatchedRouteName())
+            || contains($this->routesWithoutApiKey, $routeResult->getMatchedRouteName())
         ) {
             return $handler->handle($request);
         }

module/Rest/test-api/Action/CreateShortUrlTest.php

@@ -297,6 +297,24 @@ class CreateShortUrlTest extends ApiTestCase
         yield 'example domain' => ['example.com'];
     }
 
+    /**
+     * @test
+     * @dataProvider provideTwitterUrls
+     */
+    public function urlsWithBothProtectionCanBeShortenedWithUrlValidationEnabled(string $longUrl): void
+    {
+        [$statusCode] = $this->createShortUrl(['longUrl' => $longUrl, 'validateUrl' => true]);
+        self::assertEquals(self::STATUS_OK, $statusCode);
+    }
+
+    public function provideTwitterUrls(): iterable
+    {
+        yield ['https://twitter.com/shlinkio'];
+        yield ['https://mobile.twitter.com/shlinkio'];
+        yield ['https://twitter.com/shlinkio/status/1360637738421268481'];
+        yield ['https://mobile.twitter.com/shlinkio/status/1360637738421268481'];
+    }
+
     /**
      * @return array {
      *     @var int $statusCode

module/Rest/test/Middleware/AuthenticationMiddlewareTest.php

@@ -48,9 +48,9 @@ class AuthenticationMiddlewareTest extends TestCase
 
     /**
      * @test
-     * @dataProvider provideWhitelistedRequests
+     * @dataProvider provideRequestsWithoutAuth
      */
-    public function someWhiteListedSituationsFallbackToNextMiddleware(ServerRequestInterface $request): void
+    public function someSituationsFallbackToNextMiddleware(ServerRequestInterface $request): void
     {
         $handle = $this->handler->handle($request)->willReturn(new Response());
         $checkApiKey = $this->apiKeyService->check(Argument::any());
@@ -61,22 +61,22 @@ class AuthenticationMiddlewareTest extends TestCase
         $checkApiKey->shouldNotHaveBeenCalled();
     }
 
-    public function provideWhitelistedRequests(): iterable
+    public function provideRequestsWithoutAuth(): iterable
     {
         $dummyMiddleware = $this->getDummyMiddleware();
 
-        yield 'with no route result' => [new ServerRequest()];
-        yield 'with failure route result' => [(new ServerRequest())->withAttribute(
+        yield 'no route result' => [new ServerRequest()];
+        yield 'failure route result' => [(new ServerRequest())->withAttribute(
             RouteResult::class,
             RouteResult::fromRouteFailure([RequestMethodInterface::METHOD_GET]),
         )];
-        yield 'with whitelisted route' => [(new ServerRequest())->withAttribute(
+        yield 'route without API key required' => [(new ServerRequest())->withAttribute(
             RouteResult::class,
             RouteResult::fromRoute(
                 new Route('foo', $dummyMiddleware, Route::HTTP_METHOD_ANY, HealthAction::class),
             ),
         )];
-        yield 'with OPTIONS method' => [(new ServerRequest())->withAttribute(
+        yield 'OPTIONS method' => [(new ServerRequest())->withAttribute(
             RouteResult::class,
             RouteResult::fromRoute(new Route('bar', $dummyMiddleware), []),
         )->withMethod(RequestMethodInterface::METHOD_OPTIONS)];