[wp-trac] [WordPress Trac] #54042: Extending wpdb::prepare() to support table/field names, and IN() operator

[WordPress Trac]

#54042: Extending wpdb::prepare() to support table/field names, and IN() operator
-------------------------------------------------+-------------------------
 Reporter:  craigfrancis                         |       Owner:  (none)
     Type:  enhancement                          |      Status:  new
 Priority:  normal                               |   Milestone:  Future
                                                 |  Release
Component:  Database                             |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch dev-feedback needs-        |     Focuses:
  testing early has-unit-tests                   |
-------------------------------------------------+-------------------------

Comment (by apokalyptik):

 Over at A8C this has come up a number of times in the past month or so for
 both my team and others...

 It would be invaluable to have this functionality in core because tricks
 such as vsprintf argnum hacks used to get around code sniffers, etc,
 undermine the integrity and security that those checks are meant to
 enforce.

 I wanted to implement this as a flag: `%,d`, `%,f`, and `%,s`
 specifically.

 So I started looking into how to do this safely. It turns out that this is
 very complicated and difficult for a number of reasons. I'll list a couple
 below...

 1. prepare does not actually parse like vsprintf does, but uses a regex
 which is, i think, imperfect (aren't they all?)
 2. for legacy reasons argnum's aren't escaped (which is why they're used
 as hacks for IN() statements)
 3. to avoid (possibly infinite) recursion with wpbd::prepare() you want to
 expand the specifiers and the args but
 4. this changes the order, and number, of the arguments and thus what the
 argument numbers need to be to stay correct.

 The other thing is that it needed to be as fully backwards compatible with
 prepare() (or at least punt and return to prepare on some sort of error)
 as possible.  This meant that I needed to (and did) run my solutions
 through the entire wpdb unit testing suite (and extend it to include
 implosion specifiers.)  All tests pass, excepting assertions around
 doing_it_wrong where the tests don't expect my new doing_it_wrong's in
 another function.  To replicate this you have to edit wpdb::prepare() and
 comment out the checks which prevent this scanning from happening
 unnecessarily.

 So I ended up having to write a parser to accomplish the task. Which I
 did. This was mostly very easy except in the argnum case (which is where
 all of the complication came in.)  For this case what I ended up doing is
 de-referencing the arguments and having them "point" to the newly de-
 referenced args element.  Originally I was just going to change them
 inline after de-referencing them from, say, %1$s to %s but because the
 placeholders for those come after the positional arguments that was a no-
 go because it severely mis-ordered the query. Also, as mentioned earlier,
 they are not escaped.  I would have just interpolated the parameters
 properly inline if it weren't for that.. Also I wanted prepare to continue
 handling the actual escaping of the data... So append and re-point is what
 I went with.

 The code is more complex than I would have liked but because of the need
 to parse argnums and reorder parameters correctly it is what it is...

 My code can be found here: https://github.com/apokalyptik/wordpress-
 develop/compare/trunk...wpdb-implode-specifier?expand=1

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/54042#comment:27>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform