Do not automatically try to enable index.php-less URLs #24539

Merged
merged 1 commit into from May 12, 2016

Projects

None yet

5 participants

@LukasReschke
Member
LukasReschke commented May 10, 2016 edited

The current logic for mod_rewrite relies on the fact that people have properly configured ownCloud, basically it reads from the overwrite.cli.ur l entry and then derives the RewriteBase from it.

This usually works. However, since the ownCloud packages seem to install themselves at /owncloud (because subfolders are cool or so…) a lot of people have just created a new Virtual Host for it or have simply symlinked the path etc.

This means that overwrite.cli.url is wrong, which fails hard if it is used as RewriteBase since Apache does not know where it should serve files from. In the end the ownCloud instance will not be accessible anymore and users will be frustrated. Also some shared hosters like 1&1 (because using shared hosters is so awesome… ;-)) have somewhat dubious Apache configurations or use versions of mod_rewrite from the mediveal age. (because updating is money or so…)

Anyhow. This makes this explicitly an opt-in configuration flag. If htaccess.RewriteBase is set then it will configure index.php-less URLs, if
admins set that after installation and don't want to wait until the next ownCloud version they can run occ maintenance:update:htaccess.

For ownCloud 9.0 we also have to add a repair step to make sure that instances that already have a RewriteBase configured continue to use it by copying it into the config file. That way all existing URLs stay valid. That one is not in this PR since this is unneccessary in master.

Effectively this reduces another risk of breakage when updating from ownCloud 8 to ownCloud 9.

Fixes #24525, #24426 and probably some more.

cc @karlitschek I'd propose a backport to stable9. I'll make sure to also add a repair step there so that for users that already use index.php-less URLs nothing changes.
cc @rullzer As discussed. Mind a review? THX a lot 🚀


stable9 at #24540

@LukasReschke LukasReschke Do not automatically try to enable index.php-less URLs
The current logic for mod_rewrite relies on the fact that people have properly configured ownCloud, basically it reads from the `overwrite.cli.ur
l` entry and then derives the `RewriteBase` from it.

This usually works. However, since the ownCloud packages seem to install themselves at `/owncloud` (because subfolders are cool or so…) _a lot_ of people have just created a new Virtual Host for it or have simply symlinked the path etc.

This means that `overwrite.cli.url` is wrong, which fails hard if it is used as RewriteBase since Apache does not know where it should serve files from. In the end the ownCloud instance will not be accessible anymore and users will be frustrated. Also some shared hosters like 1&1 (because using shared hosters is so awesome… ;-)) have somewhat dubious Apache configurations or use versions of mod_rewrite from the mediveal age. (because updating is money or so…)

Anyhow. This makes this explicitly an opt-in configuration flag. If `htaccess.RewriteBase` is set then it will configure index.php-less URLs, if
admins set that after installation and don't want to wait until the next ownCloud version they can run `occ maintenance:update:htaccess`.

For ownCloud 9.0 we also have to add a repair step to make sure that instances that already have a RewriteBase configured continue to use it by copying it into the config file. That way all existing URLs stay valid. That one is not in this PR since this is unneccessary in master.

Effectively this reduces another risk of breakage when updating from ownCloud 8 to ownCloud 9.

Fixes #24525, #24426 and probably some more.
86cc8ca
@LukasReschke LukasReschke added this to the 9.1-current milestone May 10, 2016
@karlitschek
Member

hmm. I get the fix. But this is fragile and close to impossible to test all scenarios.
Backport is fine with me but need some manual testing. 👍

@rullzer
Contributor
rullzer commented May 10, 2016

Yes. Should prevent a lot of unhappy users.
I tested and it does what I expect. 👍

But I'm no apache wizzard (or regular user) so other people please test as well.

@imjalpreet imjalpreet referenced this pull request May 11, 2016
Closed

Internal server error #24525

@imjalpreet
Member

Works! 👍

@DeepDiver1975 DeepDiver1975 merged commit 52add79 into master May 12, 2016

19 of 22 checks passed

core-ci-linux-swift-primary-storage/database=mysql,label=SLAVE Build #56029 failed in 1 min 8 sec
Details
server-master-linux-externals-ci/database=sqlite,external=swift-ceph,label=SLAVE Build #10173 found unstable in 10 min
Details
ocs-api-integration-tests-ci Build #10584 in progress...
Details
Scrutinizer 2 new issues, 3 updated code elements
Details
cla-bot-core Build #3913 succeeded in 34 min
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
core-ci-linux-jsunit/database=sqlite,label=SLAVE Build #61788 succeeded in 44 sec
Details
core-ci-linux/database=mysql,label=SLAVE Build #30487 succeeded in 38 min
Details
core-ci-linux/database=oci,label=SLAVE Build #30487 succeeded in 2 hr 57 min
Details
core-ci-linux/database=pgsql,label=SLAVE Build #30487 succeeded in 1 hr 24 min
Details
core-ci-linux/database=sqlite,label=SLAVE Build #30487 succeeded in 24 min
Details
server-master-linux-externals-ci/database=sqlite,external=smb-silvershell,label=SLAVE Build #10173 succeeded in 2 min 46 sec
Details
server-master-linux-externals-ci/database=sqlite,external=webdav-ownCloud,label=SLAVE Build #10173 succeeded in 12 min
Details
server-master-linux-externals-smb-windows-ext-ci/database=sqlite,external=smb-windows,label=master Build #14649 succeeded in 4 min 25 sec
Details
server-master-linux-php5.4-ci/database=sqlite,label=SLAVE Build #3499 succeeded in 4 min 43 sec
Details
server-master-linux-php7-ci/database=sqlite,label=SLAVE Build #38753 succeeded in 4 min 17 sec
Details
smashbox-on-docker-ci/DOCKER_IMAGE=ubuntu_oc_lamp-git,TEST_NAME=litmus,mirallBranch=v2.0.2,slave=SMASH Build #14520 succeeded in 6 min 37 sec
Details
smashbox-on-docker-ci/DOCKER_IMAGE=ubuntu_oc_lamp-git,TEST_NAME=test_basicSync@0,mirallBranch=v2.0.2,slave=SMASH Build #14520 succeeded in 9 min 47 sec
Details
smashbox-on-docker-ci/DOCKER_IMAGE=ubuntu_oc_lamp-git,TEST_NAME=test_shareLink,mirallBranch=v2.0.2,slave=SMASH Build #14520 succeeded in 42 min
Details
smashbox-on-docker-ci/DOCKER_IMAGE=ubuntu_oc_lamp-git,TEST_NAME=test_sharePermissions,mirallBranch=v2.0.2,slave=SMASH Build #14520 succeeded in 28 min
Details
smashbox-on-docker-ci/DOCKER_IMAGE=ubuntu_oc_lamp-git,TEST_NAME=test_sharePropagationGroups,mirallBranch=v2.0.2,slave=SMASH Build #14520 succeeded in 13 min
Details
smashbox-on-docker-ci/DOCKER_IMAGE=ubuntu_oc_lamp-git,TEST_NAME=test_sharePropagationInsideGroups,mirallBranch=v2.0.2,slave=SMASH Build #14520 succeeded in 47 min
Details
@DeepDiver1975 DeepDiver1975 deleted the i-for-one-welcome-my-fight-with-the-crappy-web-servers branch May 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment