2.x #2767 Optimize URLHelper methods is_local and is_external

docs/v2/upgrade-guides/2.0.md

@@ -1331,6 +1331,16 @@ $genre->posts([
 ]);
 ```
 
+### Timber\URLHelper
+
+#### `Timber\URLHelper::get_params()`
+
+The `Timber\URLHelper::get_params()` method now returns `false` if passed an index that does not exist, whereas before it returned `null` in that case. This is for better consistency with other core API methods and only affects code using `===` or equivalent checks on return values from this method.
+
+#### `Timber\URLHelper::is_local()` and `Timber\URLHelper::is_external()`
+
+The `Timber\URLHelper::is_local()` and `Timber\URLHelper::is_external()` methods were updated to fix some bugs with false positives, e.g. when an URL appeared as part of the query. The methods will now analyze the hostname instead of just checking the whole URL for the site’s URL.
+
 ## Post Excerpts
 
 ### Excerpt instead of Preview
@@ -1523,10 +1533,6 @@ add_filter('timber/twig/environment/options', function ($options) {
 
 Read more about this in the [Escaping Guide](https://timber.github.io/docs/v2/guides/escaping/).
 
-## URLHelper
-
-The `URLHelper::get_params()` method now returns `false` if passed an index that does not exist, whereas before it returned `null` in that case. This is for better consistency with other core API methods and only affects code using `===` or equivalent checks on return values from this method.
-
 ## Integrations
 
 There’s a new way how integrations are initialized internally. Timber uses a new `timber/integrations` filter to get all integrations it should init. Each integration uses an interface with a `should_init()` and an `init()` method that Timber can use to check whether an integration should be initialized before it actually initializes an integration.

src/URLHelper.php

@@ -135,18 +135,26 @@ public static function get_host()
     }
 
     /**
+     * Checks whether a URL or domain is local.
+     *
+     * True if `$url` has a host name matching the server’s host name. False if
+     * a relative URL or if it’s a subdomain.
+     *
      * @api
      *
-     * @param string $url
+     * @param string $url URL to check.
      * @return bool
      */
-    public static function is_local($url)
+    public static function is_local(string $url): bool
     {
-        $host = self::get_host();
-        if (!empty($host) && \strstr($url, $host)) {
-            return true;
+        $host = \wp_parse_url($url, \PHP_URL_HOST);
+        if (null === $host || false === $host) {
+            $host = $url;
         }
-        return false;
+
+        $wp_host = self::get_host();
+
+        return $wp_host && $wp_host === $host;
     }
 
     /**
@@ -427,23 +435,25 @@ private static function is_internal_content($url)
     }
 
     /**
-     * Determines if URL is an external URL.
+     * Checks whether a URL or domain is external.
      *
-     * True if `$url` is an external url or subdomain (http://cdn.example.org = true). False if
-     * relative or local true if it's a subdomain
+     * True if the `$url` host name does not match the server’s host name.
+     * Otherwise, false.
      *
      * @api
-     * @param  string $url to evalute.
+     * @param  string $url URL to evalute.
      * @return bool
      */
-    public static function is_external($url)
+    public static function is_external(string $url): bool
     {
-        $has_http = \strstr(\strtolower($url), 'http') || \strstr(\strtolower($url), '//');
-        $on_domain = \strstr($url, self::get_host());
-        if ($has_http && !$on_domain) {
-            return true;
+        $has_scheme = \str_starts_with($url, '//') || \wp_parse_url($url, \PHP_URL_SCHEME);
+
+        if ($has_scheme) {
+            return !self::is_local($url);
         }
-        return false;
+
+        // Check with added scheme.
+        return !self::is_local('//' . $url);
     }
 
     /**

tests/test-timber-url-helper.php

@@ -257,7 +257,24 @@ public function testPathBase()
 
     public function testIsLocal()
     {
+        // Local.
+        $this->assertTrue(Timber\URLHelper::is_local($_SERVER['HTTP_HOST']));
+        $this->assertTrue(Timber\URLHelper::is_local('example.org'));
+        $this->assertTrue(Timber\URLHelper::is_local('//example.org'));
+        $this->assertTrue(Timber\URLHelper::is_local('http://example.org'));
+        $this->assertTrue(Timber\URLHelper::is_local('https://example.org'));
+        $this->assertTrue(Timber\URLHelper::is_local('https://example.org/'));
+        $this->assertTrue(Timber\URLHelper::is_local('https://example.org/example'));
+
+        // External.
+        $this->assertFalse(Timber\URLHelper::is_local('wordpress.org'));
+        $this->assertFalse(Timber\URLHelper::is_local('//wordpress.org'));
+        $this->assertFalse(Timber\URLHelper::is_local('//wordpress.org/example.org'));
         $this->assertFalse(Timber\URLHelper::is_local('http://wordpress.org'));
+        $this->assertFalse(Timber\URLHelper::is_local('https://wordpress.org'));
+        $this->assertFalse(Timber\URLHelper::is_local('https://example.com/example.org'));
+        $this->assertFalse(Timber\URLHelper::is_local('http://example.com/' . $_SERVER['HTTP_HOST']));
+        $this->assertFalse(Timber\URLHelper::is_local('http://foo' . $_SERVER['HTTP_HOST'] . '/' . $_SERVER['HTTP_HOST']));
     }
 
     public function testCurrentURLWithServerPort()
@@ -321,15 +338,29 @@ public function testIsURL()
 
     public function testIsExternal()
     {
-        $local = 'http://example.org';
-        $subdomain = 'http://cdn.example.org';
-        $external = 'http://upstatement.com';
-        $protocol_relative = '//upstatement.com';
-
-        $this->assertFalse(Timber\URLHelper::is_external($local));
-        $this->assertFalse(Timber\URLHelper::is_external($subdomain));
-        $this->assertTrue(Timber\URLHelper::is_external($external));
-        $this->assertTrue(Timber\URLHelper::is_external($protocol_relative));
+        // Local.
+        $this->assertFalse(Timber\URLHelper::is_external('example.org'));
+        $this->assertFalse(Timber\URLHelper::is_external('//example.org'));
+        $this->assertFalse(Timber\URLHelper::is_external('http://example.org'));
+        $this->assertFalse(Timber\URLHelper::is_external('https://example.org/'));
+        $this->assertFalse(Timber\URLHelper::is_external('https://example.org/example.com'));
+        $this->assertFalse(Timber\URLHelper::is_external('https://example.org/example'));
+
+        // Subdomain.
+        $this->assertTrue(Timber\URLHelper::is_external('//cdn.example.org'));
+        $this->assertTrue(Timber\URLHelper::is_external('http://cdn.example.org'));
+        $this->assertTrue(Timber\URLHelper::is_external('https://cdn.example.org'));
+        $this->assertTrue(Timber\URLHelper::is_external('cdn.example.org'));
+
+        // External.
+        $this->assertTrue(Timber\URLHelper::is_external('upstatement.com'));
+        $this->assertTrue(Timber\URLHelper::is_external('//upstatement.com'));
+        $this->assertTrue(Timber\URLHelper::is_external('http://upstatement.com'));
+        $this->assertTrue(Timber\URLHelper::is_external('https://upstatement.com'));
+
+        // Other.
+        $this->assertTrue(Timber\URLHelper::is_external('https://example.com/' . $_SERVER['HTTP_HOST']));
+        $this->assertTrue(Timber\URLHelper::is_external('https://foo' . $_SERVER['HTTP_HOST'] . '/' . $_SERVER['HTTP_HOST']));
     }
 
     public function testIsExternalContent()