Workaround to check htaccess in case of redirects

[PVince81]

In some setups, the web server will redirect any call to "data/" to the
main page. This causes the XHR to return the 200 HTTP status code and
the body contains the HTML page of the main page / files app.

This fix improves the htaccess failure detection by adding a known
string inside the test file "htaccesstest.txt". If we are able to find
this string, it means that the web server didn't block access to that
file.

Reported by greysun on IRC.

Please review @RealRancor @LukasReschke @danimo @DeepDiver1975 @guruz @georgehrke

Would be good to backport this to 9.1 and 9.0 to avoid annoying false positives.

[ghost]

Mhhh, why adding the HTACCESSFAIL:? There is already a unique and known test within the htaccesstest.txt

[PVince81]

@RealRancor because the JS side needs to check whether the 200 response contains that string.
There are some cases where a 200 OK is valid whenever the server redirects from "data/htaccesstest.txt" to "apps/files". In this case we do get a response, but it's not the file itself.

This happens in greysun's environment. We can't detect 302 redirects with XHR/ajax as the browser will automatically follow redirects.

[PVince81]

@RealRancor also I didn't want to copy the whole exact test text to the JS side in case someone decides to change it in the future... a bit risky to expect an exact match.

[LukasReschke]

Nextcloud already has a patch for this in all releases. Didn't test the ownCloud patch but it looks pretty similar and should do it's work. 👍 for the approach itself.

Ref nextcloud/server#91
Ref nextcloud/server#92

[PVince81]

@LukasReschke I think there's a slight risk if one day someone changes the message in one place but not in the other. In this case people might get false positives that their server is safe when it's not.

That why I opted for adding "HTACCESSTEST" instead with a comment. Also this has the advantage that the message is only in one place.

[PVince81]

also: slight risk in case of newlines, if fwrite would add one, not sure if it does...

[PVince81]

• TODO: adjust unit test

[ghost]

also I didn't want to copy the whole exact test text to the JS side in case someone decides to change it in the future... a bit risky to expect an exact match.

Ahhh, i didn't know that. My question above was based on the assumption that you could only match partially for e.g. This is used for testing whether htaccess.

When thinking about it the HTACCESSFAIL is also a better approach as the changes that this text got changed is less likely.

[ghost]

Fixes #25416

[PVince81]

This was tested by greysun on IRC, counting as 👍

[PVince81]

stable9.1: #25434
stable9: #25435

[ghost]

Also confirmed to work here: https://forum.owncloud.org/viewtopic.php?f=38&t=37580#p119487

[PVince81]

Damn, I didn't see that JS tests were failing.

Will send a follow up PR

[PVince81]

Here we go #25439

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.