The PhutilURI class has changed in 2019 Week 7. If you write custom code which uses PhutilURI, you may need to adjust it. (If you don't have any custom code, you can ignore this.)
Background
PhutilURI previously had some destructive behavior on certain inputs, and some PHP-flavored magic behavior on other inputs. We've removed this magic to generally increase goodwill and order in the world.
The PhutilURI internal representation of query parameters was previously a map, so URIs in the form http://example.com/?x=1&x=2 would lose information when parsed: only one value of x would be retained. Although this kind of URI is unusual and most applications do not expect it to be represented nondestructively, it's perfectly legal, and it was vaguely concerning from a purity standpoint that our URI primitive could not represent it.
The PhutilURI parser also used a PHP-flavored interpretation of parameters in the form x[]=1&x[]=2, and would parse them into a PHP value of type array. While this is somewhat useful, it's also somewhat troubling as default behavior for a URI primitive.
PhutilURI now stores parameters as a list of pairs and no longer parses PHP structures from inputs.
In all cases, query parameters must now be scalars.
API Changes
The API has changed to accommodate the new storage, disambiguate what some methods do, and remove some magic. In particular, it was not obvious whether the old setQueryParams(map) method first removed all query parameters, or removed only those query parameters with keys present in map (it did the former), and this ambiguity has become more complex with the introduction of the possibility of duplicate keys.
Since the query parameter list may now include multiple instances of the same key, the verb "set" is generally ambiguous.
These are the API changes:
| Old Method | New Method | Notes |
|---|---|---|
| getQueryParams() | getQueryParamsAsMap()/getQueryParamsAsPairList() | getQueryParamsAsMap() will throw if duplicate parameters prevent nondestructive representation as a map. |
| setQueryParams(map) | new PhutilURI(uri, map) | Essentially every callsite for setQueryParams() was immediately after construction. This is now a constructor parameter. Otherwise, iterate with appendQueryParam(key, value). |
| setQueryParams(array()) | removeAllQueryParams() | Method intent is now explicit. |
| setQueryParam(key, value) | replaceQueryParam(key, value) | Removes all copies of query parameter key, then appends a parameter named key with value value. |
| setQueryParam(key, null) | removeQueryParam(key) | Magic meaning of null removed. Method intent is now explicit. |
| appendQueryParam(key, value) | Appends a new parameter without removing anything. Allows appending duplicates. | |
| alter(key, value) | clone + replaceQueryParam(key, value) | Prefer clone over alter. |
| alter(key, null) | clone + removeQueryParam(key) | Prefer clone over alter. |
The old methods still function with (mostly) unchanged semantics, but they are now deprecated and will be removed in some future version of Phabricator.
Discussion
This change also has a lot against it: it's disruptive and doesn't serve clear goals. It grew out of D20049 causing a whole lot of relatively serious fatals, see T13250. We generally try not to make changes like this.
This change does put us in a better place overall, and fixes some very sketchy behavior in very fundamental primitives, and moves us further from the great dark ichor pools of PHP behavior, and changes like this will only get harder to make in the future (e.g., with T5055 on the horizon).