Page MenuHomePhabricator

Fix a bug where "View as Query" could replace a saved query row by ID, causing workboard 404s

Authored by epriestley on Nov 1 2018, 12:28 AM.
Referenced Files
F11538529: D19768.id47219.diff
Tue, Feb 7, 3:23 PM
Unknown Object (File)
Tue, Feb 7, 5:39 AM
Unknown Object (File)
Mon, Jan 23, 1:40 PM
Unknown Object (File)
Dec 30 2022, 3:15 PM
Unknown Object (File)
Dec 16 2022, 8:24 AM
Unknown Object (File)
Nov 22 2022, 2:24 PM
Unknown Object (File)
Nov 22 2022, 2:24 PM
Unknown Object (File)
Nov 22 2022, 2:24 PM
Restricted Owners Package



Fixes T13208. See that task for details.

The clone $query line is safe if $query is a builtin query (like "open").

However, if it's a saved query we clone not only the query parameters but the ID, too. Then when we save() the query later, we overwrite the original query.

So this would happen in the database. First, you run a query and save it as the workboard default (query key "abc123"):


Then we clone it and change the parameters, and save() it. But that causes an UPDATE ... WHERE id = 123 and the table now looks like this:


What we want is to create a new query instead, with an INSERT ...:

Test Plan
  • Followed reproduction steps from above.
    • With just the new save() guard, hit the guard error.
    • With the newCopy(), got a new copy of the query and "View as Query" remained functional without overwriting the original query row.
  • Ran migration, saw an affected board get fixed.

Diff Detail

rP Phabricator
Lint Not Applicable
Tests Not Applicable

Event Timeline

Owners added a subscriber: Restricted Owners Package.Nov 1 2018, 12:28 AM
epriestley added inline comments.

This should prevent this class of bug from ever occurring again. I made an effort to grep for other misuses of clone but didn't turn any up.

This revision is now accepted and ready to land.Nov 1 2018, 1:58 AM
This revision was automatically updated to reflect the committed changes.