Merge pull request #719 from acelaya-forks/feature/handle-HEAD-requests

Feature/handle head requests

CHANGELOG.md

@@ -25,6 +25,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 #### Fixed
 
 * [#712](https://github.com/shlinkio/shlink/issues/712) Fixed app set-up not clearing entities metadata cache.
+* [#711](https://github.com/shlinkio/shlink/issues/711) Fixed `HEAD` requests returning a duplicated `Content-Length` header.
+* [#716](https://github.com/shlinkio/shlink/issues/716) Fixed Twitter not properly displaying preview for final long URL.
 
 
 ## 2.1.2 - 2020-03-29

config/autoload/middleware-pipeline.global.php

@@ -5,8 +5,9 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink;
 
 use Laminas\Stratigility\Middleware\ErrorHandler;
-use Mezzio;
+use Mezzio\Helper;
 use Mezzio\ProblemDetails;
+use Mezzio\Router;
 use PhpMiddleware\RequestId\RequestIdMiddleware;
 
 return [
@@ -14,7 +15,7 @@ return [
     'middleware_pipeline' => [
         'error-handler' => [
             'middleware' => [
-                Mezzio\Helper\ContentLengthMiddleware::class,
+                Helper\ContentLengthMiddleware::class,
                 ErrorHandler::class,
             ],
         ],
@@ -35,14 +36,15 @@ return [
 
         'routing' => [
             'middleware' => [
-                Mezzio\Router\Middleware\RouteMiddleware::class,
+                Router\Middleware\RouteMiddleware::class,
+                Router\Middleware\ImplicitHeadMiddleware::class,
             ],
         ],
 
         'rest' => [
             'path' => '/rest',
             'middleware' => [
-                Mezzio\Router\Middleware\ImplicitOptionsMiddleware::class,
+                Router\Middleware\ImplicitOptionsMiddleware::class,
                 Rest\Middleware\BodyParserMiddleware::class,
                 Rest\Middleware\AuthenticationMiddleware::class,
             ],
@@ -50,7 +52,7 @@ return [
 
         'dispatch' => [
             'middleware' => [
-                Mezzio\Router\Middleware\DispatchMiddleware::class,
+                Router\Middleware\DispatchMiddleware::class,
             ],
         ],
 
@@ -67,4 +69,5 @@ return [
             ],
         ],
     ],
+
 ];

module/Core/src/Action/AbstractTrackingAction.php

@@ -4,7 +4,9 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Core\Action;
 
+use Fig\Http\Message\RequestMethodInterface;
 use Laminas\Diactoros\Uri;
+use Mezzio\Router\Middleware\ImplicitHeadMiddleware;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\MiddlewareInterface;
@@ -24,7 +26,7 @@ use function array_merge;
 use function GuzzleHttp\Psr7\build_query;
 use function GuzzleHttp\Psr7\parse_query;
 
-abstract class AbstractTrackingAction implements MiddlewareInterface
+abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMethodInterface
 {
     private ShortUrlResolverInterface $urlResolver;
     private VisitsTrackerInterface $visitTracker;
@@ -50,14 +52,13 @@ abstract class AbstractTrackingAction implements MiddlewareInterface
         $disableTrackParam = $this->appOptions->getDisableTrackParam();
 
         try {
-            $url = $this->urlResolver->resolveEnabledShortUrl($identifier);
+            $shortUrl = $this->urlResolver->resolveEnabledShortUrl($identifier);
 
-            // Track visit to this short code
+            if ($this->shouldTrackRequest($request, $query, $disableTrackParam)) {
-            if ($disableTrackParam === null || ! array_key_exists($disableTrackParam, $query)) {
+                $this->visitTracker->track($shortUrl, Visitor::fromRequest($request));
-                $this->visitTracker->track($url, Visitor::fromRequest($request));
             }
 
-            return $this->createSuccessResp($this->buildUrlToRedirectTo($url, $query, $disableTrackParam));
+            return $this->createSuccessResp($this->buildUrlToRedirectTo($shortUrl, $query, $disableTrackParam));
         } catch (ShortUrlNotFoundException $e) {
             $this->logger->warning('An error occurred while tracking short code. {e}', ['e' => $e]);
             return $this->createErrorResp($request, $handler);
@@ -76,6 +77,16 @@ abstract class AbstractTrackingAction implements MiddlewareInterface
         return (string) $uri->withQuery(build_query($mergedQuery));
     }
 
+    private function shouldTrackRequest(ServerRequestInterface $request, array $query, ?string $disableTrackParam): bool
+    {
+        $forwardedMethod = $request->getAttribute(ImplicitHeadMiddleware::FORWARDED_HTTP_METHOD_ATTRIBUTE);
+        if ($forwardedMethod === self::METHOD_HEAD) {
+            return false;
+        }
+
+        return $disableTrackParam === null || ! array_key_exists($disableTrackParam, $query);
+    }
+
     abstract protected function createSuccessResp(string $longUrl): ResponseInterface;
 
     abstract protected function createErrorResp(

module/Core/test/Action/RedirectActionTest.php

@@ -4,8 +4,10 @@ declare(strict_types=1);
 
 namespace ShlinkioTest\Shlink\Core\Action;
 
+use Fig\Http\Message\RequestMethodInterface;
 use Laminas\Diactoros\Response;
 use Laminas\Diactoros\ServerRequest;
+use Mezzio\Router\Middleware\ImplicitHeadMiddleware;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
@@ -89,4 +91,23 @@ class RedirectActionTest extends TestCase
 
         $handle->shouldHaveBeenCalledOnce();
     }
+
+    /** @test */
+    public function trackingIsDisabledWhenRequestIsForwardedFromHead(): void
+    {
+        $shortCode = 'abc123';
+        $shortUrl = new ShortUrl('http://domain.com/foo/bar?some=thing');
+        $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, ''))->willReturn($shortUrl);
+        $track = $this->visitTracker->track(Argument::cetera())->will(function (): void {
+        });
+
+        $request = (new ServerRequest())->withAttribute('shortCode', $shortCode)
+                                        ->withAttribute(
+                                            ImplicitHeadMiddleware::FORWARDED_HTTP_METHOD_ATTRIBUTE,
+                                            RequestMethodInterface::METHOD_HEAD,
+                                        );
+        $this->action->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal());
+
+        $track->shouldNotHaveBeenCalled();
+    }
 }