Page MenuHomePhabricator

Make qsprintf() return an object, not a string, to support %P and hardening of %Q
ClosedPublic

Authored by epriestley on Nov 7 2018, 12:23 AM.
Tags
None
Referenced Files
F18733822: D19781.id.diff
Tue, Sep 30, 10:42 PM
F18568707: D19781.id47284.diff
Wed, Sep 10, 12:22 AM
F18568706: D19781.id47247.diff
Wed, Sep 10, 12:22 AM
F18509911: D19781.id.diff
Fri, Sep 5, 3:40 AM
F18503453: D19781.diff
Thu, Sep 4, 11:06 PM
F18459515: D19781.id47247.diff
Sep 1 2025, 5:17 PM
F18368287: D19781.diff
Aug 28 2025, 12:42 AM
F18169907: D19781.id47284.diff
Aug 15 2025, 5:34 PM
Subscribers
None

Details

Summary

Ref T13217. Ref T13216. Previously, we changed csprintf() to return an object instead of a string to support %P for passwords. Prepare for a %P for qsprintf(...) too. T13217 discusses general plans here, although %P, %LA, %LO, and %LQ are not implemented yet.

This may be a little rocky, but the csprintf() change was generally fairly straightforward so I have reasonably high hopes about this one not being too terribly painful.

Test Plan

Loaded a Phabricator page -- which now generates hundreds of "unsafe query construction" errors, but still works.

Diff Detail

Repository
rPHU libphutil
Branch
qobject1
Lint
Lint Passed
SeverityLocationCodeMessage
Advicesrc/xsprintf/qsprintf.php:300XHP16TODO Comment
Unit
Tests Passed
Build Status
Buildable 21096
Build 28670: Run Core Tests
Build 28669: arc lint + arc unit

Event Timeline

amckinley added inline comments.
src/xsprintf/qsprintf.php
200

Worth adding a "TODO" here to clean up later? Or is the conversion for this going to be effectively endless?

This revision is now accepted and ready to land.Nov 7 2018, 8:27 PM