From 05a64b8d9e57f1ca54d8a0dd7b89b4f5792f35f7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 6 Dec 2019 22:38:22 +0100 Subject: [PATCH 1/3] 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; - } } From 57c91aca3cd65049bba58bbd835f0686e2f041e2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 6 Dec 2019 22:40:08 +0100 Subject: [PATCH 2/3] Updated changelog with v1.20.2 --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32621aa9..c28aa0af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ 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). -## [Unreleased] +## 1.20.2 - 2019-12-06 #### Added @@ -26,6 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#561](https://github.com/shlinkio/shlink/issues/561) Fixed `db:migrate` command failing because yaml extension is not installed, which makes config file not to be readable. * [#562](https://github.com/shlinkio/shlink/issues/562) Fixed internal server error being returned when renaming a tag to another tag's name. Now a meaningful API error with status 409 is returned. +* [#555](https://github.com/shlinkio/shlink/issues/555) Fixed internal server error being returned when invalid dates are provided for new short URLs. Now a 400 is returned, as intended. ## 1.20.1 - 2019-11-17 From 3e9b7751143c4d6654398ce7a086706fd1fc8252 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 6 Dec 2019 22:45:07 +0100 Subject: [PATCH 3/3] Fixed failing test --- module/Core/test/Model/ShortUrlMetaTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/module/Core/test/Model/ShortUrlMetaTest.php b/module/Core/test/Model/ShortUrlMetaTest.php index 7c8e5aae..fb02fcbc 100644 --- a/module/Core/test/Model/ShortUrlMetaTest.php +++ b/module/Core/test/Model/ShortUrlMetaTest.php @@ -42,9 +42,6 @@ class ShortUrlMetaTest extends TestCase ]]; yield [[ ShortUrlMetaInputFilter::VALID_UNTIL => 500, - ]]; - yield [[ - ShortUrlMetaInputFilter::MAX_VISITS => new stdClass(), ShortUrlMetaInputFilter::DOMAIN => 4, ]]; }