Possible invalid uses of wpdb::prepare()

[xknown]

One common error I see about wpdb::prepare() is that many developers tend to use it by passing only a SQL query, which may produce bugs due to the implementation of this method.

As you know, wpdb::prepare() does, generally speaking, a string replacement of the placeholders (%s, %d, etc) by using sprintf/vsprintf under the scenes. So, if one passes a SQL query with placeholders and no other parameters, then this method returns a blank string. For example:

$query = $wpdb->prepare( 'select * from table where column = %s', $user_input );

$result_set = $wpdb->get_results( $wpdb->prepare( $query ) );

If $user_input contains a placeholder (i.e. "hola%s mundo"), the query will not be executed.

I used a simple static code analyzer to detect this calls on the core and found two instances.

• ​http://core.trac.wordpress.org/browser/trunk/wp-includes/query.php#L2409
• ​http://core.trac.wordpress.org/browser/trunk/wp-includes/ms-functions.php#L1922

We should call _doing_it_wrong() if wpdb::prepare() receives only one parameter.

[nacin]

I agree on all counts.

What if we declared prepare() with a second argument, automatically causing E_WARNINGs? Might not be the prettiest for sites that show errors. (Maybe after having notices there for a few releases?)

[xknown]

Replying to nacin:

I agree on all counts.

What if we declared prepare() with a second argument, automatically causing E_WARNINGs? Might not be the prettiest for sites that show errors. (Maybe after having notices there for a few releases?)

I think is okay to add another required parameter. However, we can also remove the @ of @vsprintf( $query, $args ) in prepare(), which is the one that hides the PHP warning produced by this invalid uses.

[nacin]

Too few parameters also triggers a warning: vsprintf( '%s %s', array( 1 ) ).

[miqrogroove]

In wp-db.php with a second parameter required, the call to isset() inside of prepare() is no longer required. Removing it is an option, although it would cause a second PHP debug message when called with only one param.

[nacin]

In 22428:

Remove single-argument calls to wpdb:prepare(), which are invalid as nothing is being prepared. see #22262.

[nacin]

In 22429:

Enforce a minimum of two arguments for wpdb::prepare(). The first argument is the query (or fragment thereof), which is required. Additional arguments are values to substitute into placeholders.

This will generate E_WARNINGs for insufficient arguments when prepare() is called with no additional arguments. This should discourage improper uses of prepare() under the guise of safely running a query.

props xknown. fixes #22262.

[Otto42]

Looks like somebody didn't tell Akismet. In Akismet admin.php:

$waiting = $wpdb->get_var( $wpdb->prepare( "SELECT COUNT(*) FROM $wpdb->commentmeta WHERE meta_key = 'akismet_error'" ) );