Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#56933 closed defect (bug) (fixed)

Unexpected quotes around search text in custom LIKE queries

Reported by: alanp57's profile AlanP57 Owned by: hellofromtonya's profile hellofromTonya
Milestone: 6.1 Priority: normal
Severity: normal Version: 6.1
Component: Database Keywords: has-testing-info commit has-patch has-unit-tests dev-reviewed
Focuses: Cc:

Description (last modified by hellofromTonya)

I'm experiencing a similar issue and have just tested a like query with WP 6.1-RC5. The result is an SQL error and the output from the prepare function is like '%'my search text'%'. Notice, there is an extra set of quote marks around the search text.

In some cases, I prefer not to use WP_Query when displaying data. This is the code from the plugin:

<?php
      $sql = $wpdb->prepare("(select $wpdb->posts.ID, post_title, {$wpdb->prefix}mgmlp_folders.folder_id, pm.meta_value as attached_file, 'a' as item_type 
from $wpdb->posts
LEFT JOIN {$wpdb->prefix}mgmlp_folders ON($wpdb->posts.ID = {$wpdb->prefix}mgmlp_folders.post_id)
LEFT JOIN $wpdb->postmeta AS pm ON (pm.post_id = $wpdb->posts.ID)
LEFT JOIN $wpdb->users AS us ON ($wpdb->posts.post_author = us.ID) 
where post_type = 'mgmlp_media_folder' and pm.meta_key = '_wp_attached_file' and SUBSTRING_INDEX(pm.meta_value, '/', -1) like '%%%s%%')
union all
(select $wpdb->posts.ID, post_title, {$wpdb->prefix}mgmlp_folders.folder_id, pm.meta_value as attached_file, 'b' as item_type
from $wpdb->posts 
LEFT JOIN {$wpdb->prefix}mgmlp_folders ON( $wpdb->posts.ID = {$wpdb->prefix}mgmlp_folders.post_id) 
LEFT JOIN $wpdb->postmeta AS pm ON (pm.post_id = $wpdb->posts.ID) 
LEFT JOIN $wpdb->users AS us ON ($wpdb->posts.post_author = us.ID) 
where post_type = 'attachment' and pm.meta_key = '_wp_attached_file' and SUBSTRING_INDEX(pm.meta_value, '/', -1) like '%%%s%%') order by attached_file", $search_string, $search_string);

And here is an example of the SQL:

(select wp_posts.ID, post_title, wp_mgmlp_folders.folder_id, pm.meta_value as attached_file, 'a' as item_type 
from wp_posts
LEFT JOIN wp_mgmlp_folders ON(wp_posts.ID = wp_mgmlp_folders.post_id)
LEFT JOIN wp_postmeta AS pm ON (pm.post_id = wp_posts.ID)
LEFT JOIN wp_users AS us ON (wp_posts.post_author = us.ID) 
where post_type = 'mgmlp_media_folder' and pm.meta_key = '_wp_attached_file' and SUBSTRING_INDEX(pm.meta_value, '/', -1) like '{48bf6209debff2ee81208ffaee83c0ccfe32af6953d915a72a2fd46f1d0be2e1}'my search text'{48bf6209debff2ee81208ffaee83c0ccfe32af6953d915a72a2fd46f1d0be2e1}')
union all
(select wp_posts.ID, post_title, wp_mgmlp_folders.folder_id, pm.meta_value as attached_file, 'b' as item_type
from wp_posts 
LEFT JOIN wp_mgmlp_folders ON( wp_posts.ID = wp_mgmlp_folders.post_id) 
LEFT JOIN wp_postmeta AS pm ON (pm.post_id = wp_posts.ID) 
LEFT JOIN wp_users AS us ON (wp_posts.post_author = us.ID) 
where post_type = 'attachment' and pm.meta_key = '_wp_attached_file' and SUBSTRING_INDEX(pm.meta_value, '/', -1) like '{48bf6209debff2ee81208ffaee83c0ccfe32af6953d915a72a2fd46f1d0be2e1}'my search text'{48bf6209debff2ee81208ffaee83c0ccfe32af6953d915a72a2fd46f1d0be2e1}') order by attached_file

In WordPress 6.0.3 my SQL query works

This worked on WP 6.0.3, but doesn't on 6.1-RC5.

