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).