Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#12942 closed defect (bug) (wontfix)

Change str_replace to preg_replace for $wpdb->prepare function

Reported by: gplaurin Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.9.2
Component: General Keywords: database, prepare, sprintf
Focuses: Cc:


When using a prepare query like $db->prepare("SELECT usr.id FROM tmp_users AS usr WHERE (usr.email = '%1\$s' AND usr.date = %2\$s) OR usr.oemail = '%1\$s'"); wordpress won't properly handle the quote and double quote replacement.

I'm not an expert with regex but I think this should replace the str_replace's that are there: $query = preg_replace('/[\',"]?(%\d?\$?s)[\',"]?/i', "'\$1'", $query);

Change History (6)

comment:1 @nacin6 years ago

$wpdb->prepare adds quotes for you when you use %s. You would just do user.email = %s, not user.email = '%s'. Hence the preparation.

comment:2 @scribu6 years ago

  • Milestone Unassigned deleted
  • Resolution set to invalid
  • Status changed from new to closed

comment:3 @gplaurin6 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened
  • Type changed from enhancement to defect (bug)

That was a prepare example... not a working one. Why having a prepare function with:
$query = str_replace("'%s'", '%s', $query); in case someone mistakenly already singlequoted it
$query = str_replace('"%s"', '%s', $query);
doublequote unquoting
$query = str_replace('%s', "'%s'", $query); quote the strings

and after that using sprintf that can take care of %1$s but not the replaces below. Prepare statement will not add quotes to %1$s. Reclosed this if you want but this is a real bug, sprintf take care of %s and %1$s, prepare statement takes only care of %s but use sprintf.

comment:4 @nacin6 years ago

%1$s only works if you single-quote them. I'd rather add inline docs that say they should not be used, than to account for them.

As it is, the first two str_replace's are for misuse of prepare(). Accounting for %X$s seems to me like a wasteful regexp.

comment:5 @nacin6 years ago

  • Resolution set to wontfix
  • Status changed from reopened to closed
  • Does not support argument numbering/swapping.

Inline docs are already present. Re-closing.

comment:6 @hakre6 years ago

This is more or less a duplicate of #11608 or let's say, that ticket provides code that removes vsprintf / sprintf from prepare and implements its own parser so that such problems as this one are not an issue.

Note: See TracTickets for help on using tickets.