-
Notifications
You must be signed in to change notification settings - Fork 106
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.
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
Optimization Detective can be blocked from working due to sites disabling the REST API #1731
Comments
Where would it make the most sense to place this site health functionality: in |
It needs to go in Optimization Detective because it needs to work regardless of whether Performance Lab is active. In fact, there are other Site Health tests that would make more sense moved into their respective plugins. For example, the |
As part of this we should also update the readmes for Optimization Detective, Image Prioritizer, and Embed Optimizer to indicate that the REST API needs to be accessible from the frontend to regular visitors in order to work. |
Added in #1763 |
At some point we decided to keep all of the site health checks in the main plugin, to keep them all in one place. I agree it makes more sense for them to live within the individual plugins structurally, not sure it is worth trying to shift things now though? |
Yeah, not a high priority. There are five Site Health tests currently: The first two would remain in Performance Lab since they aren't related to any other plugins. But the last three would make sense to move to the Modern Image Formats plugin at some point. Otherwise, someone could install the Modern Image Formats plugin alone without Performance Lab and then miss out on those Site Health tests which would are dependencies for the plugin working properly. I've filed this as #1781 to discuss further. |
I've been querying HTTP Archive for sites using Image Prioritizer to see how effective it is in the field. I've noticed that for about 17% of pages, there are no URL Metrics being gathered. When looking at some of those URLs (perhaps 5% of all URLs, or 30% of the pages which have no URL Metrics), the culprit is that the REST API endpoint for storing URL Metrics is blocked either with a
403 Forbidden
or401 Unauthorized
response.Sometimes the response is blocked by WordPress, for example in one of the 401 responses I got an error message:
(In this case where anonymous access is blocked, a potential solution would be #1311.)
In other cases, I got a forbidden response directly from Nginx instead. In other words, we can't rely on checking internally in PHP for whether the REST API is enabled.
Optimization Detective needs to do some Health Check to see if the
/optimization-detective/v1/url-metrics:store
POST requests are working or not. If not, then we need to prevent Optimization Detective from trying to do any optimizations or adding the detection script to the page, since the URL Metric submissions will just get blocked anyway.I suggest we add a new Site Health check that checks periodically for healthiness of submitting a URL Metric for storage via the REST API, and for the result of the test to be stored so that the optimization logic can be short-circuited. Site Health should display the latest result of the test, whether it was OK, Forbidden, or Unauthorized.
Note that it may not be suitable to do the testing REST API request using PHP because it could be that a site allows loopback requests to the REST API but blocks requests from other origins. However, I don't believe such Ajax tests run without the user visiting Site Health. So PHP-based loopback requests may be what we have to go with. Note the avif-headers Site Health test for an example of how this could be implemented. There is also core's REST API test which could be reused or copied. Ideally the test would perform a
POST
request to the/optimization-detective/v1/url-metrics:store
and verify it is returning the expected400
status code with a JSON body saying it is missing the required parameters likeurl
,hmac
, andslug
.Note that core used to have a
rest_enabled
filter which has been deprecated and no longer is usable to disable the REST API. Therest_authentication_errors
filter is supposed to be used to restrict access instead. It's the filter that the Disable REST API plugin uses. So while we might be able to used this avoid doing a loopback request to the endpoint to determine whether it is accessible, this won't account for the other ways that the REST API can be blocked, for example via Nginx.The text was updated successfully, but these errors were encountered: