recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

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

Valerio Bozzolan 27f4b832 9ebe8562

+26 -24
+26 -24
src/applications/meta/engineextension/PhabricatorDatasourceURIEngineExtension.php
··· 10 10 public function newJumpURI($query) { 11 11 // If you search for a URI on the local install, just redirect to that 12 12 // URI as though you had pasted it into the URI bar. 13 - if (PhabricatorEnv::isSelfURI($query)) { 14 - // Strip off the absolute part of the URI. If we don't, the URI redirect 15 - // validator will get upset that we're performing an unmarked external 16 - // redirect. 13 + // Skip things that are really not full URLs, like "asdasd". 14 + // Note that the backend of "isSelfURI" is faster with a PhutilURI. 15 + $uri = new PhutilURI($query); 16 + if ($uri->getDomain() === '' || !PhabricatorEnv::isSelfURI($uri)) { 17 + return null; 18 + } 17 19 18 - // The correct host and protocol may also differ from the host and 19 - // protocol used in the search: for example, if you search for "http://" 20 - // we want to redirect to "https://" if an install is HTTPS, and 21 - // the "isSelfURI()" check includes alternate domains in addition to the 22 - // canonical domain. 20 + // Strip off the absolute part of the URI. If we don't, the URI redirect 21 + // validator will get upset that we're performing an unmarked external 22 + // redirect. 23 23 24 - $uri = id(new PhutilURI($query)) 25 - ->setDomain(null) 26 - ->setProtocol(null) 27 - ->setPort(null); 28 - 29 - $uri = phutil_string_cast($uri); 24 + // The correct host and protocol may also differ from the host and 25 + // protocol used in the search: for example, if you search for "http://" 26 + // we want to redirect to "https://" if an install is HTTPS, and 27 + // the "isSelfURI()" check includes alternate domains in addition to the 28 + // canonical domain. 29 + $uri = $uri 30 + ->setDomain(null) 31 + ->setProtocol(null) 32 + ->setPort(null); 30 33 31 - // See T13412. If the URI was in the form "http://dev.example.com" with 32 - // no trailing slash, there may be no path. Redirecting to the empty 33 - // string is considered an error by safety checks during redirection, 34 - // so treat this like the user entered the URI with a trailing slash. 35 - if (!strlen($uri)) { 36 - $uri = '/'; 37 - } 34 + $uri = phutil_string_cast($uri); 38 35 39 - return $uri; 36 + // See T13412. If the URI was in the form "http://dev.example.com" with 37 + // no trailing slash, there may be no path. Redirecting to the empty 38 + // string is considered an error by safety checks during redirection, 39 + // so treat this like the user entered the URI with a trailing slash. 40 + if (!strlen($uri)) { 41 + $uri = '/'; 40 42 } 41 43 42 - return null; 44 + return $uri; 43 45 } 44 46 }