WordPress-Coding-Standards
WordPress-Coding-Standards copied to clipboard
PreparedSQLPlaceholders.UnquotedComplexPlaceholder: allow for tablenames in backticks
Is your feature request related to a problem?
Given the following code snippet:
$wpdb->query(
$wpdb->prepare(
'TRUNCATE TABLE `%1$s`',
plugin_get_table_name( 'Name' )
)
);
WPCS will currently throw the following warning:
WARNING | Complex placeholders used for values in the query string in $wpdb->prepare() will
NOT be quoted automagically. Found: %1$s.
(WordPress.DB.PreparedSQLPlaceholders.UnquotedComplexPlaceholder)
Describe the solution you'd like
In this particular case, I believe the warning should not be thrown. Instead the sniff should recognize the backticks as valid "quotes" for a table name.
Note: this should only be accepted for table names, so, the sniff should probably look for TABLE or FROM before the placeholder.
Some research may need to be done into the various SQL syntaxes to make sure that the sniff recognizes the correct keywords and doesn't miss any real unquoted placeholders.
I think these should be allowed for column names as well.
@mundschenk-at Good point. Agreed.
I think these should be allowed for column names as well.
Yes, I totally agree with you. There should be a way to dynamically pass table name and column names otherwise it will just increase redundancy in code in CRUD operations for custom tables.
Some research may need to be done into the various SQL syntaxes to make sure that the sniff recognizes the correct keywords and doesn't miss any real unquoted placeholders.
Just a quick note: Using backticks to quote identifiers is a MySQLism. I seem to remember some efforts to make WordPress use alternative DB backends, but since even the old Codex page asserts that doing so would be, ah, difficult, I assume that is not a relevant concern.
@mundschenk-at Good point, but not something we should be concerned about until WP would support other DB backends. It does support MariaDB AFAIK, but that's a fork of MySQL, so syntax-wise they are compatible IIRC.
Yes, for all practical purposes (as far as WordPress is concerned), MariaDB is MySQL.
In WordPress 6.1 (October 2022), this won't be needed, as the %i placeholder has been added for Identifiers (table/field names):
$wpdb->prepare(
'TRUNCATE TABLE %i',
plugin_get_table_name( 'Name' )
);
This also avoids the issue where a developer may accidentally forget to add backtick quotes, or allow the user (attacker) to provide a value that contains backticks (which would need to escaped correctly), e.g.
$wpdb->prepare( 'WHERE `%1$s` IS NULL', $_GET['field'] ); // INSECURE, e.g. "WHERE `a`b` IS NULL"
Unfortunately this won't solve the problem for those still supporting 6.0 and below, so there might still be a case for ignoring this warning.