Fix #26869 honor overwritewebroot and PATH_INFO #26882

Open
wants to merge 2 commits into
from

Conversation

Projects
None yet
6 participants
@aheusingfeld

aheusingfeld commented Dec 28, 2016

Description

Fixes #26869: honor overwritewebroot config property and PATH_INFO in request handling so that owncloud can be run under a contextroot such as /owncloud or /cloud

Related Issue

#26869

Motivation and Context

see description.

How Has This Been Tested?

Please check the description and comments in #26869. I applied these changes to my owncloud 9.1 + nginx 1.10 installation in order to make it work under a contextroot of /cloud -> I tested this change on a production installation.

As my PHP is rather rusty and I'm not too familiar with how owncloud is tested, I didn't adjust https://github.com/owncloud/core/blob/37177dd54d67443cacc0061c17f95070442b93cf/tests/lib/AppFramework/Http/RequestTest.php though this is definitely necessary as I couldn't spot any tests with a contextroot!

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

I will send another pull-request with an update to https://doc.owncloud.org/server/9.0/admin_manual/installation/nginx_examples.html#owncloud-in-a-subdir-of-nginx so that other users don't have to read #26869 but can get the necessary information there.

Fix #26869 honor overwritewebroot and PATH_INFO
Fixes #26869: honor overwritewebroot config property and PATH_INFO in request handling so that owncloud can be run under a contextroot such as /owncloud or /cloud
@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Dec 28, 2016

@aheusingfeld, thanks for your PR! By analyzing the history of the files in this pull request, we identified @DeepDiver1975, @tanghus and @bartv2 to be potential reviewers.

@aheusingfeld, thanks for your PR! By analyzing the history of the files in this pull request, we identified @DeepDiver1975, @tanghus and @bartv2 to be potential reviewers.

@CLAassistant

This comment has been minimized.

Show comment
Hide comment
@CLAassistant

CLAassistant Dec 28, 2016

CLA assistant check
All committers have signed the CLA.

CLAassistant commented Dec 28, 2016

CLA assistant check
All committers have signed the CLA.

@aheusingfeld aheusingfeld referenced this pull request in owncloud/documentation Dec 28, 2016

Merged

Updated documentation regarding changes in #26869 #2787

@aheusingfeld

This comment has been minimized.

Show comment
Hide comment
@aheusingfeld

aheusingfeld Jan 3, 2017

Hey there, any feedback is highly appreciated.

Hey there, any feedback is highly appreciated.

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Jan 9, 2017

Member

@DeepDiver1975 please review, this is very-core stuff

Member

PVince81 commented Jan 9, 2017

@DeepDiver1975 please review, this is very-core stuff

@PVince81 PVince81 added this to the 10.0 milestone Jan 9, 2017

@DeepDiver1975

This comment has been minimized.

Show comment
Hide comment
@DeepDiver1975

DeepDiver1975 Jan 18, 2017

Member

What could possible go wrong 😉

Member

DeepDiver1975 commented Jan 18, 2017

What could possible go wrong 😉

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 31, 2017

What could possible go wrong

It seems the PATH_INFO stuff needs to be handled with care. In NC they have even removed the PATH_INFO stuff in that file: nextcloud/server#984 as it caused constant page refresh like also reported in core: #27053

ghost commented Jan 31, 2017

What could possible go wrong

It seems the PATH_INFO stuff needs to be handled with care. In NC they have even removed the PATH_INFO stuff in that file: nextcloud/server#984 as it caused constant page refresh like also reported in core: #27053

@PVince81

This comment has been minimized.

Show comment
Hide comment
Member

PVince81 commented Mar 22, 2017

@DeepDiver1975 @butonic @phisch @IljaN can we get this in ?

@PVince81

This comment has been minimized.

Show comment
Hide comment

@PVince81 PVince81 modified the milestones: 10.0.1, 10.0 Apr 26, 2017

@PVince81 PVince81 modified the milestones: 10.0.2, 10.0.1 May 17, 2017

@PVince81 PVince81 modified the milestones: 10.0.2, 10.0.3 May 26, 2017

@PVince81 PVince81 modified the milestones: triage, development Aug 7, 2017

@PVince81 PVince81 modified the milestones: triage, maybe some day May 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment