Skip to content

Commit

Permalink
[Query API] Use the exact redirect URL provided in the ?url= query param
Browse files Browse the repository at this point in the history 
This PR:

* Replaces the `wp_redirect()` call with `header("Location: ")` to ensure
support for all valid redirection URLs.
* Adds a timeout before probing for the embedded Playground `iframe` URL
  when the `load` event handler runs.

 ### Ditching `wp_redirect()`

The `wp_redirect()` call was introduced in #1856 to handle the post-autologin
redirect. Unfortunately, it strips valid sequences such as `%0A` and `%0D` from
the redirection URL. This isn't useful in Playground and breaks valid
use-cases such as using the `?url=` parameter to initialize `html-api-debugger`
with a valid HTML markup containing newlines.

 ### Timeout before probing `contentWindow.location.href`

When navigating to a page with %0A sequences (encoded newlines)
in the query string, the `location.href` property of the
iframe's content window doesn't seem to reflect them. Everything
else is in place, but not the %0A sequences.

Weirdly, these sequences are available after the next event
loop tick – hence the `setTimeout(0)`.

The exact cause is unclear to me. The WHATWG HTML Standard [1] has a few hints:

* Current and active session history entries may get out of
  sync for iframes.
* Documents inside iframes have "is delaying load events" set
  to true.

But there doesn't seem to be any concrete explanation and no
recommended remediation. If anyone has a clue, please share it
in a GitHub issue or start a new PR.

[1] https://html.spec.whatwg.org/multipage/document-sequences.html#nav-active-history-entry

 ## Testing instructions

CI tests aside, try this:

1. Go to http://localhost:5400/website-server/?plugin=html-api-debugger&url=%2Fwp-admin%2Fadmin.php%3Fpage%3Dhtml-api-debugger%26html%3D%253Cdiv%253E%250A1%250A2%250A3%250A%253C%252Fdiv%253E&wp=6.6&php=8.0&networking=no&language=&multisite=no&random=pf3oq1y3vx
2. Confirm the "address bar" on the Playground page reflects the actual,
   correct URL:
   `/wp-admin/admin.php?page=html-api-debugger&html=%3Cdiv%3E%0A1%0A2%0A3%0A%3C%2Fdiv%3E`
3. Confirm you can see the following HTML

```
<div>
1
2
3
</div>
```

cc @sirreal @bgrgicak
  • Loading branch information
