first argument of wpdb::prepare should have a placeholder

[ounziw]

wpdb::prepare is a function which escapes for SQL.

Some plugins/themes use wpdb::prepare in a wrong way, and they may have a vulerability.

Bad Example
$wpdb->prepare( "SELECT * FROM table WHERE id = $id", null);

I propose to add

		if ( strpos($query,'%') === false || strpos($query,'$') !== false )
			_doing_it_wrong( 'wpdb::prepare', 'first argument of wpdb::prepare() should have a preceholder.', '3.7' );

to check the first argument of wpdb::prepare

[ounziw]

		if ( strpos($query,'%') === false || strpos($query,'$') !== false )
			_doing_it_wrong( 'wpdb::prepare', 'first argument of wpdb::prepare() should have a preceholder.', '3.7' );

it should be

 		if ( strpos($query,'%') === false && strpos($query,'$') !== false )
 			_doing_it_wrong( 'wpdb::prepare', 'first argument of wpdb::prepare() should have a preceholder.', '3.7' );

(If no %, it means no placeholder, and if there is $, it must be a variable which should be escaped)

[nacin]

I like the idea of checking for the complete absence of a placeholder. The check for $ won't work (the variable will already be parsed by then).

[ounziw]

Thanks, Nacin.

I removed the $ check.

 		if ( strpos($query,'%') === false )
 			_doing_it_wrong( 'wpdb::prepare', 'first argument of wpdb::prepare() should have a preceholder.', '3.7' );

[rmccue]

preceholder should be placeholder and first should be capitalised.

[ounziw]

Thanks, rmccue,

I update the patch.
​http://core.trac.wordpress.org/attachment/ticket/25604/check_first_argument_of_wpdb__prepare_has_a_placeholder3.patch

[nacin]

Looking good. This patch needs to be updated a bit. Namely:

• Whitespace inside strpos() parentheses and after the comma to match our coding standards.
• The string should probably be translated by being wrapped in the __() function.
• Braces should be used for this if conditional (recent coding standards change).
• Version number will need to be updated to 3.9.
• __METHOD__ can be used in lieu of 'wpdb::prepare'.

[ounziw]

Thanks for suggestion, nacin.

I updated my patch.

[nacin]

I'd like to review the text of the warning itself but this is otherwise good. Thanks!

[nacin]

In 27073:

Throw an incorrect usage notice when the query argument of wpdb::prepare() does not include a placeholder.

props ounziw.
fixes #25604.

[nacin]

In 27074:

Add a comment for [27073] as someone will inevitably complain it is tricked by % in a string. see #25604.