Improper handling of 'post_type[]=' like parameters

[loushou]

PLEASE READ WHOLE BUG: because it includes reasons for severity also

THE PROBLEM:

So I have a problem.... obviously, right? So if I were to do something like:

get_posts('post_type[]=post_type_1&post_type[]=post_type_2');

I would receive a list of posts that are either of post type 'post_type_1' or of 'post_type_2'. Now if I goto a url like the following:

http://www.my-site.com/index.php?post_type[]=post_type_1&post_type[]=post_type_2&name=testing-post

I should get a very very similar result, but instead, I get a 404 error page.

How can I make the later example work? I have been browsing the WP base code (like always) and found the following line in classes.php on line 277:

$this->query_vars[$wpvar] = (string) $this->query_vars[$wpvar];

Most probably don't know what this line actually means, but in short it converts all the query vars (everything after the ? in a url) to strings, even those that should be arrays (since the WP_Query object supports arrays). So instead my slightly parsed url winds up looking like:

http://www.my-site.com/index.php?post_type=Array&name=testing-post

because in PHP when performing a string type cast on an array, you get a string with the word 'Array':

$myArray = array(
  'some value',
  'another un-important value'
);
echo (string) $myArray;
// prints out 'Array'

Then later in classes.php on line 289, we have this:

// Limit publicly queried post_types to those that are publicly_queryable
    if ( isset( $this->query_vars['post_type']) ) {
      $queryable_post_types =  get_post_types( array('publicly_queryable' => true) );
      if ( ! in_array( $this->query_vars['post_type'], $queryable_post_types ) )
        unset( $this->query_vars['post_type'] );
    }

which does what it says, eliminates my now post_type=Array from the query completely because 'Array' is not only not a proper post type, but it is not 'publicly_queryable'.

So, again I ask, how do I get a url like:

http://www.my-site.com/index.php?post_type[]=post_type_1&post_type[]=post_type_2&name=testing-post

to work, assuming I do not know which post type it is?

p.s. ~ for the really high-level programmers, it is actually a little more in depth than this, but this is the basic idea of the problem, and works exactly the same way as the actual problem. for a glimpse into the big picture problem, think about how WP interprets clean urls to determine the query used to pull the list of posts to display on the current page.

IMPORTANCE VALUE:

This is marked 'major' because on of the key features of WP3.0 was better handling of custom post types. With this problem, all translations of the url where multiple post types are being queried will fail. Not only is this a post_type problem, but above should clearly explain that any query variable that the WP_Query class normally accepts in the form of an array, will be effected. This means that now translated urls like this:

http://www.my-site.com/index.php?post_status[]=publish&post_status[]=new_custom_status&post_status[]=inherit

will also not work.

So because the scope of this problem directly conflicts with the awesome, much anticipated new features of WP, on a large scale, it is very important that it gets solved quickly.

[loushou]

Patch

[loushou]

Patch

[loushou]

Taking over the ticket

[loushou]

So I made a patch. Then I was looking at it an I found I could simplify it. So the attached file 'better-url-array-fix.patch' is the condensed one. I tested them against my install, and it performed exactly how I described it should in the bug, without breaking any of my site. However I would like to add that my site is pretty small with only BuddyPress as a plugin. This does need to be tested on bigger sites.

PATCH BREAKDOWN

Prior to casting the query variable to a string, we need to check if it is an array (since WP_Query accepts arrays as parameter values). If it is, instead of casting the entire variable to a string, we need to cycle through the array, and cast each value of the array to a string. Every other type gets processed exactly as it did before.

Then for the 'post_type' query var specifically (since it has it's own independent logic a little lower in the code), we need to modify the logic specifically targeting it. Again we check if the value of $this->query_varspost_type? is an array. If it is, we need to compare each element in the list to the publicly queryable post_types. Once again if the value is not an array, then it is processed exactly as before.

Loushou

[scribu]

A ticket isn't fixed until the patch is commited. Please read ​http://codex.wordpress.org/Reporting_Bugs

[scribu]

Note that foreach ($this->query_vars[$wpvar] as &$v) won't work in PHP4 (which still needs to be supported until 2011).

[scribu]

Anyway, I agree that it's a problem that needs to be solved.

[loushou]

Sorry about the premature closing. I must have mis-read the Reporting_Bugs page, because now looking at it, it does say that. As far as PHP4 compatibility, I flat out forgot about keeping that in mind, as for the last 4 years (at least) have worked on PHP5. I do understand the support requirement though and I can make a new patch pretty quickly that will be compatible. Finally, thank you for not only understanding the problem I proposed, but also supporting it. It does mean a lot.

[loushou]

Patch

[loushou]

This patch removes all 'references' in foreach loops, since they were not available in PHP4. Instead it uses the key-value method to replace the values with string casted values. Also removed the references in the foreach loop in the second section for the patch that dealt specifically with the post_type query variable.

Thanks for the notes again. :)

[scribu]

Please add spaces to the insides of language constructs and function calls, as shown in ​http://codex.wordpress.org/WordPress_Coding_Standards

[loushou]

Sure thing :). Thank you for being patient with me. This is my first ticket in Trac. I never make the same mistake twice. New patch soon.

[scribu]

Also, instead of

foreach ($this->query_vars['post_type'] as $key => $pType) 
    if ( ! in_array( $pType, $queryable_post_types ) )
        unset( $this->query_vars['post_type'][$key] );

