Skip to content

Canonical: Generate a stable cache key in redirect_guess_404_permalink() - #14146

Open
maheshbohara wants to merge 2 commits into
WordPress:trunkfrom
maheshbohara:66282-redirect-guess-404-cache-key
Open

maheshbohara wants to merge 2 commits into
WordPress:trunkfrom
maheshbohara:66282-redirect-guess-404-cache-key

Conversation

@maheshbohara

Copy link
Copy Markdown

[64043] added object caching to the database lookup in redirect_guess_404_permalink(). The cache key is md5( $query ), but for the default (loose) guess the query contains a LIKE clause built by $wpdb->prepare(), so the literal % has been swapped for the wpdb::placeholder_escape() token.

That token is regenerated on every request, so the cache key is different on every request. With a persistent object cache the stored value is never read back: each 404 guess is a cache miss followed by a new cache write.

This PR removes the placeholder escape before hashing, which is what WP_Query::generate_cache_key(), WP_Term_Query and WP_User_Query already do.

Testing

Before the change, running the same lookup in separate PHP processes stores a different key each time:

wp eval 'global $wp_object_cache; set_query_var( "name", "hello" ); redirect_guess_404_permalink(); $p = new ReflectionProperty( $wp_object_cache, "cache" ); foreach ( array_keys( $p->getValue( $wp_object_cache )["post-queries"] ) as $k ) { echo "$k\n"; }'

After the change the key is the same on every run.

A unit test is included. It captures the SQL through the query filter (where the placeholder escape has already been removed) and asserts that the result is cached under the key derived from that SQL. It fails on trunk and passes with this change. The existing tests from [64043] could not catch this because they call the function twice in one PHP process, where the token is constant.

npm run test:php -- --filter redirect_guess_404_permalink tests/phpunit/tests/canonical.php

Trac ticket: https://core.trac.wordpress.org/ticket/66282

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus
Used for: Investigating and reproducing the bug, and drafting the patch, unit test and this description. I reviewed the changes and ran the tests locally.


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

…nk()`.

The cache key was derived from the SQL query while it still contained the `wpdb` placeholder escape for the `LIKE` clause. That escape string is regenerated on every request, so the key was different each time and a persistent object cache could never return the cached result.

This removes the placeholder escape before hashing the query, as `WP_Query`, `WP_Term_Query` and `WP_User_Query` already do.

Follow-up to [64043].

See #66282.
@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props maheshbohara, peterwilsoncc.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@peterwilsoncc peterwilsoncc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

This change looks good to me. I took the liberty of pushing 035f12b to be a little more precise with the test.

I split the final assertion you wrote in to two:

  1. Check the cache key is generated using the SQL query
  2. Check the cache key does not include the placeholder

I'll aim to commit this on Monday Australian time, let me know if you're not happy with the changes.

@maheshbohara

Copy link
Copy Markdown
Author

Thanks for the review and the test tweak, @peterwilsoncc. I'm happy with the changes. The one failing check (Upgrade from 7.1 / PHP 8.4 / MySQL 8.4) looks like a runner issue: WP-CLI failed to install, so wp wasn't found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants