Page MenuHomePhabricator

Render query strings into concrete scalar "string" values immediately, not lazily
ClosedPublic

Authored by epriestley on Mar 5 2019, 9:42 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Mar 7, 2:03 AM
Unknown Object (File)
Feb 16 2024, 12:40 PM
Unknown Object (File)
Feb 10 2024, 3:20 PM
Unknown Object (File)
Feb 7 2024, 8:40 PM
Unknown Object (File)
Jan 4 2024, 5:20 PM
Unknown Object (File)
Dec 31 2023, 7:42 PM
Unknown Object (File)
Dec 27 2023, 8:00 PM
Unknown Object (File)
Dec 27 2023, 1:10 PM
Subscribers
None

Details

Summary

See D20067. (Previously, see T13242.) Currently, qsprintf() returns an object which is turned into a string immediately (to test for errors) and then again later, lazily, for actual use.

In theory, the two renderings may produce different results, if you pass one or more objects to qsprintf(...) and then modify the objects after the qsprintf(...) call but before the query is executed. This would likely be very, very surprising.

To avoid this, immediately render the string and store the flat string value, not the arguments.

This is technically also a performance change (it reduces calls to xsprintf_query() by about 50%, since we don't rebuild the string twice) but now justfiable purely as a correctness/simplicity change.

(I'm going to try to make a followup performance change where we just render the masked string and use it for both values if the masked and unmasked strings are identical, but I'll justify that with profiling.)

Test Plan

Browsed around, everything worked. Checked that sensitive values are properly masked in DarkConsole. (Also, added some debugging echo and observed ~50% fewer calls to xsprintf_query().)

Diff Detail

Repository
rPHU libphutil
Branch
cache1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 22189
Build 30331: Run Core Tests
Build 30330: arc lint + arc unit