[wp-trac] [WordPress Trac] #56091: Use %i for table/field names in wpdb::prepare()

WordPress Trac noreply at wordpress.org
Tue Jul 26 15:36:04 UTC 2022


#56091: Use %i for table/field names in wpdb::prepare()
--------------------------+---------------------------
 Reporter:  craigfrancis  |       Owner:  craigfrancis
     Type:  enhancement   |      Status:  assigned
 Priority:  low           |   Milestone:  6.1
Component:  Database      |     Version:  trunk
 Severity:  minor         |  Resolution:
 Keywords:  has-patch     |     Focuses:
--------------------------+---------------------------

Comment (by craigfrancis):

 First patch: Parameterise some table names.

 It will help Static Analysis / Sniffs, especially when `IN(%...d)` is
 supported (via #54042), as makes it easy to check ''all'' variables are
 escaped correctly.

 But I found some of these changes make the SQL a bit harder to read, as it
 can take a second to work out which table is being used (maybe that will
 get easier once I'm used to it?)

 This patch only affects a few queries. I do have more, where I've been
 able to get most of WordPress working with the `$table_prefix` set to `'wp
 '` (with a space), but some changes get a bit complicated, and I doubt
 many (any?) developers need a `$table_prefix` that contains characters
 other than `[a-zA-Z0-9_]`.

 ----

 A different approach, specifically for `$table_prefix`, would use the
 `literal-string` type (supported in
 [https://github.com/vimeo/psalm/releases/tag/4.8.0 Psalm] and
 [https://github.com/phpstan/phpstan/releases/tag/0.12.97 PHPStan]). This
 ensures a property/variable only contains a "developer defined string",
 and it supports concatenation.

 This means properties like [https://github.com/WordPress/wordpress-
 develop/blob/e2a45c0d8a16d9229244c15357ab3b84dd8ff3ed/src/wp-includes
 /class-wpdb.php#L391 $wpdb->posts] could use this type, and continue to be
 concatenated directly into the SQL string without escaping (ish). Then
 static analsysis tools (that trace variables from source to sink) could
 check the first argument to `wpdb::prepare()` is still a `literal-string`.
 Admittedly this won't work for variables that cannot be defined as a
 `literal-string`, or when they contain any
 [https://dev.mysql.com/doc/refman/8.0/en/identifiers.html non-permitted
 characters], or any of the
 [https://dev.mysql.com/doc/refman/8.0/en/keywords.html ~286 reserved
 words] (which is why we need `%i`).

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


More information about the wp-trac mailing list