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.

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

GET call is Made to wp.org from health check #14

Open
ipajen opened this issue Oct 16, 2024 — with Slack · 15 comments
Open

GET call is Made to wp.org from health check #14

ipajen opened this issue Oct 16, 2024 — with Slack · 15 comments
Labels
needs triage needs to be discussed and worked on

Comments

Copy link
Collaborator

ipajen commented Oct 16, 2024

On wp-admin/site-health.php?tab=debug it’s making a GET to http://Wp.org but not on AP for WP_Debug_Data::debug_data()
wp-admin/includes/class-wp-debug-data.php:430
WP_Site_Health->show_site_health_tab()
wp-admin/includes/class-wp-site-health.php:68
do_action('site_health_tab_content')
wp-includes/plugin.php:517

@namithj
Copy link
Contributor

namithj commented Oct 22, 2024

Found 3 Calls on this page and all three are being rerouted

From Debug Log for this page.


[2024-10-22 17:15:05] [STRING]: Default API Found :https://api.wordpress.org

[2024-10-22 17:15:05] [STRING]: API Rerouted to :https://api.aspirecloud.org

[2024-10-22 17:15:07] [STRING]: Default API Found :https://api.wordpress.org/core/checksums/1.0/?version=6.6.2&locale=en_US

[2024-10-22 17:15:07] [STRING]: API Rerouted to :https://api.aspirecloud.org/core/checksums/1.0/?version=6.6.2&locale=en_US

[2024-10-22 17:15:14] [STRING]: Default API Found :https://api.wordpress.org/core/version-check/1.7/?version=6.6.2&php=8.1.29&locale=en_US&mysql=8.0.16&local_package=&blogs=1&users=1&multisite_enabled=0&initial_db_version=57155&extensions%5Bbcmath%5D=8.1.29&extensions%5Bbz2%5D=8.1.29&extensions%5Bcalendar%5D=8.1.29&extensions%5Bcgi-fcgi%5D=8.1.29&extensions%5BCore%5D=8.1.29&extensions%5Bctype%5D=8.1.29&extensions%5Bcurl%5D=8.1.29&extensions%5Bdate%5D=8.1.29&extensions%5Bdom%5D=20031129&extensions%5Bexif%5D=8.1.29&extensions%5BFFI%5D=8.1.29&extensions%5Bfileinfo%5D=8.1.29&extensions%5Bfilter%5D=8.1.29&extensions%5Bftp%5D=8.1.29&extensions%5Bgd%5D=8.1.29&extensions%5Bgettext%5D=8.1.29&extensions%5Bhash%5D=8.1.29&extensions%5Biconv%5D=8.1.29&extensions%5Bimagick%5D=3.7.0&extensions%5Bimap%5D=8.1.29&extensions%5Bintl%5D=8.1.29&extensions%5Bjson%5D=8.1.29&extensions%5Blibxml%5D=8.1.29&extensions%5Bmbstring%5D=8.1.29&extensions%5Bmysqli%5D=8.1.29&extensions%5Bmysqlnd%5D=mysqlnd+8.1.29&extensions%5Bopenssl%5D=8.1.29&extensions%5Bpcre%5D=8.1.29&extensions%5BPDO%5D=8.1.29&extensions%5Bpdo_mysql%5D=8.1.29&extensions%5Bpdo_sqlite%5D=8.1.29&extensions%5BPhar%5D=8.1.29&extensions%5Breadline%5D=8.1.29&extensions%5BReflection%5D=8.1.29&extensions%5Bsession%5D=8.1.29&extensions%5BSimpleXML%5D=8.1.29&extensions%5Bsoap%5D=8.1.29&extensions%5Bsodium%5D=8.1.29&extensions%5BSPL%5D=8.1.29&extensions%5Bsqlite3%5D=8.1.29&extensions%5Bstandard%5D=8.1.29&extensions%5Btidy%5D=8.1.29&extensions%5Btokenizer%5D=8.1.29&extensions%5Bxdebug%5D=3.1.5&extensions%5Bxml%5D=8.1.29&extensions%5Bxmlreader%5D=8.1.29&extensions%5Bxmlwriter%5D=8.1.29&extensions%5Bxsl%5D=8.1.29&extensions%5BZend+OPcache%5D=8.1.29&extensions%5Bzip%5D=1.19.5&extensions%5Bzlib%5D=8.1.29&platform_flags%5Bos%5D=WINNT&platform_flags%5Bbits%5D=64&image_support%5Bgd%5D%5B0%5D=webp&image_support%5Bgd%5D%5B1%5D=avif&image_support%5Bimagick%5D%5B0%5D=webp&image_support%5Bimagick%5D%5B1%5D=avif

