From 4596aad2f5556ef1e8ae9df6cc7cb6b8aff4e8d9 Mon Sep 17 00:00:00 2001 From: Zdeno Kuzmany Date: Wed, 22 Jan 2020 13:26:23 +0100 Subject: [PATCH 1/3] URL validation improvement --- app/bundles/CoreBundle/Helper/UrlHelper.php | 14 ++++++++++++++ .../PageBundle/Controller/PublicController.php | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/bundles/CoreBundle/Helper/UrlHelper.php b/app/bundles/CoreBundle/Helper/UrlHelper.php index 7af2b87e623..2816e84ca6d 100644 --- a/app/bundles/CoreBundle/Helper/UrlHelper.php +++ b/app/bundles/CoreBundle/Helper/UrlHelper.php @@ -324,4 +324,18 @@ private static function removeTrailingNonAlphaNumeric($string) return $string; } + + /** + * @param string $url + * + * @return bool + */ + public static function isValidateUrl($url) + { + $path = parse_url($url, PHP_URL_PATH); + $encoded_path = array_map('urlencode', explode('/', $path)); + $url = str_replace($path, implode('/', $encoded_path), $url); + + return filter_var($url, FILTER_VALIDATE_URL); + } } diff --git a/app/bundles/PageBundle/Controller/PublicController.php b/app/bundles/PageBundle/Controller/PublicController.php index cce560db39f..1b82d068d91 100644 --- a/app/bundles/PageBundle/Controller/PublicController.php +++ b/app/bundles/PageBundle/Controller/PublicController.php @@ -491,7 +491,7 @@ public function redirectAction($redirectId) $url = UrlHelper::sanitizeAbsoluteUrl($url); - if (false === filter_var($url, FILTER_VALIDATE_URL)) { + if (!UrlHelper::isValidateUrl($url)) { throw $this->createNotFoundException($this->translator->trans('mautic.core.url.error.404', ['%url%' => $url])); } From caf75f333e2a024634db5d8027d953b9c7ad7b06 Mon Sep 17 00:00:00 2001 From: Zdeno Kuzmany Date: Wed, 22 Jan 2020 13:31:33 +0100 Subject: [PATCH 2/3] Typo --- app/bundles/CoreBundle/Helper/UrlHelper.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/bundles/CoreBundle/Helper/UrlHelper.php b/app/bundles/CoreBundle/Helper/UrlHelper.php index 2816e84ca6d..1a0abf1df37 100644 --- a/app/bundles/CoreBundle/Helper/UrlHelper.php +++ b/app/bundles/CoreBundle/Helper/UrlHelper.php @@ -326,6 +326,8 @@ private static function removeTrailingNonAlphaNumeric($string) } /** + * FILTER_VALIDATE_URL allow only alphanumerics [0-9a-zA-Z], the special characters "$-_.+!*'()," [not including the quotes - ed]. This validation passed also special characters in URL. + * * @param string $url * * @return bool @@ -333,8 +335,8 @@ private static function removeTrailingNonAlphaNumeric($string) public static function isValidateUrl($url) { $path = parse_url($url, PHP_URL_PATH); - $encoded_path = array_map('urlencode', explode('/', $path)); - $url = str_replace($path, implode('/', $encoded_path), $url); + $encodedPath = array_map('urlencode', explode('/', $path)); + $url = str_replace($path, implode('/', $encodedPath), $url); return filter_var($url, FILTER_VALIDATE_URL); } From 7de923e38e14c4b034f75fd5b1ac84fc343ac138 Mon Sep 17 00:00:00 2001 From: Zdeno Kuzmany Date: Wed, 22 Jan 2020 17:21:27 +0100 Subject: [PATCH 3/3] Refactor method and add unit tests --- app/bundles/CoreBundle/Helper/UrlHelper.php | 7 ++++--- .../CoreBundle/Tests/Unit/Helper/UrlHelperTest.php | 8 ++++++++ app/bundles/PageBundle/Controller/PublicController.php | 2 +- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/app/bundles/CoreBundle/Helper/UrlHelper.php b/app/bundles/CoreBundle/Helper/UrlHelper.php index 1a0abf1df37..acc0dfb45a2 100644 --- a/app/bundles/CoreBundle/Helper/UrlHelper.php +++ b/app/bundles/CoreBundle/Helper/UrlHelper.php @@ -326,18 +326,19 @@ private static function removeTrailingNonAlphaNumeric($string) } /** - * FILTER_VALIDATE_URL allow only alphanumerics [0-9a-zA-Z], the special characters "$-_.+!*'()," [not including the quotes - ed]. This validation passed also special characters in URL. + * This method return true with special characters in URL, for example https://domain.tld/é.pdf + * filter_var($url, FILTER_VALIDATE_URL) allow only alphanumerics [0-9a-zA-Z], the special characters "$-_.+!*'()," [not including the quotes - ed]. * * @param string $url * * @return bool */ - public static function isValidateUrl($url) + public static function isValidUrl($url) { $path = parse_url($url, PHP_URL_PATH); $encodedPath = array_map('urlencode', explode('/', $path)); $url = str_replace($path, implode('/', $encodedPath), $url); - return filter_var($url, FILTER_VALIDATE_URL); + return (bool) filter_var($url, FILTER_VALIDATE_URL); } } diff --git a/app/bundles/CoreBundle/Tests/Unit/Helper/UrlHelperTest.php b/app/bundles/CoreBundle/Tests/Unit/Helper/UrlHelperTest.php index 4fd71f2f5c2..1920cac975b 100644 --- a/app/bundles/CoreBundle/Tests/Unit/Helper/UrlHelperTest.php +++ b/app/bundles/CoreBundle/Tests/Unit/Helper/UrlHelperTest.php @@ -181,4 +181,12 @@ public function testGetUrlsFromPlaintextWithSymbols() ) ); } + + public function testUrlValid() + { + $this->assertTrue(UrlHelper::isValidUrl('https://domain.tld/e')); + $this->assertTrue(UrlHelper::isValidUrl('https://domain.tld/é')); + $this->assertFalse(UrlHelper::isValidUrl('notvalidurl')); + $this->assertFalse(UrlHelper::isValidUrl('notvalidurlé')); + } } diff --git a/app/bundles/PageBundle/Controller/PublicController.php b/app/bundles/PageBundle/Controller/PublicController.php index 1b82d068d91..49501ab7337 100644 --- a/app/bundles/PageBundle/Controller/PublicController.php +++ b/app/bundles/PageBundle/Controller/PublicController.php @@ -491,7 +491,7 @@ public function redirectAction($redirectId) $url = UrlHelper::sanitizeAbsoluteUrl($url); - if (!UrlHelper::isValidateUrl($url)) { + if (!UrlHelper::isValidUrl($url)) { throw $this->createNotFoundException($this->translator->trans('mautic.core.url.error.404', ['%url%' => $url])); }