Fixed regresssion issue of Utils::Url() not returning false on fa…

…ilure #2524
getgrav · May 27, 2019 · d227a82 · d227a82
1 parent 9825daa
commit d227a82
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@
     * Fixed bitwise operator in `TwigExtension::exifFunc()` [#2518](https://github.com/getgrav/grav/issues/2518)
     * Fixed issue with lang prefix incorrectly identifying as admin [#2511](https://github.com/getgrav/grav/issues/2511)
     * Fixed issue with `U0ils::pathPrefixedBYLanguageCode()` and trailing slash [#2510](https://github.com/getgrav/grav/issues/2511) 
+    * Fixed regresssion issue of `Utils::Url()` not returning `false` on failure. Added new optional `fail_gracefully` 3rd attribute to return string that caused failure [#2524](https://github.com/getgrav/grav/issues/2524)
 
 # v1.6.9
 ## 05/09/2019

diff --git a/system/src/Grav/Common/Utils.php b/system/src/Grav/Common/Utils.php
@@ -30,19 +30,24 @@ abstract class Utils
      *
      * @param string $input
      * @param bool $domain
+     * @param bool $fail_gracefully
      * @return bool|null|string
      */
-    public static function url($input, $domain = false)
+    public static function url($input, $domain = false, $fail_gracefully = false)
     {
-        if (!trim((string)$input)) {
-            $input = '/';
+        if ((!is_string($input) && !method_exists($input, '__toString')) || !trim($input)) {
+            if ($fail_gracefully) {
+                $input = '/';
+            } else {
+                return false;
+            }
         }
 
         if (Grav::instance()['config']->get('system.absolute_urls', false)) {
             $domain = true;
         }
 
-        if (Grav::instance()['uri']->isExternal($input)) {
+        if (Uri::isExternal($input)) {
             return $input;
         }
 
@@ -57,26 +62,34 @@ public static function url($input, $domain = false)
         if (Utils::contains((string)$input, '://')) {
             /** @var UniformResourceLocator $locator */
             $locator = Grav::instance()['locator'];
-
             $parts = Uri::parseUrl($input);
 
             if ($parts) {
-                $resource = $locator->findResource("{$parts['scheme']}://{$parts['host']}{$parts['path']}", false);
+                try {
+                    $resource = $locator->findResource("{$parts['scheme']}://{$parts['host']}{$parts['path']}", false);
+                } catch (\Exception $e) {
+                    if ($fail_gracefully) {
+                        return $input;
+                    } else {
+                        return false;
+                    }
+                }
 
-                if (isset($parts['query'])) {
+                if ($resource && isset($parts['query'])) {
                     $resource = $resource . '?' . $parts['query'];
                 }
             } else {
                 // Not a valid URL (can still be a stream).
                 $resource = $locator->findResource($input, false);
             }
 
-
         } else {
             $resource = $input;
         }
 
-
+        if (!$fail_gracefully && $resource === false) {
+            return false;
+        }
 
         return rtrim($uri->rootUrl($domain), '/') . '/' . ($resource ?? '');
     }

diff --git a/tests/unit/Grav/Common/UtilsTest.php b/tests/unit/Grav/Common/UtilsTest.php
@@ -379,33 +379,53 @@ public function testUrl()
     {
         $this->uri->initializeWithUrl('http://testing.dev/path1/path2')->init();
 
+        // Fail hard
+        $this->assertSame(false, Utils::url('', true));
+        $this->assertSame(false, Utils::url(''));
+        $this->assertSame(false, Utils::url('foo://bar/baz'));
+        $this->assertSame(false, Utils::url(new stdClass()));
+        $this->assertSame(false, Utils::url(['foo','bar','baz']));
+
+        // Fail Gracefully
+        $this->assertSame('/', Utils::url('/', false, true));
+        $this->assertSame('/', Utils::url('', false, true));
+        $this->assertSame('foo://bar/baz', Utils::url('foo://bar/baz', false, true));
+        $this->assertSame('/', Utils::url(new stdClass(), false, true));
+        $this->assertSame('/', Utils::url(['foo','bar','baz'], false, true));
+
+        $this->assertSame('/', Utils::url('/'));
         $this->assertSame('http://testing.dev/', Utils::url('/', true));
-        $this->assertSame('http://testing.dev/', Utils::url('', true));
         $this->assertSame('http://testing.dev/path1', Utils::url('/path1', true));
-        $this->assertSame('/', Utils::url('/'));
-        $this->assertSame('/', Utils::url(''));
         $this->assertSame('/path1', Utils::url('/path1'));
         $this->assertSame('/path1/path2', Utils::url('/path1/path2'));
-
         $this->assertSame('http://testing.dev/foobar.jpg', Utils::url('foobar.jpg', true));
         $this->assertSame('http://testing.dev/foobar.jpg', Utils::url('/foobar.jpg', true));
         $this->assertSame('http://testing.dev/path1/foobar.jpg', Utils::url('/path1/foobar.jpg', true));
         $this->assertSame('/foobar.jpg', Utils::url('/foobar.jpg'));
         $this->assertSame('/foobar.jpg', Utils::url('foobar.jpg'));
         $this->assertSame('/path1/foobar.jpg', Utils::url('/path1/foobar.jpg'));
         $this->assertSame('/path1/path2/foobar.jpg', Utils::url('/path1/path2/foobar.jpg'));
+
     }
 
     public function testUrlWithRoot()
     {
         $this->uri->initializeWithUrlAndRootPath('http://testing.dev/subdir/path1/path2', '/subdir')->init();
 
+        // Fail hard
+        $this->assertSame(false, Utils::url('', true));
+        $this->assertSame(false, Utils::url(''));
+        $this->assertSame(false, Utils::url('foo://bar/baz'));
+
+        // Fail Gracefully
+        $this->assertSame('/subdir/', Utils::url('/', false, true));
+        $this->assertSame('/subdir/', Utils::url('', false, true));
+        $this->assertSame('foo://bar/baz', Utils::url('foo://bar/baz', false, true));
+
         $this->assertSame('http://testing.dev/subdir/', Utils::url('/', true));
-        $this->assertSame('http://testing.dev/subdir/', Utils::url('', true));
         $this->assertSame('http://testing.dev/subdir/path1', Utils::url('/path1', true));
         $this->assertSame('http://testing.dev/subdir/path1', Utils::url('/subdir/path1', true));
         $this->assertSame('/subdir/', Utils::url('/'));
-        $this->assertSame('/subdir/', Utils::url(''));
         $this->assertSame('/subdir/path1', Utils::url('/path1'));
         $this->assertSame('/subdir/path1/path2', Utils::url('/path1/path2'));
         $this->assertSame('/subdir/path1/path2', Utils::url('/subdir/path1/path2'));