Opened 14 years ago
Closed 14 years ago
#17040 closed defect (bug) (fixed)
Completely ignore ?post_type=page
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 3.2 | Priority: | normal |
Severity: | normal | Version: | 3.0 |
Component: | Posts, Post Types | Keywords: | has-patch |
Focuses: | Cc: |
Description
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.
Attachments (4)
Change History (33)
#3
@
14 years ago
- Milestone changed from Future Release to 3.2
Moving to 3.2, as this is technically a regression, introduced along with CPT archives in WP 3.1.
#4
@
14 years ago
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.
#7
@
14 years ago
Please describe the regression that 17040.diff would introduce.
#8
@
14 years ago
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.
#9
@
14 years ago
- Cc gaarai@… added
- Keywords has-patch added
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.
#10
@
14 years ago
Way to think outside the box. :)
We should still fix 'post_type' being set as both a private and a public query var, though.
#11
@
14 years ago
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.
#12
@
14 years ago
Clever, but that will do some damage. We use publicly_queryable in a few different ways.
#13
@
14 years ago
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?
#15
@
14 years ago
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.
#16
@
14 years ago
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?
#17
@
14 years ago
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
.
@
14 years ago
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.
#18
@
14 years ago
@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:
- 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.
- 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.
#19
@
14 years ago
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.
#20
@
14 years ago
This is interesting, but I think it's too big of a low level change to make during beta.
#21
@
14 years ago
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.
#22
follow-up:
↓ 23
@
14 years ago
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.
#23
in reply to:
↑ 22
@
14 years ago
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.
#24
@
14 years ago
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'.
#27
@
14 years ago
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.
Huh, in class WP, 'post_type' is set as both a public and a private query var.