Make WordPress Core

Opened 6 months ago

Last modified 3 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's profile 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.


6 months ago
#1

  • Keywords has-patch added; needs-patch removed

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

  • Have a page "Projects" with a slug of /projects/
  • Have a custom post type with an archive slug of /projects/
  • Visit /Projects/ in your frontend
  • Observe how WordPress resolves to the single page instead of the archive page

Proposed 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

#2 @nonverbla
6 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 @nonverbla
3 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.


3 months ago

This ticket was mentioned in Slack in #core-php by nonverbla. View the logs.


3 months ago

#6 @jrf
3 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 @nonverbla
3 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 @nonverbla
3 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.


3 months ago

#10 @nonverbla
3 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).

Version 0, edited 3 months ago by nonverbla (next)
Note: See TracTickets for help on using tickets.