Change Details

The URIEngineExtension should require a valid full URI. Not just "something internal", since "asdasd" may be considered an internal resource. Note that getDomain() is always a string and never null, so the strict check. This fix also involves a micro-optimization to avoid duplicate URI parsing. See 328aee033fbd Ref Q121

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 do extra URI validation, to require a valid URI, with at least a domain. Note that PhutilURI#getDomain() is always a string and never null, so the 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

The URIEngineExtension should require a valid full URI.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, Not just "something internal",there are some undocumented and untested cases like "asdasd" that may be considered since "asdasd" may be consideredinternal URIs as well. Or not. But this is just an internal resourceundocumented corner-case. Note that getDomain() is always a string and never null, so the strict check.In short: This fix also involves a micro-optimization to avoid duplicate URI parsingPhabricatorDatasourceURIEngineExtension should not be built over undocumented corner-cases and should do extra URI validation, to require a valid URI, with at least a domain. See 328aee033fbdNote that PhutilURI#getDomain() is always a string and never null, so the 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. Ref Q121Closes T15763