Completely ignore ?post_type=page

[scribu]

Follow-up to #17033

If you go to ?post_type=page, you will get a list of pages, displayed as posts, even though the 'page' post type doesn't have 'has_archive' => true.

That is very unexpected behaviour.

The 'post_type' arg should be stripped out before it reaches $wp_query, so that visitors can't access this view, unless explicitly enabled by a plugin.

Huh, in class WP, 'post_type' is set as both a public and a private query var.

Moving to 3.2, as this is technically a regression, introduced along with CPT archives in WP 3.1.

This isn't a regression. Not from 3.1 anyway.

We shouldn't trip out post_type. That'd create a regression.

We should selectively block post_type=page.

Please describe the regression that 17040.diff​ would introduce.

Querying ?post_type=my_custom_post_type where there is no has_archive, i.e. a CPT archive page I rolled myself pre-3.1.

I may be oversimplifying things, but isn't all that's needed is a modification to the arguments used to register the page post type? The publicly_queryable argument is available but it defaults to true if public is true, which it is for pages.

I'm attaching a patch showing what I'm talking about.

Way to think outside the box. :)

We should still fix 'post_type' being set as both a private and a public query var, though.

Yeah. I noticed that while taking a gander at class-wp.php. Odd to say the least.

I haven't dug into that one to find out the possible consequences of changing it.

Clever, but that will do some damage. We use publicly_queryable in a few different ways.

To me, publicly_queryable should do what it says on the tin: allow the post type to be queried publicly. If that isn't all that it means, then it stands to reason (IMHO) that those additional uses should be broken out to separate arguments.

Is there a reason that this does more than just allow the post type to be queried publicly?

To me, pagename=about or page_id=2 is querying that post type publicly. Perhaps I'm wrong in that thought.

See #13301, #14321.

Unless I'm missing something, I would not object to walking that back, so publicly_queryable is for WP() only, and the other checks in core (there's a few) are moved to public.

Dang. I wasn't think of using query vars for that. I had my site set up with permalinks and those worked just fine with it set to false.

Seems like the string for has_archive might have to be tugged to disallow public queries for archives of post types with that set to false. Thoughts?

Even with has_archive => false, post_type=foo should still work, where foo != page.

I like your idea of re-defining publicly_queryable to being the query var itself and restricted to the WP class. We should revert other changes and move them to public.

This patch changes publicly_queryable to only apply to direct post_type query arg requests. Changed public to be used in its place in appropriate locations.

@nacin -- Hmm... And here I thought I was completely off base and didn't know what I was talking about. Go figure.

I made a patch based upon what you said. I'm not terribly familiar with modifying the query structures, but I believe it does what you were suggesting:

1. It replaces the publicly_queryable arg with public in places where decisions are made on whether or not a post type is able to be displayed publicly.
2. It changes the page post type to have publicly_queryable set to false.

I did some tests on this and it allowed for queries of ?page_id and ?pagename but produced a home page query when ?post_type=page was requested.

It definitely needs more testing though by someone much more capable in this regard than myself.

A home page query when ?post_type=foo is requested, when has_archive = false, is expected. The important point is that the query does not look for pages; rather, it is ignored.

I also just wrote the same patch, so I've uploaded it.

This is interesting, but I think it's too big of a low level change to make during beta.

Sounds fine. I'd like to change from publicly_queryable to public now -- or at least on the checks we introduced during 3.2. The others were all on 3.1, those should also be safe. The pages bit, that can wait.

I don't really understand the need to redefine publicly_queryable.

Even with has_archive => false, post_type=foo should still work, where foo != page.

It should work, as long as it's paired with name=bar, i.e. when viewing a single post.

Replying to scribu:

I don't really understand the need to redefine publicly_queryable.

It's not redefining it, it's restoring its original intention.

Even with has_archive => false, post_type=foo should still work, where foo != page.

It should work, as long as it's paired with name=bar, i.e. when viewing a single post.

No, that's incorrect. ?post_type=foo must be allowed to work on non-single posts. The sole exception is when post_type=page.

For my own reference (each item in the list implies the previous one):

• public: ?post_type=foo&name=bar
• publicly_queryable: ?post_type=foo
• has_archive: rewrite rules and query flags

The docblock for 'public' should be updated, as it's currently the same as show_ui.

Related: #17609

Even if we don't commit the register_post_type call, which we should, I'd like to change every incorrect publicly_queryable reference to public. And if we can't do that, then let's at least change the ones we did in 3.2.

After some discussion with nacin and some helpful clarity from scribu's comment, I think this should go in. The use of publicly_queryable was a mistake.

In [18234]:

Fix mistaken use of publicly_queryable when public was what was intended. props nacin. fixes #17040