Merge pull request #653 from acelaya-forks/feature/remove-default-domain-from-body

Feature/remove default domain from body

CHANGELOG.md

@@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
 #### Fixed
 
 * [#648](https://github.com/shlinkio/shlink/issues/648) Ensured any user can write in log files, in case shlink is run by several system users.
+* [#650](https://github.com/shlinkio/shlink/issues/650) Ensured default domain is ignored when trying to create a short URL.
 
 
 ## 2.0.4 - 2020-02-02

module/Rest/config/dependencies.config.php

@@ -37,7 +37,7 @@ return [
             Middleware\BodyParserMiddleware::class => InvokableFactory::class,
             Middleware\CrossDomainMiddleware::class => InvokableFactory::class,
             Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class => InvokableFactory::class,
-            Middleware\ShortUrl\DropDefaultDomainFromQueryMiddleware::class => ConfigAbstractFactory::class,
+            Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class => ConfigAbstractFactory::class,
         ],
     ],
 
@@ -74,7 +74,7 @@ return [
         Action\Tag\CreateTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class],
         Action\Tag\UpdateTagAction::class => [Service\Tag\TagService::class, LoggerInterface::class],
 
-        Middleware\ShortUrl\DropDefaultDomainFromQueryMiddleware::class => ['config.url_shortener.domain.hostname'],
+        Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class => ['config.url_shortener.domain.hostname'],
     ],
 
 ];

module/Rest/config/routes.config.php

@@ -4,8 +4,8 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Rest;
 
