From 27f4b8321a5e39bdf6f869192d817c94aa2c7515 Mon Sep 17 00:00:00 2001
From: Valerio Bozzolan <nsa+gitspam@succhia.cz>
Date: Tue, 26 Mar 2024 07:29:53 +0100
Subject: [PATCH] Fix important regression in search engine

Summary:
Interestingly Phorge allows to paste a Phorge URL in the search bar to be redirected there...

Now this interesting feature was crashing the whole search engine for simple queries like "asdasd"
with the following exception message:

    Refusing to redirect to local resource "asdasd". This URI is not formatted in a recognizable way.

You are affected by this crash after this commit:

328aee033fbdc704620e2facae4aa68b836217bb

This specific commit is probably legitimate since "#asdasd" and "/asdasd" are indeed internal URIs,
plus, yes, there are some undocumented and untested cases like "asdasd" that may be considered
internal URIs as well. Or not. But this is just an undocumented corner-case.

In short:

PhabricatorDatasourceURIEngineExtension should not be built over undocumented corner-cases and
should require an absolute URI, with at least a domain.

Note that PhutilURI#getDomain() is always a string and never null, so we can do a strict check.

This fix also involves a micro-optimization to avoid duplicate URI parsing. Since the method was calling
the constructor PhutilURI(string) twice (line 13, line 24). Now just once.

Closes T15763

Test Plan:
Search "lol", no crash anymore.

Copy a random Phorge URL and paste that in your search bar. It still redirects there.

Reviewers: zhe, avivey, O1 Blessed Committers, speck

Reviewed By: O1 Blessed Committers, speck

Subscribers: speck, tobiaswiese, Matthew, Cigaryno

Maniphest Tasks: T15763

Differential Revision: https://we.phorge.it/D25561
---
 ...habricatorDatasourceURIEngineExtension.php | 58 ++++++++++---------
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/src/applications/meta/engineextension/PhabricatorDatasourceURIEngineExtension.php b/src/applications/meta/engineextension/PhabricatorDatasourceURIEngineExtension.php
index 16d8dd9315..76d60af1e6 100644
--- a/src/applications/meta/engineextension/PhabricatorDatasourceURIEngineExtension.php
+++ b/src/applications/meta/engineextension/PhabricatorDatasourceURIEngineExtension.php
@@ -10,35 +10,37 @@ final class PhabricatorDatasourceURIEngineExtension
   public function newJumpURI($query) {
     // If you search for a URI on the local install, just redirect to that
     // URI as though you had pasted it into the URI bar.
-    if (PhabricatorEnv::isSelfURI($query)) {
-      // Strip off the absolute part of the URI. If we don't, the URI redirect
-      // validator will get upset that we're performing an unmarked external
-      // redirect.
-
-      // The correct host and protocol may also differ from the host and
-      // protocol used in the search: for example, if you search for "http://"
-      // we want to redirect to "https://" if an install is HTTPS, and
-      // the "isSelfURI()" check includes alternate domains in addition to the
-      // canonical domain.
-
-      $uri = id(new PhutilURI($query))
-        ->setDomain(null)
-        ->setProtocol(null)
-        ->setPort(null);
-
-      $uri = phutil_string_cast($uri);
-
-      // See T13412. If the URI was in the form "http://dev.example.com" with
-      // no trailing slash, there may be no path. Redirecting to the empty
-      // string is considered an error by safety checks during redirection,
-      // so treat this like the user entered the URI with a trailing slash.
-      if (!strlen($uri)) {
-        $uri = '/';
-      }
-
-      return $uri;
+    // Skip things that are really not full URLs, like "asdasd".
+    // Note that the backend of "isSelfURI" is faster with a PhutilURI.
+    $uri = new PhutilURI($query);
+    if ($uri->getDomain() === '' || !PhabricatorEnv::isSelfURI($uri)) {
+      return null;
     }
 
-    return null;
+    // Strip off the absolute part of the URI. If we don't, the URI redirect
+    // validator will get upset that we're performing an unmarked external
+    // redirect.
+
+    // The correct host and protocol may also differ from the host and
+    // protocol used in the search: for example, if you search for "http://"
+    // we want to redirect to "https://" if an install is HTTPS, and
+    // the "isSelfURI()" check includes alternate domains in addition to the
+    // canonical domain.
+    $uri = $uri
+      ->setDomain(null)
+      ->setProtocol(null)
+      ->setPort(null);
+
+    $uri = phutil_string_cast($uri);
+
+    // See T13412. If the URI was in the form "http://dev.example.com" with
+    // no trailing slash, there may be no path. Redirecting to the empty
+    // string is considered an error by safety checks during redirection,
+    // so treat this like the user entered the URI with a trailing slash.
+    if (!strlen($uri)) {
+      $uri = '/';
+    }
+
+    return $uri;
   }
 }