#39785 closed defect (bug) (wontfix)
posts_selection hook incorrectly concatenates selection query strings
Reported by: | bigevilbrain | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 4.7.2 |
Component: | Query | Keywords: | |
Focuses: | Cc: |
Description
"posts_selection" hook incorrectly concatenates selection query strings. Documentation states this is "For use by caching plugins". Perhaps valid SQL is not need here?
In "wp-includes/class-wp-query.php" in the "get_posts" function:
do_action( 'posts_selection', $where . $groupby . $orderby . $limits . $join );
Missing spaces can result in improper concatenation of the assembled selection query under certain circumstances:
$groupby, $orderby and $limits do not have spaces in front of the values:
Here are values created by a custom query:
where = " AND ( ( wp_postmeta.meta_key = '_pt_event-date' AND CAST(wp_postmeta.meta_value AS DATE) BETWEEN '2017-02-12' AND '2017-03-06' ) ) AND wp_posts.post_type = 'event' AND ((wp_posts.post_status = 'publish'))" groupby = "wp_posts.ID" // <-- No leading space orderby = "wp_posts.post_date DESC" // <-- No leading space limits = "LIMIT 0, 3" // <-- No leading space join = " INNER JOIN wp_postmeta ON ( wp_posts.ID = wp_postmeta.post_id )"
Note this output in the lines below:
wp_posts.IDwp_posts.post_date DESCLIMIT 0, 3
Here is the resulting values announced by the hook:
posts_selection = " AND ( ( wp_postmeta.meta_key = '_pt_event-date' AND CAST(wp_postmeta.meta_value AS DATE) BETWEEN '2017-02-12' AND '2017-03-06' ) ) AND wp_posts.post_type = 'event' AND ((wp_posts.post_status = 'publish'))wp_posts.IDwp_posts.post_date DESCLIMIT 0, 3 INNER JOIN wp_postmeta ON ( wp_posts.ID = wp_postmeta.post_id )"
How to recreate:
This WP_Query acts on a custom post type, however can be see in ALL get_post calls:
<?php $wp_query = null; $wp_query = new WP_Query(); $defaults['post_type'] = 'event'; $defaults['meta_key'] = '_pt_event-date'; $defaults['meta_type'] = 'DATE'; $defaults['meta_value'] = array( date('Y-m-d', time() + (8 * 24 * 60 * 60)), date('Y-m-d', time() + (30 * 24 * 60 * 60)) ); $defaults['meta_compare'] = 'BETWEEN'; $defaults['orderby'] = '_pt_event-location'; // meta_value_num $defaults['order'] = 'DESC'; $defaults['post_status'] = 'publish'; $defaults['posts_per_page'] = 3; $wp_query->query($defaults); // posts_per_page ?>
Change History (4)
#2
follow-up:
↓ 4
@
7 years ago
- Milestone Awaiting Review deleted
- Resolution set to wontfix
- Status changed from new to closed
#3
@
7 years ago
@boonebgorges Yes, I don't see any harm in leaving it as is. I just noticed it because I was debugging some crazy query calls. Thanks for the response!
#4
in reply to:
↑ 2
@
7 years ago
Replying to boonebgorges:
I don't see any meaningful use of it on GitHub, and while I haven't done a proper search of the plugin repo, I'd be quite surprised if anyone's done anything meaningful with it there either.
For reference, here are the results from the plugin repo:
add-headers/add-headers.php advanced-caching/advanced-caching.php bulk-delete/include/util/class-bulk-delete-by-days.php catablog/lib/CataBlog.class.php co-authors-plus/co-authors-plus.php debug-bar-extender/debug-bar-extender.php fulltext-search/includes/wpfts_core.php gts-translation/wordpress/GtsPluginWordpress.php horizontal-meta/controllers/hm_post_meta.php leads-edit-core-email-templates/classes/metaboxes.php light-messages/light_messages.php logged-in/logged-in.php mailbase/library/wp-framework/Base.php mcboards/library/wp-framework/Base.php mingle-users-online/scb/QueryManipulation.php newsletter-artisan/app/network/nodes_network/NodesNetwork.php newsletter-artisan/app/newsletter_controller/NewsletterController.php newsletter-artisan/app/posts_selection_management/PostsSelectionManagement.php picasa-albums/1.0.5/scb/QueryManipulation.php picasa-albums/scb/QueryManipulation.php presstest/ptests/samples/query.php pricing-table-dynamite/custom-post-type.php push-syndication/push-syndication.php qtranslate-x/qtranslate_hooks.php revisionary/hardway/hardway-admin_non-administrator_rvy.php search-filter/search-filter.php seo-rets/seo-rets.php sphinx-search-related-omnibus/sphinxOmnibusIntegration.php templ33t/templ33t.php woo-advanced-discounts/includes/class-wad.php woocommerce-checkout-password-strength-meter/woo-checkout-password-strength-meter.php wp-autoloader/lib/WPExtend/BasicWPActions.php wp-e-commerce/wpsc-includes/customer-private.php wp-waf/wp-waf.php wptuner/wptuner.php wpzintext/wpZintext.php
@bigevilbrain Thanks for the ticket, and welcome to WordPress Trac!
Very strange - I have seen this action before, but never noticed the way it concatenated query fragments. The hook was introduced in [5916] #4799 and hasn't been touched since. I don't see any meaningful use of it on GitHub, and while I haven't done a proper search of the plugin repo, I'd be quite surprised if anyone's done anything meaningful with it there either.
That said, it's harmless. And changing the data passed to the action could potentially break someone who *is* managing to use it in the right way. So I think we should probably leave it as is.