it would be more elegant to use array_intersect:

$this->query_vars['post_type'] = array_intersect( $this->query_vars['post_type'], $queryable_post_types );

[loushou]

Patch with coding standards and php4 ompatibility

[scribu]

PS: Trac will automatically rename attachments with the same name like so:

• array-fix.diff
• array-fix.2.diff
• array-fix.3.diff
• etc.

PPS: instead of the non-helpful 'patch', you might as well leave the description field blank.

[scribu]

What I'm trying to say is that you should use the description field to describe your patch, instead of the attachment name.

[loushou]

Patch with brilliance

[loushou]

Properly named patch that accounts for array values in url queries, instead of destroying them, compiant with PHP4 and the coding standards, with the elegant code suggested in an earlier post

[loushou]

You make an excellent point. Thanks.

[hakre]

Secondary approach

[hakre]

I just ran over this ticket and skimmed throught your discussion here and I thought I throw in my inspiration in form of a second patch:

Instead of handling multiple input types, in this patch I normalize the input first, so it's more easy to deal with afterwards.

As you can see, the code size is reduced and it's less complex.

[wpmuguru]

Replying to loushou:

PLEASE READ WHOLE BUG: because it includes reasons for severity also

THE PROBLEM:

So I have a problem.... obviously, right? So if I were to do something like:

get_posts('post_type[]=post_type_1&post_type[]=post_type_2');

That's where you are going wrong. Try

get_posts('post_type=post_type_1,post_type_2');

[loushou]

Hakre - excellent suggestion. Looks really good and does the same exact thing. Wish I would have thought of that.

WpmuGuru - while that may work for the 'get_post' function, it will not work for a regular url query string. In classes.php on line 224, while the url params are getting looped over, we do not have a statement that will explode each param on ',', however it does make use of:

parse_str($query, $perma_query_vars);

which makes my query work. That being said, perhaps that notation should be added to the patch as well. What does everyone think?

[loushou]

Hakre - On second thought, I may have reacted a little too quickly. Your solution does solve the main problem; however, now ALL the params are getting turned to an array. First I am not sure that the extra logic for every param being an array is being accounted for with you solution (as there is plenty of logic below the for loop that deals with each param individually) and second, I am pretty sure that WP_Query only accepts arrays for certain params, not all params. For instance, had we included a date and time in our URL, the hour, minute, second, month, day, and year params all require single values, not arrays. Same for the name param. This solution is not as complete as I originally thought at first glance. :(

[scribu]

Replying to wpmuguru:

That's where you are going wrong. Try

get_posts('post_type=post_type_1,post_type_2');

Yes, this works, but it doesn't fix the problem. Suppose one wants to implement a form that queries multiple post types:

<form method="get" action="">
  <input type="checkbox" name="post_type[]" value="post_type_1">
  <input type="checkbox" name="post_type[]" value="post_type_2">

...
</form>

The resulting URL will be post_type[]=post_type_1&post_type[]=post_type_2, which is a perfectly reasonoable way to send query variables.

Because of the current bug, you would have to add further code (either JavaScript or a 'request' hook) to transform it into 'post_type=post_type_1,post_type_2', which is a waste.

Still, since this is something that can be worked around and doesn't affect any part in Core, I think it's severity is 'normal'.

[scribu]

Note: the patch I think is commit worthy is ​patch.patch

[scribu]

(In [15737]) Proper handling of array query variables. Props loushou. Fixes #14330

[nacin]

Add an Image popup > Media Library tab:

Catchable fatal error: Object of class stdClass could not be converted to string in /Users/nacin/Sites/3.1/wp-includes/classes.php on line 284

Call Stack:
    0.0005     689064   1. {main}() /Users/nacin/Sites/3.1/wp-admin/media-upload.php:0
    0.4897   28972104   2. do_action() /Users/nacin/Sites/3.1/wp-admin/media-upload.php:120
    0.4897   28973624   3. call_user_func_array() /Users/nacin/Sites/3.1/wp-includes/plugin.php:395
    0.4897   28973656   4. media_upload_library() /Users/nacin/Sites/3.1/wp-includes/plugin.php:0
    0.4897   28973968   5. wp_iframe() /Users/nacin/Sites/3.1/wp-admin/includes/media.php:752
    0.4980   29005416   6. call_user_func_array() /Users/nacin/Sites/3.1/wp-admin/includes/media.php:338
    0.4980   29005752   7. media_upload_library_form() /Users/nacin/Sites/3.1/wp-admin/includes/media.php:0
    0.5679   29142016   8. wp_edit_attachments_query() /Users/nacin/Sites/3.1/wp-admin/includes/media.php:1846
    0.5689   29150032   9. wp() /Users/nacin/Sites/3.1/wp-admin/includes/post.php:923
    0.5689   29150080  10. WP->main() /Users/nacin/Sites/3.1/wp-includes/functions.php:1512
    0.5689   29150080  11. WP->parse_request() /Users/nacin/Sites/3.1/wp-includes/classes.php:519

The object is a post object of the first image in the library.

[scribu]

(In [15793]) Don't attempt to convert objects to strings in WP->parse_request(). Fixes #14330

[scribu]

Related: #15112

[dd32]

In 42419:

WP: Don't attempt to convert multiple-nested arrays to a string in WP->parse_request().

See #14330.
Fixes #42752.