From 05a64b8d9e57f1ca54d8a0dd7b89b4f5792f35f7 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Fri, 6 Dec 2019 22:38:22 +0100
Subject: [PATCH] Ensured dates parsing does not mask actual validation errors

---
 .../src/Command/ShortUrl/GenerateShortUrlCommand.php | 11 ++---------
 module/Core/src/Model/ShortUrlMeta.php               |  7 ++++++-
 module/Core/test/Model/ShortUrlMetaTest.php          | 12 ++++++++++++
 .../src/Action/ShortUrl/CreateShortUrlAction.php     | 10 ++--------
 4 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php
index 9d9c464b..d7323a0d 100644
--- a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php
+++ b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php
@@ -4,7 +4,6 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\CLI\Command\ShortUrl;
 
-use Cake\Chronos\Chronos;
 use Shlinkio\Shlink\CLI\Util\ExitCodes;
 use Shlinkio\Shlink\Core\Exception\InvalidUrlException;
 use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException;
@@ -127,8 +126,8 @@ class GenerateShortUrlCommand extends Command
                 new Uri($longUrl),
                 $tags,
                 ShortUrlMeta::createFromParams(
-                    $this->getOptionalDate($input, 'validSince'),
-                    $this->getOptionalDate($input, 'validUntil'),
+                    $input->getOption('validSince'),
+                    $input->getOption('validUntil'),
                     $customSlug,
                     $maxVisits !== null ? (int) $maxVisits : null,
                     $input->getOption('findIfExists'),
@@ -151,10 +150,4 @@ class GenerateShortUrlCommand extends Command
             return ExitCodes::EXIT_FAILURE;
         }
     }
-
-    private function getOptionalDate(InputInterface $input, string $fieldName): ?Chronos
-    {
-        $since = $input->getOption($fieldName);
-        return $since !== null ? Chronos::parse($since) : null;
-    }
 }
diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php
index bf31a01b..24e770c6 100644
--- a/module/Core/src/Model/ShortUrlMeta.php
+++ b/module/Core/src/Model/ShortUrlMeta.php
@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Core\Model;
 
 use Cake\Chronos\Chronos;
+use DateTimeInterface;
 use Shlinkio\Shlink\Core\Exception\ValidationException;
 use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
 
@@ -96,7 +97,7 @@ final class ShortUrlMeta
     }
 
     /**
-     * @param string|Chronos|null $date
+     * @param string|DateTimeInterface|Chronos|null $date
      */
     private function parseDateField($date): ?Chronos
     {
@@ -104,6 +105,10 @@ final class ShortUrlMeta
             return $date;
         }
 
+        if ($date instanceof DateTimeInterface) {
+            return Chronos::instance($date);
+        }
+
         return Chronos::parse($date);
     }
 
diff --git a/module/Core/test/Model/ShortUrlMetaTest.php b/module/Core/test/Model/ShortUrlMetaTest.php
index d606e07f..7c8e5aae 100644
--- a/module/Core/test/Model/ShortUrlMetaTest.php
+++ b/module/Core/test/Model/ShortUrlMetaTest.php
@@ -9,6 +9,7 @@ use PHPUnit\Framework\TestCase;
 use Shlinkio\Shlink\Core\Exception\ValidationException;
 use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
 use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter;
+use stdClass;
 
 class ShortUrlMetaTest extends TestCase
 {
@@ -35,6 +36,17 @@ class ShortUrlMetaTest extends TestCase
             ShortUrlMetaInputFilter::VALID_SINCE => '2017',
             ShortUrlMetaInputFilter::MAX_VISITS => 5,
         ]];
+        yield [[
+            ShortUrlMetaInputFilter::VALID_SINCE => new stdClass(),
+            ShortUrlMetaInputFilter::VALID_UNTIL => 'foo',
+        ]];
+        yield [[
+            ShortUrlMetaInputFilter::VALID_UNTIL => 500,
+        ]];
+        yield [[
+            ShortUrlMetaInputFilter::MAX_VISITS => new stdClass(),
+            ShortUrlMetaInputFilter::DOMAIN => 4,
+        ]];
     }
 
     /** @test */
diff --git a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php
index 3b0a0b61..96145ce0 100644
--- a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php
+++ b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php
@@ -4,7 +4,6 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Rest\Action\ShortUrl;
 
-use Cake\Chronos\Chronos;
 use Psr\Http\Message\ServerRequestInterface as Request;
 use Shlinkio\Shlink\Core\Exception\InvalidArgumentException;
 use Shlinkio\Shlink\Core\Exception\ValidationException;
@@ -32,8 +31,8 @@ class CreateShortUrlAction extends AbstractCreateShortUrlAction
 
         try {
             $meta = ShortUrlMeta::createFromParams(
-                $this->getOptionalDate($postData, 'validSince'),
-                $this->getOptionalDate($postData, 'validUntil'),
+                $postData['validSince'] ?? null,
+                $postData['validUntil'] ?? null,
                 $postData['customSlug'] ?? null,
                 $postData['maxVisits'] ?? null,
                 $postData['findIfExists'] ?? null,
@@ -45,9 +44,4 @@ class CreateShortUrlAction extends AbstractCreateShortUrlAction
             throw new InvalidArgumentException('Provided meta data is not valid', -1, $e);
         }
     }
-
-    private function getOptionalDate(array $postData, string $fieldName): ?Chronos
-    {
-        return isset($postData[$fieldName]) ? Chronos::parse($postData[$fieldName]) : null;
-    }
 }