Repository navigation
Rewrite Rules: Send no-cache and noindex headers for random content that is not redirected - #15825
rene-hermenau wants to merge 3 commits into
Conversation
…hat is not redirected. When the permalink of the randomly selected post is on a host outside allowed_redirect_hosts, or is empty, wp_random_content_redirect() returned before sending nocache_headers() and the X-Robots-Tag header. The random one-post archive was then served as a cacheable, indexable 200 response, so a page cache could pin /random/ to one post. Send both headers before the permalink check. The redirect itself is unchanged. Follow-up to [64402]. See #66625.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
|
Test using WordPress PlaygroundThe 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
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
|
@propsbot add ReneHermi |
aaronjorbin
left a comment
aaronjorbin
left a comment
There was a problem hiding this comment.
Mostly looks good, a few small suggestions.
|
|
||
| $headers = xdebug_get_headers(); | ||
|
|
||
| $this->assertContains( 'Cache-Control: no-cache, must-revalidate, max-age=0, no-store, private', $headers, 'The random content response should not be cached.' ); |
There was a problem hiding this comment.
This hard-codes the value wp_get_nocache_headers() returns for a logged-out request, so the test breaks if that string ever changes for unrelated reasons.
| $this->assertContains( 'Cache-Control: no-cache, must-revalidate, max-age=0, no-store, private', $headers, 'The random content response should not be cached.' ); | |
| $this->assertContains( 'Cache-Control: ' . wp_get_nocache_headers()['Cache-Control'], $headers, 'The random content response should not be cached.' ); |
peterwilsoncc
left a comment
peterwilsoncc
left a comment
There was a problem hiding this comment.
@rene-hermenau Thanks for the pull request.
@aaronjorbin and I had a brief discussion and have come to the shared view it's probably best to use wp_redirect() rather than wp_safe_redirect().
Our logic is that the only way a permalink can be modified to return a URL that is not on the same site is via a plugin. As the plugin is intentionally setting the permalink to another domain then it can be considered safe enough to redirect to that location.
Another consideration is that if the post_content is being used to note why the permalink points to another domain then that content ought not be exposed. The use of wp_safe_redirect() is currently exposing that date.
I've popped a couple of suggestion inline to remove the key lines that limit the redirect to the safe redirect allow list.
| if ( wp_safe_redirect( $location, 302 ) ) { | ||
| exit; | ||
| } |
There was a problem hiding this comment.
| if ( wp_safe_redirect( $location, 302 ) ) { | |
| exit; | |
| } | |
| wp_redirect( $location, 302 ); | |
| exit; |
| // Only consider redirecting permitted locations so wp_safe_redirect doesn't redirect to the fallback. | ||
| if ( ! $location || ! wp_validate_redirect( $location, '' ) ) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
| // Only consider redirecting permitted locations so wp_safe_redirect doesn't redirect to the fallback. | |
| if ( ! $location || ! wp_validate_redirect( $location, '' ) ) { | |
| return; | |
| } |
|
@rene-hermenau I've made updates based on mine and @peterwilsoncc's review. Does this look good to you? |
wp_random_content_redirect()sentnocache_headers()andX-Robots-Tag: noindex, followonly after the selected post's permalink passedwp_validate_redirect(). When the permalink is on a host outsideallowed_redirect_hosts, or empty, the request rendered the randomly selected one-post archive as a200without either header, so a page cache could pin/random/to one post.This sends both headers before the permalink check. The redirect itself is unchanged.
The new test file follows the
@runTestsInSeparateProcesses+xdebug_get_headers()pattern oftests/phpunit/tests/oembed/headers.php. Without the change, the off-site and empty-permalink cases fail; with it, all three pass.tests/phpunit/tests/query/randomContentRedirect.phppasses unchanged on single site and multisite.Trac ticket: https://core.trac.wordpress.org/ticket/66625
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Reproducing the issue on a local 7.2 nightly, writing the change and the test, and running the tests. I reviewed the change and the test results.
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.