Repository navigation
Conversation
On a subfolder network the site lookup falls back to shorter paths, so a request for a path that does not belong to any site still resolves to one. Requesting /typo/wp-admin/ therefore loaded the main site's dashboard while the URL kept saying /typo/, which reads as though the requested site exists and makes it easy to edit the wrong site. When an admin request resolves to a site whose path does not match the requested path, send it to the admin URL of the site that was actually found. Front end requests are untouched, and a request that does match its site is never redirected, so the next request settles. Props MadtownLems, ddean, nacin, jeremyfelt, Denis-de-Bernardy, ocean90. Fixes #17376.
Cover that an admin request whose path resolves to a different site is redirected to the found site's admin URL, that a matching path is not redirected, that front end requests are unaffected, and that the destination is filterable. Props MadtownLems, ddean, nacin, jeremyfelt, Denis-de-Bernardy, ocean90. Fixes #17376.
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On a subfolder network, requesting a path that has no site behind it still resolves to a site, because
get_site_by_path()walks back through shorter paths and finally tries/. The consequence for admin requests is thatfoo.org/typo/wp-admin/loads the main site's dashboard while the URL keeps saying/typo/. Nothing indicates the requested site does not exist, so a typo silently lands you in a different site's admin, and the reporter changed the wrong site that way. It is also convenient for scanners, which can reach a real dashboard on an unlimited number of paths.The bug is still present on trunk. The bootstrap already handles the case where no site can be found at all: it redirects to the network path. What it does not handle is the case where a site is found at a different path than the one requested.
This adds that case to
ms_load_current_site_and_network(). When the request is for the admin area and the resolved site's path does not match the requested path, the function returns a destination string for the site that was actually found, andms-settings.phpsends it as a redirect.foo.org/typo/wp-admin/therefore lands onfoo.org/wp-admin/, andfoo.org/bar/typo/wp-admin/lands onfoo.org/bar/wp-admin/rather than serving/bar/at the bogus path.Three details worth noting for review:
$current_blog->path, the site actually found, not from the requested path. That is what makes the URL afterwards match the site being served.is_admin()requests are touched. A front end request to/typo/keeps resolving to the main site as it does today, so this does not change front end behaviour.A filter,
ms_site_path_mismatch_redirect, exposes the destination.The alternative of teaching
get_site_by_path()about admin requests was raised during the ticket's history and rejected in comment #19, since it would put global state into a general purpose utility. The check here stays in the bootstrap where the redirect decision is already made.Trac ticket: https://core.trac.wordpress.org/ticket/17376
Use of AI Tools
AI assistance: Yes
Tool(s): Claude
Model(s): Sonnet
Used for: test cases, code review
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.