WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#7423 closed defect (bug) (fixed)

in WP_Query, sanitization breaks usage of multiple post_status values

Reported by: markjaquith Owned by: markjaquith
Milestone: 2.6.1 Priority: high
Severity: normal Version: 2.6
Component: General Keywords:
Focuses: Cc:

Description (last modified by markjaquith)

I was trying to get WP_Query to give me all draft OR future posts by other authors, but found that it would not accept comma-separated post_status values. Turns out we're using sanitize_user() on the query values of post_status which breaks that functionality. We need to allow commas so that WP_Query can process post_status=draft,future

Switching that sanitize_user() line to a [^a-z0-9,_-] preg_replace() sanitization line allowed my WP_Query call to work as intended.

We're also doing that sanitization on post_type, but it doesn't look like the functionality to accept multiple post_type values is there (and it's certainly less useful than accepting multiple post_statuses).

Attachments (1)

7423.001.diff (541 bytes) - added by markjaquith 7 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 @markjaquith7 years ago

  • Description modified (diff)
  • Summary changed from in WP_Query, sanitization breaks usage of multiple post_type or post_status values to in WP_Query, sanitization breaks usage of multiple post_status values

@markjaquith7 years ago

comment:2 @markjaquith7 years ago

  • Owner changed from anonymous to markjaquith
  • Status changed from new to assigned

Patch uploaded.

Would also like to get this into 2.6.1

comment:3 follow-up: @azaozz7 years ago

Looks good, just one question: post_status can be published, draft, pending, future, etc. and post_type can be post, page or attachment. Shouldn't we sanitize with

preg_replace('|[^a-z,]|', '', $qv['post_status']); 

That may affect some plugins, but don't think plugins are allowed to change these. Otherwise will have to use exactly the same regexp as sanitize_user

preg_replace('|[^a-z0-9 _.\-@]|i', '', $username);

so nothing breaks.

comment:4 in reply to: ↑ 3 ; follow-up: @filosofo7 years ago

Replying to azaozz:

That may affect some plugins, but don't think plugins are allowed to change these. Otherwise will have to use exactly the same regexp as sanitize_user

Actually, I use a custom post_type with some regularity, and the API makes it easy to do with set_post_type() and get_post_type().

My suggestion is that you do what set_post_type() does and use sanitize_post_field() to sanitize the values in the 'db' context. It applies the 'pre_post_type' filter callbacks, which by default is sanitize_user.

comment:5 @DD327 years ago

I've found that using a custom post_type w/ WP_Reqwrite & using p=0123 syntax can cause anoyances rather simply, Which stems from http://trac.wordpress.org/browser/trunk/wp-includes/query.php#L1287 specifically setting the post_type if it thinks the request is for one of those, It might be worth noting that for this ticket, to get it to trust the passed post_type if need be?

comment:6 in reply to: ↑ 4 @azaozz7 years ago

Replying to filosofo:

Actually, I use a custom post_type with some regularity, and the API makes it easy to do with set_post_type() and get_post_type().

I was commenting about the post_status not post_type, sorry for any misunderstandings.

As you mention, post_type can be customized and the new type is filtered with sanitize_user(). So the new type can include number, space, underscore, dot, dash, @ and must start with a letter. Perhaps it should be recommended that the new type follows the existing format: one word, all lower case ascii.

DD32, it seems that would affect other functions too. Perhaps make a patch and open new ticket if it needs to be changed, so it can be tracked properly.

comment:7 @azaozz7 years ago

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

(In [8504]) Allow comma-separated post_status values in WP_Query. Props markjaquith, fixes #7423 for 2.7

comment:8 @azaozz7 years ago

  • Milestone changed from 2.7 to 2.6.1
  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-open for 2.6.1

comment:9 @azaozz7 years ago

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

(In [8505]) Allow comma-separated post_status values in WP_Query. Props markjaquith, fixes #7423 for 2.6.1

Note: See TracTickets for help on using tickets.