@adamziel
adamziel committed Oct 25, 2024
1 parent 2ce6b10 commit e57f55b
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 129 deletions.
27 changes: 26 additions & 1 deletion packages/playground/remote/src/lib/boot-playground-remote.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,33 @@ export async function bootPlaygroundRemote() {
// Manage the address bar
wpFrame.addEventListener('load', async (e: any) => {
try {
/**
* When navigating to a page with %0A sequences (encoded newlines)
* in the query string, the `location.href` property of the
* iframe's content window doesn't seem to reflect them. Everything
* else is in place, but not the %0A sequences.
*
* Weirdly, these sequences are available after the next event
* loop tick – hence the `setTimeout(0)`.
*
* The exact cause is unclear at the moment of writing of this
* comment. The WHATWG HTML Standard [1] has a few hints:
*
* * Current and active session history entries may get out of
* sync for iframes.
* * Documents inside iframes have "is delaying load events" set
* to true.
*
* But there doesn't seem to be any concrete explanation and no
* recommended remediation. If anyone has a clue, please share it
* in a GitHub issue or start a new PR.
*
* [1] https://html.spec.whatwg.org/multipage/document-sequences.html#nav-active-history-entry
*/
const contentWindow = e.currentTarget!.contentWindow;
await new Promise((resolve) => setTimeout(resolve, 0));
const path = await playground.internalUrlToPath(
e.currentTarget!.contentWindow.location.href
contentWindow.location.href
);
fn(path);
} catch (e) {
Expand Down
19 changes: 14 additions & 5 deletions packages/playground/website/playwright/e2e/query-api.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,20 @@ test('should not login the user in if the login query parameter is set to no', a
);
});

['/wp-admin/', '/wp-admin/post.php?post=1&action=edit'].forEach((path) => {
test(`should correctly redirect encoded wp-admin url to ${path}`, async ({
website,
wordpress,
}) => {
[
['/wp-admin/', 'should redirect to wp-admin'],
['/wp-admin/post.php?post=1&action=edit', 'should redirect to post editor'],
/**
* There is no reason to remove encoded control characters from the URL.
* For example, the html-api-debugger accepts markup with newlines encoded
* as %0A via the query string.
*/
[
'/wp-admin/?control-characters=%0A%0D',
'should retain encoded control characters in the URL',
],
].forEach(([path, description]) => {
test(description, async ({ website, wordpress }) => {
await website.goto(`./?url=${encodeURIComponent(path)}`);
expect(
await wordpress.locator('body').evaluate((body) => body.baseURI)
Expand Down
9 changes: 0 additions & 9 deletions packages/playground/website/src/lib/state/redux/slice-clients.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,6 @@ const clientsSlice = createSlice({
name: 'clients',
initialState,
reducers: {
// addClientInfo: (state, action: PayloadAction<ClientInfo>) => {
// return clientsAdapter.addOne(state, action.payload);
// },
// updateClientInfo: (state, action: PayloadAction<ClientInfo>) => {
// return clientsAdapter.updateOne(state, {
// id: action.payload.siteSlug,
// changes: action.payload,
// });
// },
addClientInfo: clientsAdapter.addOne,
updateClientInfo: (
state,
Expand Down
14 changes: 10 additions & 4 deletions packages/playground/wordpress/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,16 @@ export async function setupPlatformLevelMuPlugins(php: UniversalPHP) {
$parts = explode('/', $redirect_url);
$redirect_url = '/' . implode('/', array_slice($parts, 2));
}
wp_redirect(
home_url($redirect_url),
302
);
$redirect_url = home_url($redirect_url);
/**
* Intentionally do not use wp_redirect() here. It removes
* %0A and %0D sequences from the URL, which we don't want.
* There are valid use-cases for encoded newlines in the query string,
* for example html-api-debugger accepts markup with newlines
* encoded as %0A via the query string.
*/
header( "Location: $redirect_url", true, 302 );
exit;
}
/**
Expand Down
189 changes: 79 additions & 110 deletions tsconfig.base.json
View file
Original file line number Diff line number Diff line change
@@ -1,112 +1,81 @@
{
"compileOnSave": false,
"compilerOptions": {
"rootDir": ".",
"sourceMap": true,
"declaration": false,
"moduleResolution": "node",
"esModuleInterop": true,
"emitDecoratorMetadata": true,
"experimentalDecorators": true,
"importHelpers": true,
"resolveJsonModule": true,
"jsx": "react",
"target": "ES2021",
"module": "esnext",
"lib": [
"ES2022",
"esnext.disposable",
"dom"
],
"skipLibCheck": true,
"skipDefaultLibCheck": true,
"baseUrl": ".",
"paths": {
"@php-wasm/cli": [
"packages/php-wasm/cli/src/main.ts"
],
"@php-wasm/fs-journal": [
"packages/php-wasm/fs-journal/src/index.ts"
],
"@php-wasm/logger": [
"packages/php-wasm/logger/src/index.ts"
],
"@php-wasm/node": [
"packages/php-wasm/node/src/index.ts"
],
"@php-wasm/node-polyfills": [
"packages/php-wasm/node-polyfills/src/index.ts"
],
"@php-wasm/private": [
"packages/php-wasm/private/src/index.ts"
],
"@php-wasm/progress": [
"packages/php-wasm/progress/src/index.ts"
],
"@php-wasm/scopes": [
"packages/php-wasm/scopes/src/index.ts"
],
"@php-wasm/stream-compression": [
"packages/php-wasm/stream-compression/src/index.ts"
],
"@php-wasm/universal": [
"packages/php-wasm/universal/src/index.ts"
],
"@php-wasm/util": [
"packages/php-wasm/util/src/index.ts"
],
"@php-wasm/web": [
"packages/php-wasm/web/src/index.ts"
],
"@php-wasm/web-service-worker": [
"packages/php-wasm/web-service-worker/src/index.ts"
],
"@wp-playground/blueprints": [
"packages/playground/blueprints/src/index.ts"
],
"@wp-playground/cli": [
"packages/playground/cli/src/cli.ts"
],
"@wp-playground/client": [
"packages/playground/client/src/index.ts"
],
"@wp-playground/common": [
"packages/playground/common/src/index.ts"
],
"@wp-playground/components": [
"packages/playground/components/src/index.ts"
],
"@wp-playground/nx-extensions": [
"packages/nx-extensions/src/index.ts"
],
"@wp-playground/remote": [
"packages/playground/remote/src/index.ts"
],
"@wp-playground/storage": [
"packages/playground/storage/src/index.ts"
],
"@wp-playground/sync": [
"packages/playground/sync/src/index.ts"
],
"@wp-playground/unit-test-utils": [
"packages/playground/unit-test-utils/src/index.ts"
],
"@wp-playground/website": [
"packages/playground/website/src/index.ts"
],
"@wp-playground/wordpress": [
"packages/playground/wordpress/src/index.ts"
],
"@wp-playground/wordpress-builds": [
"packages/playground/wordpress-builds/src/index.ts"
],
"isomorphic-git": [
"./isomorphic-git/src"
]
}
},
"exclude": [
"node_modules",
"tmp"
]
"compileOnSave": false,
"compilerOptions": {
"rootDir": ".",
"sourceMap": true,
"declaration": false,
"moduleResolution": "node",
"esModuleInterop": true,
"emitDecoratorMetadata": true,
"experimentalDecorators": true,
"importHelpers": true,
"resolveJsonModule": true,
"jsx": "react",
"target": "ES2021",
"module": "esnext",
"lib": ["ES2022", "esnext.disposable", "dom"],
"skipLibCheck": true,
"skipDefaultLibCheck": true,
"baseUrl": ".",
"paths": {
"@php-wasm/cli": ["packages/php-wasm/cli/src/main.ts"],
"@php-wasm/fs-journal": [
"packages/php-wasm/fs-journal/src/index.ts"
],
"@php-wasm/logger": ["packages/php-wasm/logger/src/index.ts"],
"@php-wasm/node": ["packages/php-wasm/node/src/index.ts"],
"@php-wasm/node-polyfills": [
"packages/php-wasm/node-polyfills/src/index.ts"
],
"@php-wasm/private": ["packages/php-wasm/private/src/index.ts"],
"@php-wasm/progress": ["packages/php-wasm/progress/src/index.ts"],
"@php-wasm/scopes": ["packages/php-wasm/scopes/src/index.ts"],
"@php-wasm/stream-compression": [
"packages/php-wasm/stream-compression/src/index.ts"
],
"@php-wasm/universal": ["packages/php-wasm/universal/src/index.ts"],
"@php-wasm/util": ["packages/php-wasm/util/src/index.ts"],
"@php-wasm/web": ["packages/php-wasm/web/src/index.ts"],
"@php-wasm/web-service-worker": [
"packages/php-wasm/web-service-worker/src/index.ts"
],
"@wp-playground/blueprints": [
"packages/playground/blueprints/src/index.ts"
],
"@wp-playground/cli": ["packages/playground/cli/src/cli.ts"],
"@wp-playground/client": [
"packages/playground/client/src/index.ts"
],
"@wp-playground/common": [
"packages/playground/common/src/index.ts"
],
"@wp-playground/components": [
"packages/playground/components/src/index.ts"
],
"@wp-playground/nx-extensions": [
"packages/nx-extensions/src/index.ts"
],
"@wp-playground/remote": [
"packages/playground/remote/src/index.ts"
],
"@wp-playground/storage": [
"packages/playground/storage/src/index.ts"
],
"@wp-playground/sync": ["packages/playground/sync/src/index.ts"],
"@wp-playground/unit-test-utils": [
"packages/playground/unit-test-utils/src/index.ts"
],
"@wp-playground/website": [
"packages/playground/website/src/index.ts"
],
"@wp-playground/wordpress": [
"packages/playground/wordpress/src/index.ts"
],
"@wp-playground/wordpress-builds": [
"packages/playground/wordpress-builds/src/index.ts"
],
"isomorphic-git": ["./isomorphic-git/src"]
}
},
"exclude": ["node_modules", "tmp"]
}

0 comments on commit e57f55b

Please sign in to comment.