The error is less opaque in PHP 7.3:

>>> UNRECOVERABLE FATAL ERROR <<<

Method PhutilURI::__toString() must not throw an exception, caught Exception: HTTP query parameter (with key &quot;repositoryPHIDs&quot;) is not a scalar. Parameters must all be scalars.

/Users/epriestley/dev/core/lib/phabricator/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php:0


┻━┻ ︵ ¯\_(ツ)_/¯ ︵ ┻━┻

I'm strongly considering allowing phutil_build_http_querystring() to build PHP-flavored query strings out of arrays again.

One problem right now is that this code:

$str = 'http://example.com/?X';
$uri = new PhutilURI($str);
(string)$uri;

...can fatal on the string cast for some values of X. It seems like:

• If the (string) cast would fatal, the fatal should probably be moved to construction time (new PhutilURI()) instead.
• No "reasonable" value of X should fatal (maybe NULL bytes or invalid UTF8 are allowed to fatal, but normal character strings should not fatal).

Today, we use the PHP-flavored x[]=1&x[]=2 query string construction in some UIs. For example, the checkboxes in Slowvote approval votes are constructed like this. These aren't exceptionally common, and it's possible we should move away from them, but they're hard to grep for and may be difficult to exorcise decisively. If we shift the fatal to construction time, some actual Phabricator URIs will not be valid PhutilURIs.

Maybe the cleaner approach is to change the PhutilURI parameter representation to allow duplicates and use parseQueryStringToPairList(), not parseQueryString(). I'm leaning in this direction? Then we can shift the fatal without creating any weird contradictions.

There are a very small number of noncritical setQueryParam() callsites in arcanist/ (3), instances/ (1), and services/ (3). Of these 7 callsites, 6 are about building Diviner links. I'm going to leave these as-is for now to generally simplify rolling this out, but they should be updated before setQueryParam() is actually removed.

(See T13251 for followup.)