redirect_guess_404_permalink() can catch the wrong post types

[nacin]

When I access /blah, and that 404s, redirect_guess_404_permalink() will try to find a matching post name.

If blah is the start of a post name for a random post type (perhaps one used as internal storage), it'll match it and redirect (and then end up 404ing, potentially). There's no constraint on the post type if the post_type query var isn't set, which is going to be common.

We need to come up with a stricter query. (And while we're at it, we should hook redirect_guess_404_permalink() into redirect_canonical() so it may be unhooked without removing all canonical support.)

[jeremyfelt]

Adds stricter post_type query to redirect_guess_404_permalink

[jeremyfelt]

This patch addresses part 1 of the ticket by only looking for post types that are registered as publicly queryable if a post type has not already been specified as one of the query vars.

I might need clarification to understand part 2.

[nacin]

I am fine with this. It should be 'public' rather than 'publicly_queryable'.

[jeremyfelt]

I think it's safe to be stay specific with 'publicly_queryable' here.

There is a possibility of a post type being registered with 'public' => false and an overriding 'publicly_queryable' => true (or vice versa) that we should allow for.

[nacin]

publicly_querable is simply for whether you should be able to do ?post_type=tweet (yes, this post type is publicly queryable) or ?post_type=page (no, this post type is not publicly queryable). It does not affect whether the post type is publicly accessible — that is controlled by the "public" flag. Anyone setting public to false but publicly_queryable to true is asking for it. :-)

[SergeyBiryukov]

19693.2.diff​ is an attempt to address the second point of the ticket as well (the ability to unhook redirect_guess_404_permalink()).

[nacin]

In [20688]:

Only look for public post types in redirect_guess_404_permalink() when no post type query var is present. props jeremyfelt. fixes #19693.

[jeremyfelt]

In the current state, if I have a post type registered as ( 'public' => true, 'publicly_queryable' => false ), redirect_guess_404_permalink() will attempt to redirect '/blah' to '/non-publicly-queryable/blah/', resulting in a 404 at the new URL.

19693.3.diff​ corrects this by adding 'publicly_queryable' => true as a second argument to get_post_types()

[nacin]

There is nothing wrong with a public post type not being publicly_queryable. For example, pages.

/non-publicly-queryable/blah is a valid construct. What isn't is /non-publicly-queryable — this is what publicly_queryable prevents.

[jeremyfelt]

I understand the coexistence of public and !publicly_queryable. I think that is why redirect_guess_404_permalink() needs both arguments.

When a guess occurs, the resulting redirect initiates a new parse_request(), which will see the guessed query_var, which is directly related to the publicly_queryable argument.

Using my internal post type of 'my-special-post-type', that I have registered as 'public' => true, 'publicly_queryable' => false, 'rewrite' => true , the following happens:

• mysite/post-name
  • not found, initiates redirect_guess_404_permalink()
• mysite/my-special-post-type/post-name
  • becomes the redirected URL after guess, which results in 404 because publicly_queryable is false
  • if publicly_queryable is set to true, this URL becomes valid.

With 'public' => true, 'publicly_queryable' => false, 'rewrite' => false , I get:

• mysite/?my-special-post-type=post-name
  • becomes the redirected URL after guess, which results in 404 because publicly_queryable is false
  • if publicly_queryable is set to true, this URL becomes valid.

19693.3.diff​ adds the argument 'publicly_queryable' => true to the existing 'public' => true, resulting in a 404 at mysite/post-name - leaving out the incorrect guess.

Pages may need to be included separately, as that is a special case.

[ryan]

Per bug scrub, going with 19693.3.diff. Eventually, the confusion here will be eliminated.

[ryan]

19693.3.diff breaks some page related unit tests because publicly_queryable is false for pages, as mentioned in comment:10.

On second thought, we're going to call [20688] good enough. It's certainly an improvement. We'll clean the remainder up in 3.5.

[SergeyBiryukov]

And while we're at it, we should hook redirect_guess_404_permalink() into redirect_canonical() so it may be unhooked without removing all canonical support.

Related: #16557

[MikeSchinkel]

What we really need is a hook to allow us to disable redirect_guess_404_permalink() per #16557.

[wonderboymusic]

I just uploaded a patch to #13041 that has way more nonsense in it https://core.trac.wordpress.org/attachment/ticket/13041/13041.diff

Let's work on it there.