-$contentNegotiationMiddleware = [Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class];
-$dropDomainMiddleware = [Middleware\ShortUrl\DropDefaultDomainFromQueryMiddleware::class];
+$contentNegotiationMiddleware = Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class;
+$dropDomainMiddleware = Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class;
 
 return [
 
@@ -13,16 +13,16 @@ return [
         Action\HealthAction::getRouteDef(),
 
         // Short codes
-        Action\ShortUrl\CreateShortUrlAction::getRouteDef($contentNegotiationMiddleware),
-        Action\ShortUrl\SingleStepCreateShortUrlAction::getRouteDef($contentNegotiationMiddleware),
-        Action\ShortUrl\EditShortUrlAction::getRouteDef($dropDomainMiddleware),
-        Action\ShortUrl\DeleteShortUrlAction::getRouteDef($dropDomainMiddleware),
-        Action\ShortUrl\ResolveShortUrlAction::getRouteDef($dropDomainMiddleware),
+        Action\ShortUrl\CreateShortUrlAction::getRouteDef([$contentNegotiationMiddleware, $dropDomainMiddleware]),
+        Action\ShortUrl\SingleStepCreateShortUrlAction::getRouteDef([$contentNegotiationMiddleware]),
+        Action\ShortUrl\EditShortUrlAction::getRouteDef([$dropDomainMiddleware]),
+        Action\ShortUrl\DeleteShortUrlAction::getRouteDef([$dropDomainMiddleware]),
+        Action\ShortUrl\ResolveShortUrlAction::getRouteDef([$dropDomainMiddleware]),
         Action\ShortUrl\ListShortUrlsAction::getRouteDef(),
-        Action\ShortUrl\EditShortUrlTagsAction::getRouteDef($dropDomainMiddleware),
+        Action\ShortUrl\EditShortUrlTagsAction::getRouteDef([$dropDomainMiddleware]),
 
         // Visits
-        Action\Visit\GetVisitsAction::getRouteDef($dropDomainMiddleware),
+        Action\Visit\GetVisitsAction::getRouteDef([$dropDomainMiddleware]),
 
         // Tags
         Action\Tag\ListTagsAction::getRouteDef(),

module/Rest/src/Middleware/ShortUrl/DropDefaultDomainFromRequestMiddleware.php

@@ -9,7 +9,7 @@ use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
 
-class DropDefaultDomainFromQueryMiddleware implements MiddlewareInterface
+class DropDefaultDomainFromRequestMiddleware implements MiddlewareInterface
 {
     private string $defaultDomain;
 
@@ -20,12 +20,18 @@ class DropDefaultDomainFromQueryMiddleware implements MiddlewareInterface
 
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
     {
-        $query = $request->getQueryParams();
-        if (isset($query['domain']) && $query['domain'] === $this->defaultDomain) {
-            unset($query['domain']);
-            $request = $request->withQueryParams($query);
-        }
+        $request = $request->withQueryParams($this->sanitizeDomainFromPayload($request->getQueryParams()))
+                           ->withParsedBody($this->sanitizeDomainFromPayload($request->getParsedBody()));
 
         return $handler->handle($request);
     }
+
+    private function sanitizeDomainFromPayload(array $payload): array
+    {
+        if (isset($payload['domain']) && $payload['domain'] === $this->defaultDomain) {
+            unset($payload['domain']);
+        }
+
+        return $payload;
+    }
 }

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

@@ -228,6 +228,22 @@ class CreateShortUrlActionTest extends ApiTestCase
         $this->assertEquals($url, $payload['url']);
     }
 
+    /** @test */
+    public function defaultDomainIsDroppedIfProvided(): void
+    {
+        [$createStatusCode, ['shortCode' => $shortCode]] = $this->createShortUrl([
+            'longUrl' => 'https://www.alejandrocelaya.com',
+            'domain' => 'doma.in',
+        ]);
+        $getResp = $this->callApiWithKey(self::METHOD_GET, '/short-urls/' . $shortCode);
+        $payload = $this->getJsonResponsePayload($getResp);
+
+        $this->assertEquals(self::STATUS_OK, $createStatusCode);
+        $this->assertEquals(self::STATUS_OK, $getResp->getStatusCode());
+        $this->assertArrayHasKey('domain', $payload);
+        $this->assertNull($payload['domain']);
+    }
+
     /**
      * @return array {
      *     @var int $statusCode

module/Rest/test/Middleware/ShortUrl/DropDefaultDomainFromRequestMiddlewareTest.php

@@ -12,29 +12,30 @@ use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\RequestHandlerInterface;
-use Shlinkio\Shlink\Rest\Middleware\ShortUrl\DropDefaultDomainFromQueryMiddleware;
+use Shlinkio\Shlink\Rest\Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware;
 
-class DropDefaultDomainFromQueryMiddlewareTest extends TestCase
+class DropDefaultDomainFromRequestMiddlewareTest extends TestCase
 {
-    private DropDefaultDomainFromQueryMiddleware $middleware;
+    private DropDefaultDomainFromRequestMiddleware $middleware;
     private ObjectProphecy $next;
 
     public function setUp(): void
     {
         $this->next = $this->prophesize(RequestHandlerInterface::class);
-        $this->middleware = new DropDefaultDomainFromQueryMiddleware('doma.in');
+        $this->middleware = new DropDefaultDomainFromRequestMiddleware('doma.in');
     }
 
     /**
      * @test
      * @dataProvider provideQueryParams
      */
-    public function domainIsDroppedWhenDefaultOneIsProvided(array $providedQuery, array $expectedQuery): void
+    public function domainIsDroppedWhenDefaultOneIsProvided(array $providedPayload, array $expectedPayload): void
     {
-        $req = ServerRequestFactory::fromGlobals()->withQueryParams($providedQuery);
+        $req = ServerRequestFactory::fromGlobals()->withQueryParams($providedPayload)->withParsedBody($providedPayload);
 
-        $handle = $this->next->handle(Argument::that(function (ServerRequestInterface $request) use ($expectedQuery) {
-            Assert::assertEquals($expectedQuery, $request->getQueryParams());
+        $handle = $this->next->handle(Argument::that(function (ServerRequestInterface $request) use ($expectedPayload) {
+            Assert::assertEquals($expectedPayload, $request->getQueryParams());
+            Assert::assertEquals($expectedPayload, $request->getParsedBody());
             return $request;
         }))->willReturn(new Response());