Make WordPress Core

Opened 11 years ago

Closed 10 years ago

#26896 closed defect (bug) (wontfix)

Quotes not added around strings using wpdb->prepare with sprintf position specifiers

Reported by: neonwired's profile neonWired Owned by:
Milestone: Priority: normal
Severity: minor Version: 3.2.1
Component: Database Keywords:
Focuses: Cc:


The docs state that 'you do not have to worry about quoting strings' when using prepare however this is not the case if you use position specifiers.

For example:
$wpdb->prepare('INSERT INTO table (col1, col2, col3) VALUES (%s, %s, $d)', $email, $hash, $post_id);

will quote the strings, however using position specifiers like the below quotes are not added cause a mysql error.

$wpdb->prepare('INSERT INTO table (col1, col2, col3) VALUES (%1$s, %2$s, $3$d)', $email, $hash, $post_id);

If this is not a bug then the docs need updating to reflect this behavior.

Change History (3)

#1 @johnbillion
11 years ago

  • Keywords close needs-docs added

Thanks for the report!

Position specifiers aren't supported by WPDB's prepare() method. It doesn't use sprintf() under the hood like you may well expect.

This ticket is therefore invalid, however I'll leave this open for now as we may need to clarify the documentation.

#2 @neonWired
10 years ago

it uses vsprintf (wp-includes/wp-db.php line 890) which is the same as sprintf only uses an array of args.

it uses the following 3 lines (886, 887 & 888) to add the quotes but obviously if you use position specifiers this will no longer work

$query = str_replace( "'%s'", '%s', $query ); in case someone mistakenly already singlequoted it
$query = str_replace( '"%s"', '%s', $query );
doublequote unquoting
$query = preg_replace( '|(?<!%)%s|', "'%s'", $query ); quote the strings, avoiding escaped strings like %%s

it just needs the regex pattern updating to support position specifier, i don't think it's a huge fix for the benefit and for the consistency.

Not sure if this should be reclassified as an enhancement?

Last edited 10 years ago by neonWired (previous) (diff)

#3 @pento
10 years ago

  • Keywords close needs-docs removed
  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Hi neonWired!

Thanks for the bug report! As mentioned in the developer reference, prepare() doesn't support argument numbering. While I understand it's partially available due to our use of vsprintf(), we don't plan on completing support for it.

Note: See TracTickets for help on using tickets.