Opened 12 months ago
Last modified 10 months ago
#59766 new defect (bug)
Custom post type archive URLs are being resolved in an unexpected way if a page with the same slug exists and the URL's casing doesn't exactly match the custom post type's rewrite slug
Reported by: | nonverbla | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 6.3.2 |
Component: | Posts, Post Types | Keywords: | has-patch |
Focuses: | Cc: |
Description
Steps to reproduce:
- Have a custom post type with an archive slug of
/projects/
- Have a post with post_type
page
with an identical slug, e.g. "Projects", with a slug of "/projects/" - Visit "https://example.com/Projects" (note the Titlecase spelling here)
- Observe how the request is being resolved to the single page "Projects" instead of the (expected) post type archive page, even though the rule for the post type comes first.
Proposed solution
- I was able to fix this behavior locally by making all archive rewrite rules for custom post types case-insensitive. In
class-wp-post-type.php
, line #658:
<?php // ignore the case of the URL $archive_slug = '(?i)' . $archive_slug; add_rewrite_rule( "{$archive_slug}/?$", "index.php?post_type=$this->name", 'top' ); // ...
Change History (10)
This ticket was mentioned in PR #5591 on WordPress/wordpress-develop by @nonverbla.
12 months ago
#1
- Keywords has-patch added; needs-patch removed
#2
@
12 months ago
Temporary workaround
As a temporary workaround I implemented this filter for rewrite_rules_array
in my theme (just for reference):
<?php /** * Make all archive rewrite rules case-insensitive */ function rah_make_archives_rewrite_rules_case_insensitive(array $rules): array { $new_rules = []; $archive_slugs = array_map( fn ($post_type) => (get_post_type_object($post_type))->has_archive, get_post_types([ 'has_archive' => true, '_builtin' => false ]) ); /** Produces something like "cpt1|cpt2|cpt3" */ $joined_archive_slugs = implode("|", $archive_slugs); /** * Walk through all rules, and if one matches one of the * post type slugs at the beginning, make it case-insensitive */ foreach ($rules as $rule => $rewrite) { if (preg_match("/^($joined_archive_slugs)/", $rule)) { $new_rules["(?i)$rule"] = $rewrite; continue; } $new_rules[$rule] = $rewrite; } return $new_rules; } add_filter('rewrite_rules_array', 'rah_make_archives_rewrite_rules_case_insensitive');
#3
@
10 months ago
Will this be considered at some point? I think it's actually a quite nasty bug. Especially when combined with WP Super Cache, that ignores the case of the URL.
This ticket was mentioned in Slack in #core by nonverbla. View the logs.
10 months ago
This ticket was mentioned in Slack in #core-php by nonverbla. View the logs.
10 months ago
#6
@
10 months ago
As a question about this ticket was posted on Slack, I'm reposting my reply from Slack here:
To be honest, as the handling like that has been in WP for a long time, changing it to be case-insensitive could be a breaking change.
It would need a thorough investigation of how taxonomies are handled throughout Core, as if there is any chance at all that multiple taxonomies could be registered using the same keyword but in a different case, even if only via a plugin, this will break sites.
#7
@
10 months ago
Thank you @jrf for your explanation. It's amazing how thoughtful the WordPress maintainers are about introducing a breaking change.
If I understand correctly, the concern is that with the current behavior, some users might have registered for example two post types, one with an archive slug of books
and the other with a slug of Books
and are relying on WordPress' behavior to treat these two as separate resources?
This would break as soon as they would install a static caching plugin, for example WP Super Cache in expert mode. There, the rewrite rules for static files looks like this:
RewriteCond %{DOCUMENT_ROOT}/wp-content/cache/supercache/%{SERVER_NAME}/$1/index-https.html.gz -f RewriteRule ^(.*) "/wp-content/cache/supercache/%{SERVER_NAME}/$1/index-https.html.gz" [L]
In my tests with Super Cache, both /Books
as well as /books
or even /bOoKs
always matches the same file:
/wp-content/cache/supercache/%{SERVER_NAME}/books/index-https.html.gz
So with the current behavior, the first page being hit with a cache miss would be cached, and subsequent requests would be served with that cached response, no matter the casing in the URL. We have created a response randomizing machine ;)
Should this not be considered broken behavior? Looking forward to what you think.
#8
@
10 months ago
So, I just learned about WordPress' policy of never introducing breaking changes. Then maybe there is another way to help people to "do it right" and save them the trouble I went through to find this?
How about introducing a hint on all appropriate docs pages (register_post_type
, register_taxonomy
, add_rewrite_endpoint
, add_rewrite_rule
, ... come to mind)? Something like:
"Please prefer all-lowercase words for your archive and rewrite slugs."
"Please prefer all-lowercase words when registering custom rewrite rules."
"..."
Or even some kind of doing_it_wrong
warning that is being logged if rewrite rules containing uppercase letters are detected and WP_DEBUG
is true
? This "feature" was causing me a few very stressful moments when it was randomly popping up a while back on a client's website.
It was also very hard to debug, since I never had the idea to actually try the URL with capital letters in my manual tests. In the end, I only found it by writing a custom logger that would send me an email when the response for the URL was looking wrong...
This ticket was mentioned in Slack in #core-php by nonverbla. View the logs.
10 months ago
#10
@
10 months ago
I've created a Gist of what's necessary for me to work around this issue. Posting it here as it may be useful for others, as well. Hope it's not too off-topic.
https://gist.github.com/hirasso/844c6aadf4c7931751c5a3f7e338476c
The script redirects URLs to their lowercase counterpart on the server as well as on the client side (that can become necessary when the page is being served by a static html page created by a caching plugin).
Matches the archive slug of custom post type archives case-insensitive, so that existing pages with the same slug don't take precedence.
The Problem
/projects/
/projects/
/Projects/
in your frontendProposed Solution
Make the rewrite rule for post type archives case-insensitive using the inline modifier
(?i)
as described here.Trac ticket: https://core.trac.wordpress.org/ticket/59766#ticket