[2024-10-22 17:15:14] [STRING]: API Rerouted to :https://api.aspirecloud.org/core/version-check/1.7/?version=6.6.2&php=8.1.29&locale=en_US&mysql=8.0.16&local_package=&blogs=1&users=1&multisite_enabled=0&initial_db_version=57155&extensions%5Bbcmath%5D=8.1.29&extensions%5Bbz2%5D=8.1.29&extensions%5Bcalendar%5D=8.1.29&extensions%5Bcgi-fcgi%5D=8.1.29&extensions%5BCore%5D=8.1.29&extensions%5Bctype%5D=8.1.29&extensions%5Bcurl%5D=8.1.29&extensions%5Bdate%5D=8.1.29&extensions%5Bdom%5D=20031129&extensions%5Bexif%5D=8.1.29&extensions%5BFFI%5D=8.1.29&extensions%5Bfileinfo%5D=8.1.29&extensions%5Bfilter%5D=8.1.29&extensions%5Bftp%5D=8.1.29&extensions%5Bgd%5D=8.1.29&extensions%5Bgettext%5D=8.1.29&extensions%5Bhash%5D=8.1.29&extensions%5Biconv%5D=8.1.29&extensions%5Bimagick%5D=3.7.0&extensions%5Bimap%5D=8.1.29&extensions%5Bintl%5D=8.1.29&extensions%5Bjson%5D=8.1.29&extensions%5Blibxml%5D=8.1.29&extensions%5Bmbstring%5D=8.1.29&extensions%5Bmysqli%5D=8.1.29&extensions%5Bmysqlnd%5D=mysqlnd+8.1.29&extensions%5Bopenssl%5D=8.1.29&extensions%5Bpcre%5D=8.1.29&extensions%5BPDO%5D=8.1.29&extensions%5Bpdo_mysql%5D=8.1.29&extensions%5Bpdo_sqlite%5D=8.1.29&extensions%5BPhar%5D=8.1.29&extensions%5Breadline%5D=8.1.29&extensions%5BReflection%5D=8.1.29&extensions%5Bsession%5D=8.1.29&extensions%5BSimpleXML%5D=8.1.29&extensions%5Bsoap%5D=8.1.29&extensions%5Bsodium%5D=8.1.29&extensions%5BSPL%5D=8.1.29&extensions%5Bsqlite3%5D=8.1.29&extensions%5Bstandard%5D=8.1.29&extensions%5Btidy%5D=8.1.29&extensions%5Btokenizer%5D=8.1.29&extensions%5Bxdebug%5D=3.1.5&extensions%5Bxml%5D=8.1.29&extensions%5Bxmlreader%5D=8.1.29&extensions%5Bxmlwriter%5D=8.1.29&extensions%5Bxsl%5D=8.1.29&extensions%5BZend+OPcache%5D=8.1.29&extensions%5Bzip%5D=1.19.5&extensions%5Bzlib%5D=8.1.29&platform_flags%5Bos%5D=WINNT&platform_flags%5Bbits%5D=64&image_support%5Bgd%5D%5B0%5D=webp&image_support%5Bgd%5D%5B1%5D=avif&image_support%5Bimagick%5D%5B0%5D=webp&image_support%5Bimagick%5D%5B1%5D=avif

@namithj
Copy link
Contributor

namithj commented Oct 22, 2024

Can you share the exact GET url to investigate further

@ipajen
Copy link
Collaborator Author

ipajen commented Oct 22, 2024

The GET call is going to https://wordpress.org and not https://api.wordpress.org
The debug is not logging the GET call in AP.

Not its in the wp-admin/site-health.php?tab=debug (Go to Health Check and click "Info")

its correctly rerouting the POST on https://api.wordpress.org/core/version-check/1.7/ to https://api.aspirecloud.org/core/version-check/1.7/

I wonder if it could be the GET is to provide the information is the health check:
Communication with WordPress.org | WordPress.org is reachable

@costdev
Copy link
Contributor

