nextcloud / updater

:arrows_counterclockwise: The updater app to keep your Nextcloud up-to-date
GNU Affero General Public License v3.0
45 stars 33 forks source link

Generate URLs via JavaScript #448

Closed kesselb closed 1 year ago

kesselb commented 1 year ago

Fix #265, Close #312

URL generation via PHP do not consider cases where the application URL at a reverse proxy is different from the local URL.

Example:

A reverse proxy forwards the request internally. As the updater is not installed in a subdirectory, the generated (via PHP) URLs are wrong. nextcloud/server has all the code to generate proper URLs for such cases. However, this code is not available here.

Generating the URLs via JavaScript is simpler as the browser is already visiting the right page.

To configure nginx-proxy to serve an application from a subdirectory for testing:

  - VIRTUAL_HOST=server2.internal
  - VIRTUAL_PATH=/cloud/
  - VIRTUAL_DEST=/
szaimen commented 1 year ago

/rebase

szaimen commented 1 year ago

@come-nc @blizzz eho has the rights to merge this?

blizzz commented 1 year ago

LGTM but didnt test

Would feel more confident if it was tested indeed ;)

szaimen commented 1 year ago

/rebase

fermulator commented 1 year ago

I will test this weekend.

fermulator commented 1 year ago

TEST RESULTS:

Caveats/Notes:

Pulling in Changes:

cd /srv/nextcloud/updater
mv index.php index.php.bak
wget https://raw.githubusercontent.com/nextcloud/updater/94058243a64fbac38685b69cd077fbccd7d4d935/index.web.php
mv index.web.php index.php
sudo chown www-data:www-data index.php
sudo chmod 644 index.php
sudo setfacl -b index.php

Then visit: https://NEXTCLOUD-URI/cloud/index.php/settings/admin/overview# , click on "Open Updater" which redirects to https://NEXTCLOUD-URI/cloud/updater/

❌ EMPTY / broken (the page is just a white/empty page)

If I restore the original version ... the updater page loads as expected.

Is there some sort of breaking changes on newer versions of the updater that would prevent it from working on an older core Nextcloud version? Potentially a compatibility issue here.

fermulator commented 1 year ago

Next Attempt: (try see if there is indeed a compatibility issue ...)

Conducted a few upgrades using the manual workaround steps to get to a supported version. (and sometimes :/srv/nextcloud/updater$ sudo -u www-data php updater.phar)

Current version is 23.0.7.
Update to Nextcloud 23.0.12 available. (channel: "stable")

✔️

Current version is 23.0.12.
Update to Nextcloud 24.0.8 available. (channel: "stable")

✔️

^ so there, up to v24 (currently supported)

fermulator commented 1 year ago

Repeating test:

steps noted in https://github.com/nextcloud/updater/pull/448#issuecomment-1356354883

[Nextcloud (Redbrick)](NEXTCLOUD-URI)24.0.8
A new version is available: Nextcloud 25.0.2

ends up with:

fermulator@nextcloud:/srv/nextcloud/updater$ ll
total 848
drwxrwx---+  2 www-data www-data   4096 Dec 17 16:13 ./
drwxrwx---+ 14 www-data www-data   4096 Dec 17 13:29 ../
-rw-r--r--   1 www-data www-data  32208 Dec 17 16:13 index.php
-rw-r--r--   1 www-data www-data  66320 Dec 17 13:29 index.php.bak
-rw-r--r--   1 www-data www-data  32208 Dec 17 12:50 index.php.new
-rw-r--r--   1 www-data www-data 723250 Dec 17 13:29 updater.phar

same problem ❌

(in my experience/past, this does usually mean there's a bug in the PHP/javascript/HTML somewhere ... hard to pinpoint)

fermulator commented 1 year ago

tried to enable error reporting but it isn't having any affect (tried index.html, index.php, and updater/index.php) https://phoenixnap.com/kb/php-error-reporting

AH-HA ... wow ... so i was adding all this at the TOP of the file, but apparently we're randomly disabling it on line ~120

ini_set('display_errors', '0');
ini_set('log_errors', '1');

grrr

fermulator commented 1 year ago

https://docs.nextcloud.com/server/latest/admin_manual/configuration_server/config_sample_php_parameters.html#logging

OK here we go

tail -f /var/log/apache2/error.log

[Sat Dec 17 21:42:21.995174 2022] [php7:error] [pid 27624] [client 10.0.0.52:60376] PHP Fatal error:  Uncaught Error: Class 'Updater' not found in /srv/nextcloud/updater/index.php:121\nStack trace:\n#0 {main}\n  thrown in /srv/nextcloud/updater/index.php on line 121
fermulator commented 1 year ago

Last test for the day ... because of the Updater() class error there, I'm thinking indeed there is an incompatibility with this version of index.web.php with the version I'm running.

SO -- I manually patched my index.php (from version 24.0.8), with the changeset proposed by this PR; (the file is different enough that I couldn't do a git patch) -- took each diff, found it's respectively section, and applied

Current version is 24.0.8.
Update to Nextcloud 25.0.2 available. (channel: "stable")

Then visit: https://nextcloud-uri/cloud/index.php/settings/admin/overview# , click on "Open Updater" which redirects to https://nextcloud-uri/cloud/updater/

✔️ it worked! nextcloud-updater-patched nextcloud-updater-success

fermulator commented 1 year ago

/rebase

fermulator commented 1 year ago

@PVince81 - for your final review

fermulator commented 1 year ago

one other note. someone maybe should test to ensure this works in non-proxy scenario

fermulator commented 1 year ago

what's the hold up? now that we have a viable fix, we should really target this for delivery ASAP