Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#39785 closed defect (bug) (wontfix)

posts_selection hook incorrectly concatenates selection query strings

Reported by: bigevilbrain's profile 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)

#1 @SergeyBiryukov
7 years ago

  • Component changed from Posts, Post Types to Query

#2 follow-up: @boonebgorges
7 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

@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.

#3 @bigevilbrain
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 @SergeyBiryukov
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
Note: See TracTickets for help on using tickets.