costdev commented Oct 26, 2024

@ipajen I wonder if it could be the GET is to provide the information is the health check:
Communication with WordPress.org | WordPress.org is reachable

That's exactly what it is. Ref

@asirota
Copy link
Member

asirota commented Oct 26, 2024

So is this a bug? @namithj @costdev

@ipajen
Copy link
Collaborator Author

ipajen commented Oct 26, 2024

If I use AspireUpdate, is there any case when want we communications should go to WP and not AP? Hopefully not but if so it’s important it’s include in the docs.

@namithj
Copy link
Contributor

namithj commented Oct 26, 2024

It’s not a bug but something we should look at later. I won’t consider it relevant for V1. Wordpress strangely checks if the api is available by making a get call to Wordpress.org instead of api.wordpress.org.

It probability bad programming or an attempt to figure out if someone is bypassing the actual API.

we need to decide on how important it is, and whether we need to do anything to handle this call.

@costdev
Copy link
Contributor

costdev commented Oct 26, 2024

Agreed. Not a bug and not necessary for 1.0.0.

This is a callout to dotorg essentially as a basic check that, in theory, the site will be able to connect to the dotorg APIs, and to the RSS feeds for the dashboard widget, for example. It doesn't pass any query arguments and it's just a check for 200 response.

For completeness, I do think this is worth considering so that Site Health can report if the API's host can be connected to, rather than the user only finding out on certain screens.

Similar to the other requests, this can be filtered using 'pre_http_request', and checking the $url argument for https://wordpress.org (possibly also checking $args['method'] for 'GET' for a more specific check), then replacing with the host.

Since the host may respond with an API error code, it may be worth checking for a specific range of response codes, rather than simply 200. We can work that out though.

Note also that a 'gettext_default' filter may need to be added in the 'pre_http_request' callback to replace 'WordPress.org' references with the host in the test result string. This can then be unhooked inside its own callback to minimize performance impact.

We can take some time to investigate and determine the best and simplest approach.

I'm happy to put together a proof of concept for this early this week to get us started, unless someone else wants to jump on in.

@costdev
Copy link
Contributor

costdev commented Oct 26, 2024

If I use AspireUpdate, is there any case when want we communications should go to WP and not AP? Hopefully not but if so it’s important it’s include in the docs.

Maybe for the Dashboard RSS widget for news and events if there's no AspireUpdate mechanism to provide an alternative, or no alternative is provided.

While this may suggest adding a separate Site Health test for the API, I think it's okay to replace the existing dotorg test rather, since whether the dashboard widget can contact dotorg isn't particularly important.

@namithj
Copy link
Contributor

namithj commented Oct 26, 2024

This is taken care of, we just proxy the request to w.org when it’s not relevant.

@costdev
Copy link
Contributor

costdev commented Oct 26, 2024

@namithj Thanks! Can you link to where this is being handled?

@namithj
Copy link
Contributor

namithj commented Oct 26, 2024

@costdev its part of AspireCloud behaviour, not handled in the plugin side of things

@costdev
Copy link
Contributor

costdev commented Oct 26, 2024

Ah gotcha, thanks!

@ipajen
Copy link
Collaborator Author

ipajen commented Nov 10, 2024

What about
change in health check : Communication with WordPress.org | WordPress.org is reachable
to: Communication with aspirepress.org | aspirepress.org is reachable

and make a setting in AU where its possible to disable the news/events widget. Like https://github.com/dimadin/disable-wordpress-events-and-news-dashboard-widget

@namithj
Copy link
Contributor

namithj commented Nov 10, 2024

We can easily change that API call to aspirepress but it's just a check whether the API is accessible strangely done to w.org. The real health check happens in api.w.org itself. When we implement the endpoint the health check will work as usual (not sure if we need to do that) until then it will be mirrored and work as usual.

The question is whether we should ping aspirepress.org like w.org is doing. It's silly, it should be hitting api.w.org and reading the response to check (check itself is not required as we will get the status from the actual api call). Changing it might be problematic as we won't have context in the empty api call. This needs thought.

Regarding removing the news widget, that would be a policy decision and not a technical decision and requires triage. Let's move it to a separate ticket.

@ipajen ipajen added the needs triage needs to be discussed and worked on label Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage needs to be discussed and worked on
Projects
None yet
Development

No branches or pull requests

4 participants