Original support topic: https://wordpress.org/support/topic/prepare-function-removes-percent-signs-from-like-sql-statement/.

Attachments (2)

56933.diff (17.4 KB) - added by SergeyBiryukov 2 years ago.
56933-props.png (269.4 KB) - added by SergeyBiryukov 2 years ago.

Download all attachments as: .zip

Change History (25)

#1 @desrosj
2 years ago

Some notes, I have gone and made this ticket on behalf of @AlanP57 for the following reasons:

  • It's not completely clear if this is being caused by #56802 (Post IDs cached for search and other 'LIKE' queries are unreachable) or #52506 (Add escaping method for table names in SQL queries).
  • 6.1 is due out in ~24 hours.
  • The other tickets are closed and completed on the 6.1 milestone and they should remain there if those changes ship in the release.
  • This separate ticket allows it to be milestoned and triaged appropriately as more information is discovered.
  • This seems to be an edge case from custom code and not one from using WP_Query directly.

This ticket was mentioned in Slack in #core by desrosj. View the logs.


2 years ago

#3 @hellofromTonya
2 years ago

  • Description modified (diff)

#4 @hellofromTonya
2 years ago

  • Description modified (diff)
  • Keywords needs-testing needs-unit-tests removed

#5 @hellofromTonya
2 years ago

Using a fresh install of WP 6.1-RC5 with the sample code hooked into 'loop_start', I can reproduce the reported results.

#6 @sethta
2 years ago

We are also getting the same issue on our plugin in the WP repo (mediavine-create).

Because we didn't discover/confirm the issue until Friday, and this affects most of our SQL queries for the plugin, we know we can't QA a release to go out before WP 6.1 is released.

The update to 6.1 breaks the ability to add or edit any custom content as we are using custom tables and have some LIKE queries that take place before we save to the database. Our solution is less than ideal as we have to adjust the prepare placeholder so it's just a string placeholder without any added %%, and then change the value to be %%$value%%.

We have our QA team checking functionality right now so we can release soon, but we are currently advising publishers using Create to not update to 6.1 until after we have pushed out a release.

This ticket was mentioned in Slack in #core by bernhardreiter. View the logs.


2 years ago

#8 @desrosj
2 years ago

As an update here. It was decided that the changes from #52506 will be reverted and an RC6 will be made available at some point in the next 6-8 hours.

This should address all of the problems described here, but testing and confirmation would be greatly appreciated.

This ticket was mentioned in Slack in #core by desrosj. View the logs.


2 years ago

#10 @AlanP57
2 years ago

If is noted here when the RC is released, then I will download and test it.

@SergeyBiryukov
2 years ago

#11 @hellofromTonya
2 years ago

  • Keywords has-testing-info added

PR 3541 is the revert and added test from 56933.diff.

#12 @hellofromTonya
2 years ago

  • Keywords commit has-patch has-unit-tests added

PR 3541 is now ready for commit.

#13 @hellofromTonya
2 years ago

  • Owner set to hellofromTonya
  • Resolution set to fixed
  • Status changed from new to closed

In 54733:

Database: Revert [53575].

When using '%%%s%%' pattern with $wpdb->prepare(), it works on 6.0.3 but does not on 6.1-RC. Why? The inserted value is wrapped in quotes on 6.1-RC5 whereas it is not on <= 6.0.3.

With 6.1 final release tomorrow, more time is needed to further investigate and test. Reverting this changeset to restore the previous behavior.

This commit also adds a dataset for testing the '%%%s%%' pattern.

Props SergeyBiryukov, hellofromTonya, bernhard-reiter, desrosj, davidbaumwald, jorbin.

Fixes #56933.
See #52506.

#14 @hellofromTonya
2 years ago

  • Keywords dev-feedback added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-opening for backport to 6.1

#15 @SergeyBiryukov
2 years ago

  • Keywords dev-reviewed added; dev-feedback removed

[54733] looks good to backport.

#16 @SergeyBiryukov
2 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 54734:

Database: Revert [53575].

When using '%%%s%%' pattern with $wpdb->prepare(), it works on 6.0.3 but does not on 6.1-RC. Why? The inserted value is wrapped in quotes on 6.1-RC5 whereas it is not on <= 6.0.3.

