[wp-trac] [WordPress Trac] #25775: WP_Date_Query table prefixing

WordPress Trac noreply at wordpress.org
Tue Oct 7 20:37:22 UTC 2014


#25775: WP_Date_Query table prefixing
-------------------------------------+------------------
 Reporter:  ew_holmes                |       Owner:
     Type:  defect (bug)             |      Status:  new
 Priority:  normal                   |   Milestone:  4.1
Component:  Query                    |     Version:  3.7
 Severity:  major                    |  Resolution:
 Keywords:  has-patch needs-testing  |     Focuses:
-------------------------------------+------------------

Comment (by boonebgorges):

 Replying to [comment:32 Viper007Bond]:
 > Thanks for your work on this! :)
 >
 > One quick question: why set `$dotpos` if you aren't going to use it? Why
 not just do `false === strpos( $column, '.' )` ?

 Whoops - this was a remnant of a different strategy I'd tried. Fixed in
 [attachment:25775.2.patch].

 Viper007Bond and others - I'd like some feedback on the following:

 1. Does the patch deal adequately with the situations listed above? Does
 it resolve the original problem of taxonomy/meta JOINs? Does it maintain
 compatibility with real-world uses of the 'date_query_valid_columns'
 filter that you're familiar with? I think the unit tests show that it does
 all of these things, but a sanity check would be nice.
 2. The one potential downside of my approach is that `validate_column()`
 does a bit less than it did before. It used to be that you could
 circumvent its logic only by filtering 'date_query_valid_columns'. Now you
 can also pass a table-prefixed value. Do we care? Note that we are
 *sanitizing* the value (ie against SQL injection). The "validation" in
 question is merely a developer service: we try to make sure that no one
 attempts to do a date_query against an incompatible column. My personal
 opinion is that it's actually *better* to do less validation, because it
 means that passing a bad column name will result in SQL errors rather than
 mysteriously falling back to the value of 'post_date', which might
 silently return unintended results. So, with my patch, the real purpose of
 the method is not so much validation as it is convenience: it allows you
 to pass merely 'post_date' instead of requiring you to concatenate
 `$wpdb->posts . '.post_date'`.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/25775#comment:33>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list