WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 8 weeks ago

#16949 new defect (bug)

Custom post types: error on page if query contains multiple post types

Reported by: mastermind Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.1
Component: Posts, Post Types Keywords: 2nd-opinion
Focuses: Cc:

Description

I'm using custom post types, and I modified the $query to include multiple post types. This works quite well, until I enter a URL to a non-existant post (modifying the last part, i.e. the post's slug).

// some HOWTO linked from the Codex suggests this,
// can't find the link right now

function add_posttypes_to_query($query)
{
	if (!is_admin() && !$query->query_vars['suppress_filters'])
	{
		if (!is_home())
			$post_type = array('video', 'post');

		$query->set('post_type', $post_type);
	}

	return $query;
}

add_filter('pre_get_posts', 'add_posttypes_to_query');

The error I get is: “Warning: Illegal offset type in isset or empty in /home/lxg/www/shops.supreme.de/supremeshops/branches/private_wp3/wp-includes/post.php on line 811”.

It appears that the function get_post_type_object gets the array passed if no posts were found.

Also, I realized that custom post single pages don't issue a HTTP 404 status on errors. But this may be due to local modifications. I am currently not able to set up an environment where I isolate this problem.

Attachments (2)

16949.diff (498 bytes) - added by aaroncampbell 3 years ago.
Don't attempt old slug redirect for multi-post_type queries
16949.2.diff (1.6 KB) - added by aaroncampbell 3 years ago.
Allow wp_old_slug_redirect() to handle multiple post types

Download all attachments as: .zip

Change History (16)

comment:1 nacin3 years ago

  • Milestone Awaiting Review deleted
  • Version set to 3.1

There's a few get_post_type_object() calls in query.php. At least one might allow an array to slip through.

Can anyone track this down? This looks like a good bug to fix.

comment:2 aaroncampbell3 years ago

It looks like this is caused by the call to is_post_type_hierarchical() in wp_old_slug_redirect(). It just passes through what it gets from get_query_var('post_type') which can be an array. The check was added in [16818] and [16819] to fix #15140

It seems like we should just check for an array, loop through, and return if ANY of them is hierarchical? I'll test it an put up a patch in a few minutes.

comment:3 nacin3 years ago

  • Milestone set to 3.2

3.1 regression, 3.2 for now.

aaroncampbell3 years ago

Don't attempt old slug redirect for multi-post_type queries

aaroncampbell3 years ago

Allow wp_old_slug_redirect() to handle multiple post types

comment:4 aaroncampbell3 years ago

The problem turned out to be that wp_old_slug_redirect() expects post_type to be a string. It not only passes it straight to is_post_type_hierarchical() but a couple lines later it pushes into a query using $wpdb->prepare(). This leaves two options: Upgrade wp_old_slug_redirect() to handle multiple post types, or bail when we see we have multiple post types.

As nacin pointed out in chat " One might argue that you can't guess which page to redirect to if it could be one of a few post types". It seems to me that our guess would be less accurate, but we still might be able to do it. Both options have their advantages.

Not guessing would open things up for whoever is adding the additional post types to the query to do the guessing themselves. A little less "magical" but pretty flexible. This is attached as 16949.diff

If we want more magic we could actually force post_type to an array, loop through and generate the post_type= bits of the query from the non-heirarchical post_types, which creates a query like this:

SELECT post_id FROM wp_postmeta, wp_posts WHERE ID = post_id AND ( post_type = 'somepostype' OR post_type = 'anotherposttype' ) AND meta_key = '_wp_old_slug' AND meta_value = 'bad-slug'

This is attached as 16949.2.diff

comment:5 aaroncampbell3 years ago

  • Keywords has-patch 2nd-opinion added

comment:6 ryan3 years ago

Fine with 16949.diff for 3.2. The other can be considered for another day.

comment:7 nacin3 years ago

Per IRC: Small patch now, big one in 3.3. One fixes a bug, one adds support for something that's never worked.

comment:8 ryan3 years ago

In [18330]:

Don't attempt old slug redirect for multi-post_type queries. Props aaroncampbell. see #16949

comment:9 ryan3 years ago

  • Milestone changed from 3.2 to Future Release

comment:10 SergeyBiryukov3 years ago

  • Milestone changed from Future Release to 3.3

comment:11 ryan2 years ago

  • Milestone changed from 3.3 to Future Release

Punting the remainder.

comment:12 avryl8 months ago

  • Component changed from General to Post Types

comment:13 aaroncampbell8 months ago

  • Keywords has-patch removed

What's left here is making wp_old_slug_redirect() work it's magic even on queries with more than one post type. I'm on the fence. The guessing won't be as accurate, but then again it'll be better than it is now. The question is: How many times, when multiple post types are queried, would it be beneficial to redirect based on _wp_old_slug meta?

Honestly, it seems like the redirects that happen would be pretty accurate since they're based on _wp_old_slug, but it's also really simple to hook into template_redirect and handle this stuff yourself if you're modifying the query.

comment:14 SergeyBiryukov8 weeks ago

#17337 was marked as a duplicate.

Note: See TracTickets for help on using tickets.