search

aspirepress / AspireUpdate

A plugin that allows for rewriting the URLs used to fetch updates from WordPress.org to some other endpoint
18 stars 5 forks source link

API call is wrongly Made to WP on reinstall WP #28

Open ipajen opened 3 days ago

ipajen commented 3 days ago

Tested on dev

  1. Set AP API point
  2. Goto /wp-admin/update-core.php
  3. Press reinstall Wordpress

Excepted: an api call is made to AP trying to download WP (and in current state would fail).

Actual: an api call is made to WP.org and the file is downloaded and reinstal WP. no call to AP is made

sarah-savage commented 3 days ago

Are you able to dig in and see where that call is made? Is the call made through the WP_Http class or something else?

We only rewrite rules that are passed through that class, so if they're made some other way (which wouldn't shock me) they won't be rewritten.

ipajen commented 3 days ago

Unfortunately I can’t see it in Query monitor when it’s updating and logs don’t save where the call is made from only calling Wp.

Before when using main bransch it called AP but now in dev it’s calling WP.

ipajen commented 3 days ago

For testing. Set XXX as end point and try reinstall. It’s installing instead of giving a error message of not found..

ipajen commented 3 days ago

Same issue with updating plugin. Is updating the plugin from Wp and not AP when testing dev bransch. But is making correct update check from wp_update_plugins() wp-includes/update.php:441 do_action('load-update-core.php') wp-includes/plugin.php:517 WordPress Core

namithj commented 6 hours ago

Did anyone manage to get to the root of this. Does it have anything to do with plugins not being loaded when Wordpress is getting updated?

sarah-savage commented 5 hours ago

I think that what is happening is one call is being made to us (to call the update info api) and then that info, passed through from WP, is directing them to download at .org. This is expected behavior.

ipajen commented 5 hours ago

behavior.

It you try https://github.com/aspirepress/AspireUpdate/issues/28#issuecomment-2423280465

@sarah-savage you should be enable reproduce the issue. With a incorrect host it should give a error but it dosent.

namithj commented 4 hours ago

@ipajen #35 is trying to resolve that issue.

ipajen commented 3 hours ago

@ipajen #35 is trying to resolve that issue.

Maybe I am missing something but #35 would not solve this issue. as it’s never triggering the rewrite and going to AP only to WP. To demonstrate it I was to change to a host that is not valid. Because if it was trigged it should give an error message and not going to WP. It worked before the refactoring in the old main…

namithj commented 3 hours ago

There is no place for us to trigger an error message in the workflow but we can make sure the API end points are genuine. The API calls are being made from a lot of different places and from cron jobs, we can’t tie all it up to a place in the UI to show those messages, atleast without a lot of custom work for each api call. But what we can easily do is to make sure the API end points do exists before we even save it and let the calls hit it.

ipajen commented 3 hours ago

It’s not about the error message it’s about the AP is not called and WP is call instead wrongly. And best way to test this is to have an invalid host. Until #35 is place then I need to find another way to demonstrate that wrong api is called. As if it was calling AP is should call the invalid host and not WP