Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Raw response for Optimization Detective storage request failure could be displayed in IFRAME with srcdoc instead of as raw HTML in PRE #1828

Open
westonruter opened this issue Jan 27, 2025 · 5 comments · May be fixed by #1849
Open

Raw response for Optimization Detective storage request failure could be displayed in IFRAME with srcdoc instead of as raw HTML in PRE #1828

westonruter opened this issue Jan 27, 2025 · 5 comments · May be fixed by #1849
Labels
[Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature
Milestone
optimization-dete...

Comments

@westonruter
Copy link
Member

westonruter commented Jan 27, 2025 •
edited
Loading

Feature Description

When the storage endpoint for Optimization Detective is not accessible (see #1762), an error notice is displayed:

Image

When expanding the raw response, this can can result in an HTML page being shown in code:

Image

It would be preferable if this were presented more nicely, such as in an IFRAME via srcdoc (with sandbox), like so:

Image

This should be straightforward to do simply by checking if the cached response has a text/html type, and if so, present in an IFRAME instead of as a PRE tag.
@westonruter westonruter added [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature labels Jan 27, 2025
@github-project-automation github-project-automation bot moved this to Not Started/Backlog 📆 in WP Performance 2024 Jan 27, 2025
@westonruter westonruter moved this from Not Started/Backlog 📆 to To Do 🔧 in WP Performance 2024 Jan 27, 2025
@SohamPatel46
Copy link
Contributor

@westonruter I am not able to generate the raw response mentioned in the ticket. Am I missing anything ?

Image

@westonruter
Copy link
Member Author

@SohamPatel46 yeah, you won't be able to using wp-env since loopback requests don't work. In wp-env only a WP_Error will occur, so there is no response. To force wp-env to show the response you'll have to add a plugin that adds a pre_http_request filter which then returns the response you want to test for. The two main possibilities are a JSON response from the REST API when a plugin is disabling unauthenticated requests, or an HTML response in which case Nginx or some higher level is blocking the request.

@westonruter
Copy link
Member Author

See this relevant test case:

performance/plugins/optimization-detective/tests/test-site-health.php

Lines 117 to 128 in d94977f

'nginx_forbidden' => array(
'mocked_response' => array(
'response' => array(
'code' => 403,
'message' => 'Forbidden',
),
'body' => "<html>\n<head><title>403 Forbidden</title></head>\n<body>\n<center><h1>403 Forbidden</h1></center>\n<hr><center>nginx</center>\n</body>\n</html>",
),
'expected_option' => '1',
'expected_status' => 'recommended',
'expected_unavailable' => true,
),

The test case there should have included:

'headers' => array( 'content-type' => 'text/html' )

But that can be added in the next PR related to this.

Here is a plugin you can use to emulate this:

<?php
/**
 * Plugin Name: Simulate REST API Blocked by Web Server
 */

add_filter(
	'pre_http_request',
	static function ( $pre, $args, $url ) {
		if ( ! str_starts_with( $url, rest_url() ) ) {
			return $pre;
		}
		return array(
			'response' => array(
				'code'    => 403,
				'message' => 'Forbidden',
			),
			'headers' => array(
				'content-type' => 'text/html',
			),
			'body'     => "<html>\n<head><title>403 Forbidden</title></head>\n<body>\n<center><h1>403 Forbidden</h1></center>\n<hr><center>nginx</center>\n</body>\n</html>",
		);
	},
	10,
	3
);

This is what that results in on the Site Health screen:

Image

@SohamPatel46
Copy link
Contributor

@westonruter The warning displayed is rendered using wp_admin_notice() function.
And wp_admin_notice() uses wp_kses_post() function to echo the notices. Hence, using iframes would be clipped before it gets displayed.

To avoid this issue, we need to whitelist iframes to be allowed in wp_kses_post fuctions using something like this - https://gist.github.com/bjorn2404/8afe35383a29d2dd1135ae0a39dc018c

Let me know if this is a good idea to implement and whitelisting iframes is safe.

@westonruter
Copy link
Member Author

westonruter commented Feb 4, 2025 •
edited
Loading

I see that wp_admin_notice() is just a wrapper around wp_get_admin_notice() which runs its output through Kses. I suggest therefore to just call wp_get_admin_notice() directly with the appropriate parameters for wp_kses().

(Sorry, I accidentally submitted this comment prematurely.)

@github-project-automation github-project-automation bot moved this from To Do 🔧 to Done 😃 in WP Performance 2024 Feb 4, 2025
@westonruter westonruter reopened this Feb 4, 2025
@github-project-automation github-project-automation bot moved this from Done 😃 to Not Started/Backlog 📆 in WP Performance 2024 Feb 4, 2025
@SohamPatel46 SohamPatel46 linked a pull request Feb 5, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
[Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature
Projects
WP Performance 2024
Status: Not Started/Backlog 📆
Milestone
optimization-detective 1.0.0-beta2
2 participants
@westonruter @SohamPatel46