Fix some "URI->alter(X, null)" callsites
ClosedPublic
Actions

Authored by epriestley on Feb 13 2019, 5:24 PM.

Details

Reviewers

amckinley

Maniphest Tasks

T13250: Some typeaheads use nonscalar HTTP parameters, which fatal under new "phutil_build_http_query()" rules

Commits

rPbe21dd3b52b8: Fix some "URI->alter(X, null)" callsites

Summary

Ref T13250. This internally calls replaceQueryParam(X, null) now, which fatals if the second parameter is null. I hit these legitimately, but I'll look for more callsites and follow up by either allowing this, removing alter(), fixing the callsites, or some combination.

(I'm not much of a fan of alter().)

Test Plan

Browsing a paginated list no longer complains about URI construction.

Diff Detail

Repository

rP Phabricator

Branch

uri7

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 21985
Build 30029: Run Core Tests
Build 30028: arc lint + arc unit

Event Timeline

epriestley created this revision.Feb 13 2019, 5:24 PM

There are no more explicit ->alter(X, null) callsites so I'm likely to let sleeping dogs lie for now. I'd like to gradually replace all alter() callsites but don't want to tempt fate too much given how much refactoring is already in this upcoming release.

Harbormaster completed remote builds in B21985: Diff 48141.Feb 13 2019, 5:26 PM

epriestley requested review of this revision.Feb 13 2019, 5:26 PM

amckinley accepted this revision.Feb 13 2019, 6:38 PM

This revision is now accepted and ready to land.Feb 13 2019, 6:38 PM

Closed by commit rPbe21dd3b52b8: Fix some "URI->alter(X, null)" callsites (authored by epriestley). · Explain WhyFeb 14 2019, 7:59 PM

This revision was automatically updated to reflect the committed changes.