With 6.1 final release tomorrow, more time is needed to further investigate and test. Reverting this changeset to restore the previous behavior.

This commit also adds a dataset for testing the '%%%s%%' pattern.

Props SergeyBiryukov, hellofromTonya, bernhard-reiter, desrosj, davidbaumwald, jorbin.
Reviewed by hellofromTonya, SergeyBiryukov.
Merges [54733] to the 6.1 branch.
Fixes #56933.
See #52506.

#17 @hellofromTonya
2 years ago

@AlanP57 Thank you for reporting this issue. The changeset has been reverted on trunk and the 6.1 branch and will be bundled 6.1-RC6 for testing. I manually tested and the previous behavior is restored (test report coming).

In prepping the commit message, I overlooked including you in the props list as I was focused on the conversations that happened today in Slack. I apologize for not including you in the commit message. However, @SergeyBiryukov has ensured you do get a prop (see props pic).

Last edited 2 years ago by hellofromTonya (previous) (diff)

#18 @SergeyBiryukov
2 years ago

It looks like some props were missed in the commits here. Sorry for that!

I have updated the props list for [54733] and [54734] in the Core Props tool on make/core to include the props for @AlanP57 and @sethta, as well as @threadi for pointing to a relevant ticket on support forums: 56933-props.png.

Version 2, edited 2 years ago by SergeyBiryukov (previous) (next) (diff)

#19 @hellofromTonya
2 years ago

Test Report

Env

Test Instructions

Note: The hex codes will be different in your testing.

  • In your test site, create a mu-plugins folder inside of wp-content.
  • Copy and paste this file into that new folder. This code is the sample code provided in the ticket's description but made into a must-use plugin for testing purposes.
  • Using WordPress 6.0.3, open the site in the front-end and note what is rendered on the screen:
(select wp_posts.ID, post_title, wp_mgmlp_folders.folder_id, pm.meta_value as attached_file, 'a' as item_type 
from wp_posts
LEFT JOIN wp_mgmlp_folders ON(wp_posts.ID = wp_mgmlp_folders.post_id)
LEFT JOIN wp_postmeta AS pm ON (pm.post_id = wp_posts.ID)
LEFT JOIN wp_users AS us ON (wp_posts.post_author = us.ID) 
where post_type = 'mgmlp_media_folder' and pm.meta_key = '_wp_attached_file' and SUBSTRING_INDEX(pm.meta_value, '/', -1) like '{ec3ef6eec1ef785abece889c0e9046e9297b06b512e407f3f726977705f81079}hello{ec3ef6eec1ef785abece889c0e9046e9297b06b512e407f3f726977705f81079}')
union all
(select wp_posts.ID, post_title, wp_mgmlp_folders.folder_id, pm.meta_value as attached_file, 'b' as item_type
from wp_posts 
LEFT JOIN wp_mgmlp_folders ON( wp_posts.ID = wp_mgmlp_folders.post_id) 
LEFT JOIN wp_postmeta AS pm ON (pm.post_id = wp_posts.ID) 
LEFT JOIN wp_users AS us ON (wp_posts.post_author = us.ID) 
where post_type = 'attachment' and pm.meta_key = '_wp_attached_file' and SUBSTRING_INDEX(pm.meta_value, '/', -1) like '{ec3ef6eec1ef785abece889c0e9046e9297b06b512e407f3f726977705f81079}hello{ec3ef6eec1ef785abece889c0e9046e9297b06b512e407f3f726977705f81079}') order by attached_file

