Portal redirect: leave sub-resource fetches alone, so Jetpack's admin-bar sparkline renders - #737
Merged
Conversation
…-bar sparkline renders
Jetpack's admin-bar Stats node is an <img> whose src is
admin.php?page=stats&noheader&proxy&chart=admin-bar-hours-scale: core
skips the header on `noheader` and the page hook echoes PNG bytes.
The admin_init redirect in includes/portal.php treated that request
as a user landing on a plain admin page and forwarded it to the shell
screen, so the <img> received the shell's HTML and drew a broken image
with its alt text ("Statistics") where the chart should be. The same
happens to any XHR a plugin aims at an admin.php?page=…&noheader
endpoint, the Jetpack Stats screen's own report loader included.
The browser already says what it is fetching for. A new
openstation_is_subresource_request() reads Sec-Fetch-Mode: `navigate`
is a document or frame load, the only kind of request worth
forwarding into the desktop; `no-cors` (an <img>, a <script>) and
`cors` (fetch, XHR) are sub-resources, and the redirect now bails on
them next to its AJAX, REST, cron and non-GET bail-outs. A missing
header answers false, so old browsers and header-stripping proxies
keep today's behaviour.
Tests pin the predicate and both sides of the redirect: the sparkline
URL fetched as an image is left alone, and the same screen loaded as
a navigation still forwards to the shell screen. The architecture and
hooks docs say the redirect only applies to navigations.
Collaborator
Author
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.

What it does
Jetpack's admin-bar Stats sparkline renders again inside the desktop. More generally, the
admin_initredirect that sends plain/wp-admin/URLs into the shell now leaves sub-resource fetches alone: an<img>, a<script>or an XHR whose URL is an admin page gets the bytes it asked for instead of the shell's HTML.Rationale
Jetpack's admin-bar node is an
<img>whose src is an admin URL:Core's
admin.phpskips the header onnoheaderand Jetpack's page hook echoes PNG bytes it proxies from stats.wp.com.openstation_redirect_plain_admin_to_portal()treated that request as a user landing on a plain admin page and forwarded it to the shell screen, so the image element received an HTML document and the admin bar drew the broken-image glyph with the alt text, "Statistics", where the chart should be.The same thing happens to any XHR a plugin aims at an
admin.php?page=…&noheaderendpoint. The Jetpack Stats screen loads its report body that way.Implementation
A new predicate in
includes/core/routing.phpreads the browser-setSec-Fetch-Modeheader:navigateis a document or frame load, the only kind of request worth forwarding into the desktop.no-cors(an<img>, a<script>) andcors(fetch, XHR) are sub-resources. A missing header answers false, so old browsers and proxies that strip the header keep today's behaviour rather than gaining a new one.The redirect bails on it next to its existing AJAX, REST, cron,
admin-post.phpand non-GET bail-outs, before the frozen-flag alias route and before theopenstation_admin_redirect_to_portalfilter run. Chromeless iframe loads were already handled throughSec-Fetch-Dest: iframeand are unaffected.Sec-Fetch-Modewas chosen overSec-Fetch-Destbecause it answers the exact question ("is this a navigation?") with one value, where the destination header would need an allowlist ofdocument,iframeandframe.Docs: the request-lifecycle entry in
docs/architecture.mdand theopenstation_admin_redirect_to_portalentry indocs/hooks-reference.mdnow say the redirect only applies to navigations.Testing instructions
npm run test:php -- --filter='Tests_OpenStation_Portal|Tests_OpenStation_OpenStation'New tests: three for the predicate (header missing,
navigate,no-corsandcors) and two for the redirect (the sparkline URL fetched as an image is left alone; the same screen loaded as a navigation still forwards to the shell screen).To see it over HTTP, log in on a wp-env instance with the desktop enabled for the user and compare a navigation with an image-style fetch of the same admin URL:
On a site with Jetpack Stats, load the desktop: the sparkline draws in the admin bar instead of the broken "Statistics" image.