implement getUpdaterUrl to handle reverse proxy configurations

[fermulator]

As per #265 - #265 (comment) @kesselb suggestion

These changes

1. remove updaterUrl logic (which was previously set in MAIN BODY) in a new function entitled: getUpdaterUrl() with the updater class
2. add support for adjusting the updaterUrl from getConfigOption('overwritewebroot')
3. add [info] and [debug] logging here where appropriate/helpful

Test Results:

1. Logs:

tail -f /opt/nextcloud/updater.log

2020-11-16T20:00:01-0500 Fzl86ny556 [info] getUpdaterUrl()
2020-11-16T20:00:01-0500 Fzl86ny556 [debug] getUpdaterUrl() initial from _SERVER array: /updater/
2020-11-16T20:00:01-0500 Fzl86ny556 [debug] getUpdaterUrl() after trimming and index.php suffix: /updater/index.php
2020-11-16T20:00:01-0500 Fzl86ny556 [info] getUpdaterUrl() detected overwrite web root configuration >/cloud<, updating for reverse proxy
2020-11-16T20:00:01-0500 Fzl86ny556 [info] getUpdaterUrl() final: /cloud/updater/index.php

2. verified locally that this version of index.php (from my branch) correctly handles the updater links (which I had previously manually verified in Webbased Nextcloud updater fails in reverse proxy scenario with / vs. /something #265 (comment)

updater-endpoint == value="/cloud/updater/index.php"
go back button == href=/cloud/updater/../

[MorrisJobke]

Code looks good 👍

[MorrisJobke]

CI fails because Nextcloud master does not install on PHP 7.2 anymore (7.3+ is needed). But the other CI tests look good. Let me look into those tests later.

[MorrisJobke]

@kesselb Any opinion on this?

[kesselb]

Looks good to me 👍

I wonder if we need the new method at all.

updater/index.php

Line 1875 in 1ed9c8e

httpRequest.open('POST', document.getElementById('updater-endpoint').value);

We could also use window.location.href here? Afaik the request is always sent to self so the browser should know the correct url already. But I don't know the web based updater at all ;)

[fermulator]

CI fails because Nextcloud master does not install on PHP 7.2 anymore (7.3+ is needed). But the other CI tests look good. Let me look into those tests later.

@MorrisJobke - I presume there would be a separate ticket for this? - I agree it needs to be fixed

[fermulator]

Looks good to me +1

I wonder if we need the new method at all.

updater/index.php

Line 1875 in 1ed9c8e

httpRequest.open('POST', document.getElementById('updater-endpoint').value);

We could also use window.location.href here? Afaik the request is always sent to self so the browser should know the correct url already. But I don't know the web based updater at all ;)

@kesselb - I am not sure myself about it - if you think it is worth pursuing we could fork/split a ticket/issue; (I would say it is out of scope from this change)

EDIT: I might see how it is in scope; if you're saying there is a different approach recommended, I can look into it. (though, I guess it was you yourself who suggested this initial approach ;o #265 (comment))

[fermulator]

@MorrisJobke @kesselb - are we doing this? (not sure what the expectations are for PR rework/acceptance ;o)

[fermulator]

?

[kesselb]

Mind to test the self version?

[fermulator]

Mind to test the self version?

🤷 (out of scope for now I'd say)

EDIT: to clarify, i don't know what the "self version" is

[fermulator]

@MorrisJobke @kesselb "poke" :( sorry!

[kesselb]

updater/index.php

Line 1875 in 1ed9c8e

httpRequest.open('POST', document.getElementById('updater-endpoint').value);

We could also use window.location.href here? Afaik the request is always sent to self so the browser should know the correct url already. But I don't know the web based updater at all ;)

Afaik that's the right way.

[fermulator]

updater/index.php

Line 1875 in 1ed9c8e

httpRequest.open('POST', document.getElementById('updater-endpoint').value);

We could also use window.location.href here? Afaik the request is always sent to self so the browser should know the correct url already. But I don't know the web based updater at all ;)

Afaik that's the right way.

How? window.location.href is javascript, we're in PHP land in the web updater. I don't think it is worth holding up this merge over.

[kesselb]

How?

Please make a backup of the current version just in case.

First revert the php changes.

And then change httpRequest.open('POST', document.getElementById('updater-endpoint').value); to httpRequest.open('POST', window.location.href);

Thanks 👍

[fermulator]

OK! (finally attention to this ... sorry)

@kesselb

(current baseline: [Nextcloud (snip)] 22.2.9, A new version is available: Nextcloud 23.0.6)

1. /srv/nextcloud/updater/index.php (changes are auto-reverted on nextcloud updates, and I've done a few manually using the working since this work)
2. Edited per your suggestion. httpRequest.open('POST', window.location.href);
3. Open updater (view overview/settings) https://cloud/updater/
4. DO NOT apply any manual workaround/tweaks.
5. Initiate update "Start Update" button
6. tail -f /opt/nextcloud/updater.log (monitor ...)
7. after pre-steps, we get to "Continue with web based updater" (BUTTON CLICK)
8. << HERE IS THE TEST >> --- FAIL (it redirected me to https:/// ...) - had to manually suffix /cloud to it.

[kesselb]

Thanks for testing 👍

Your output looks good to me. The initial issue was something like

Receive "Check for expected files: Parsing response failed. […]"

because the post request to trigger the update went to the wrong path. If I understand you correct the files are replaced but the redirect back to Nextcloud failed. That's a related but different story. I guess we have to adjust some more javascript here.

[fermulator]

OK I can retest w/ more attention to detail (TBH I was multi-tasking) -
any particular tips for DEBUG/analysis of the latter issue?

[fermulator]

For this PR I also think there is value in retaining some of the PHP refactoring.

[fermulator]

In PR #448, @kesselb has a different proposal for solution (which may supersede this PR)