The hex codes will be different. Note that the hello inserted between the hex codes is not wrapped in single quotation marks }hello{.

  • Switch to 6.1-RC5.
  • Refresh the home page on the front-end. Note that the hello string is wrapped in single quotation marks }'hello'{ 🐞
(select wp_posts.ID, post_title, wp_mgmlp_folders.folder_id, pm.meta_value as attached_file, 'a' as item_type 
from wp_posts
LEFT JOIN wp_mgmlp_folders ON(wp_posts.ID = wp_mgmlp_folders.post_id)
LEFT JOIN wp_postmeta AS pm ON (pm.post_id = wp_posts.ID)
LEFT JOIN wp_users AS us ON (wp_posts.post_author = us.ID) 
where post_type = 'mgmlp_media_folder' and pm.meta_key = '_wp_attached_file' and SUBSTRING_INDEX(pm.meta_value, '/', -1) like '{b11657509e9e1a989804104976342f19d25c708d637da20f8beca38a8de081be}'hello'{b11657509e9e1a989804104976342f19d25c708d637da20f8beca38a8de081be}')
union all
(select wp_posts.ID, post_title, wp_mgmlp_folders.folder_id, pm.meta_value as attached_file, 'b' as item_type
from wp_posts 
LEFT JOIN wp_mgmlp_folders ON( wp_posts.ID = wp_mgmlp_folders.post_id) 
LEFT JOIN wp_postmeta AS pm ON (pm.post_id = wp_posts.ID) 
LEFT JOIN wp_users AS us ON (wp_posts.post_author = us.ID) 
where post_type = 'attachment' and pm.meta_key = '_wp_attached_file' and SUBSTRING_INDEX(pm.meta_value, '/', -1) like '{b11657509e9e1a989804104976342f19d25c708d637da20f8beca38a8de081be}'hello'{b11657509e9e1a989804104976342f19d25c708d637da20f8beca38a8de081be}') order by attached_file
  • Switch to the current 6.1 branch (which has the revert).
  • Refresh the home page on the front-end. Note the hello is no longer wrapped in single quotation marks (}hello{) which is the same behavior in 6.0.3.
(select wp_posts.ID, post_title, wp_mgmlp_folders.folder_id, pm.meta_value as attached_file, 'a' as item_type 
from wp_posts
LEFT JOIN wp_mgmlp_folders ON(wp_posts.ID = wp_mgmlp_folders.post_id)
LEFT JOIN wp_postmeta AS pm ON (pm.post_id = wp_posts.ID)
LEFT JOIN wp_users AS us ON (wp_posts.post_author = us.ID) 
where post_type = 'mgmlp_media_folder' and pm.meta_key = '_wp_attached_file' and SUBSTRING_INDEX(pm.meta_value, '/', -1) like '{5322ec299a9c19e0eef425bd8c57a795cdf1cde3421a119c904b36219a4d6651}hello{5322ec299a9c19e0eef425bd8c57a795cdf1cde3421a119c904b36219a4d6651}')
union all
(select wp_posts.ID, post_title, wp_mgmlp_folders.folder_id, pm.meta_value as attached_file, 'b' as item_type
from wp_posts 
LEFT JOIN wp_mgmlp_folders ON( wp_posts.ID = wp_mgmlp_folders.post_id) 
LEFT JOIN wp_postmeta AS pm ON (pm.post_id = wp_posts.ID) 
LEFT JOIN wp_users AS us ON (wp_posts.post_author = us.ID) 
where post_type = 'attachment' and pm.meta_key = '_wp_attached_file' and SUBSTRING_INDEX(pm.meta_value, '/', -1) like '{5322ec299a9c19e0eef425bd8c57a795cdf1cde3421a119c904b36219a4d6651}hello{5322ec299a9c19e0eef425bd8c57a795cdf1cde3421a119c904b36219a4d6651}') order by attached_file

Test Results

  • Able to reproduce the reported issue 🐞 ✅
  • Confirmed the current version of 6.1-branch with the revert PR restores the previous behavior ✅

#20 follow-up: @AlanP57
2 years ago

I can verify that the like query works now in 6.1 RC6.

#21 in reply to: ↑ 20 @hellofromTonya
2 years ago

Replying to AlanP57:

I can verify that the like query works now in 6.1 RC6.

That's very good news. Thank you @AlanP57 for retesting

#22 @sethta
2 years ago

I also can verify that the LIKE queries in mediavine-create work properly in 6.1 RC 6. Thank you.

#23 @craigfrancis
2 years ago

I've created a new PR to support %i - 3724.

It supports '%%%s%%', '%%%s', etc; but this undocumented feature will ideally be removed in the future.

Please note the prepare documentation says "percentage wildcards [...] cannot be inserted directly in the query string". The original example should use "LIKE %s" and the value provided as either:

  • '%' . $search_string . '%'
  • '%' . $wpdb->esc_like( $search_string ) . '%'.

Ideally wpdb should quote all user values, as we cannot rely on developers never making a mistake (as this often introduces an Injection Vulnerability).

Note: See TracTickets for help on using tickets.