[Query API] Use the exact redirect URL provided in the ?url= query pa…

…ram (#1945)

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.7&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

---------

Co-authored-by: Bero <[email protected]>

packages/playground/remote/src/lib/boot-playground-remote.ts

@@ -129,8 +129,35 @@ 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
+					 */
+					// Get the content window while e.currentTarget is available.
+					// It will be undefined on the next event loop tick.
+					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) {

packages/playground/website/playwright/e2e/query-api.spec.ts

@@ -92,14 +92,47 @@ 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'],
+].forEach(([path, description]) => {
+	test(description, async ({ website, wordpress }) => {
 		await website.goto(`./?url=${encodeURIComponent(path)}`);
 		expect(
-			await wordpress.locator('body').evaluate((body) => body.baseURI)
+			await wordpress
+				.locator('body')
+				.evaluate((body) => body.ownerDocument.location.href)
 		).toContain(path);
 	});
 });
+
+/**
+ * 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.
+ */
+test('should retain encoded control characters in the URL', async ({
+	website,
+	wordpress,
+	browserName,
+}) => {
+	test.skip(
+		browserName === 'firefox' || browserName === 'webkit',
+		`It's unclear why this test fails in Firefox and Safari. The actual feature seems to be working in manual testing. ` +
+		`Let's figure this out and re-enable the test at one point. The upsides of merging the original PR sill ` +
+		`outweighted the downsides of disabling the test on FF.`
+	);
+	const path =
+		'/wp-admin/admin.php?page=html-api-debugger&html=%3Cdiv%3E%0A1%0A2%0A3%0A%3C%2Fdiv%3E';
+	// We need to use the html-api-debugger plugin to test this because
+	// most wp-admin pages enforce a redirect to a sanitized (broken)
+	// version of the URL.
+	await website.goto(
+		`./?url=${encodeURIComponent(path)}&plugin=html-api-debugger`
+	);
+	expect(
+		await wordpress
+			.locator('body')
+			.evaluate((body) => body.ownerDocument.location.href)
+	).toContain(path);
+});

packages/playground/website/src/lib/state/redux/slice-clients.ts

@@ -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,

packages/playground/wordpress/src/index.ts

@@ -161,10 +161,15 @@ export async function setupPlatformLevelMuPlugins(php: UniversalPHP) {
 			 * WordPress uses cookies to determine if the user is logged in,
 			 * so we need to reload the page to ensure the cookies are set.
 			 */
-			wp_redirect(
-				$_SERVER['REQUEST_URI'],
-				302
-			);
+			$redirect_url = $_SERVER['REQUEST_URI'];
+			/**
+			 * 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;
 		}
 		/**