Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redirecting of /apps/contacts/carddav.php, /apps/calendar/caldav.php not working with separate apps-directory #243

Closed
svolke opened this issue Nov 4, 2012 · 28 comments

Comments

@svolke
Copy link

svolke commented Nov 4, 2012

When checking out owncloud from the github repositories and having the apps repo in the apps2 directory,
using carddav and caldav via the "old" default uri /apps/contacts/carddav.php and /apps/calendar/caldav.php does not work.
I get a SabreDAV-error:

  • Sabre_DAV_Exception_Forbidden
  • Requested uri (/apps/contacts/carddav.php) is out of base uri (/remote.php/carddav/)

Steps to reproduce:

@svolke
Copy link
Author

svolke commented Nov 4, 2012

The problem is: when doing the redirect, the $baseuri of SabreDAV needs to be adjusted.
When accessing via /remote.php/service/, the $baseuri is set to OC::$WEBROOT . '/remote.php/'.$service.'/'

For a direct access via OC_App::getAppWebPath('contacts').'/carddav.php') (i.e. /apps2/contacts/carddav.php) the $baseuri is set in contacts/appinfo/remote.php (and likewise for calendar app).
When accessing via /apps/contacts/carddav.php (! not apps2!), the wrong $baseuri is set and that case is not handled within the app.

@svolke
Copy link
Author

svolke commented Nov 4, 2012

The following patch fixes the issue (and also makes the $baseuri robust against an aliased base URL):

diff --git a/calendar/appinfo/remote.php b/calendar/appinfo/remote.php
index 9a43161..d7b29d6 100644
--- a/calendar/appinfo/remote.php
+++ b/calendar/appinfo/remote.php
@@ -8,7 +8,11 @@
 OCP\App::checkAppEnabled('calendar');

 if(substr($_SERVER["REQUEST_URI"],0,strlen(OC_App::getAppWebPath('calendar').'/caldav.php')) == OC_App::getAppWebPath('calendar'). '/caldav.php') {
-       $baseuri = OC_App::getAppWebPath('calendar').'/caldav.php';
+       $baseuri = OC::$WEBROOT . OC_App::getAppWebPath('calendar').'/caldav.php';
+}
+if(substr($_SERVER["REQUEST_URI"],0,strlen('/apps/calendar/caldav.php')) == '/apps/calendar/caldav.php') 
+{
+       $baseuri = OC::$WEBROOT . '/apps/calendar/caldav.php';
 }

 // only need authentication apps
diff --git a/contacts/appinfo/remote.php b/contacts/appinfo/remote.php
index d8677d7..7590c3d 100644
--- a/contacts/appinfo/remote.php
+++ b/contacts/appinfo/remote.php
@@ -23,7 +23,10 @@
 OCP\App::checkAppEnabled('contacts');

 if(substr($_SERVER["REQUEST_URI"], 0, strlen(OC_App::getAppWebPath('contacts').'/carddav.php')) == OC_App::getAppWebPath('contacts').'/carddav.php') {
-       $baseuri = OC_App::getAppWebPath('contacts').'/carddav.php';
+       $baseuri = OC::$WEBROOT . OC_App::getAppWebPath('contacts').'/carddav.php';
+}
+if(substr($_SERVER["REQUEST_URI"], 0, strlen('/apps/contacts/carddav.php')) == '/apps/contacts/carddav.php') {
+       $baseuri = OC::$WEBROOT . '/apps/contacts/carddav.php';
 }

 // only need authentication apps

@ghost ghost assigned georgehrke and tanghus Nov 4, 2012
@tanghus
Copy link
Contributor

tanghus commented Nov 5, 2012

@eMerzh I can see you've worked with this some months ago. Can you verify the patch?

@eMerzh
Copy link
Member

eMerzh commented Nov 5, 2012

I don't really understand why we want to do

 $baseuri = OC::$WEBROOT . OC_App::getAppWebPath('contacts').'/carddav.php';

as getAppWebPath should already contains the webroot.

also i don't get why we would access /apps/contacts/carddav.php instead of /apps2/contacts/carddav.php

@georgehrke
Copy link
Contributor

we support /apps/contacts/carddav.php for legacy reasons - might be good to remove it with the next major release

@svolke
Copy link
Author

svolke commented Nov 5, 2012

Concerning the OC::$WEBROOT: I did not know, that getAppWebPath already contained this. So this part of the patch is wrong....

As for /apps/contacts/carddav.php: this URL is hardcoded in a lot of applications, ranging from kdepim to the carddav/caldav-synchronization apps for android. That is also the reason of the redirect in .htaccess.
It should be supported to allow proper transition within these apps.

In any case: the redirect is useless without the patch, when a separate apps-directory is used and that should be fixed.

@eMerzh
Copy link
Member

eMerzh commented Nov 5, 2012

cann't we do a rewrite rule or something?
because i don't see how the server will fetch /apps2/ if it's /apps/ that is requested .

The best solution is to totaly remove those /apps* path and access it through OC like remote, ....
something like http://host/index.php/apps/contacts/carddav.php

@georgehrke
Copy link
Contributor

RewriteRules are only working with apache

@tanghus
Copy link
Contributor

tanghus commented Nov 5, 2012

Can we send a 301, permanently moved header from /apps/contacts/carddav.php and /apps/calendar/caldav.php?

@svolke
Copy link
Author

svolke commented Nov 6, 2012

I can't follow your discussion for the moment....

Historically, the CalDAV/CardDAV access was done via http://host/apps/contacts/carddav.php.
Then it got moved and the official access URL is now http://host/remote.php/carddav/
This is more generic and was a good decision.
A rewrite rule has been installed to .htaccess to internally redirect http://host/apps/contacts/carddav.php to http://host/remote.php/carddav/
Still, a direct access via http://host/apps/contacts/carddav.php is possible and should be to allow for external app transition.

Problem is: this rewrite rule doesn't work anymore when moving the contacts app to another directory.
The already existing $baseuri rewriting allowed the redirect to work when the contacts app was still at its old place.
Now, its purpose changed and it allows direct access via http://host/apps2/contacts/carddav.php.
The lines, that I added, make the redirect work again.

@tanghus
Copy link
Contributor

tanghus commented Nov 6, 2012

@svolke As far as I understand the .htaccess redirect only works for Apache. Hence I ask - without any deep knowledge of the HTTP protocol - if a header redirect to remote/[caldav|carddav] won't work?

Having a bit of trouble following the discussion as well ;)

@svolke
Copy link
Author

svolke commented Nov 6, 2012

@thangus The .htaccess works only with Apache. That is right. That doesn't change the fact, that it should work!

Of course a header redirect could be done. But wouldn't that stop e.g. the android sync app stop from working?
I thought, the only reason of the (now non-working) redirect was to achieve both: transition to generic URL and support of legacy applications.

For all I know, using apache is still the default way of running owncloud. When using NGinx, you have to go through some work to enhance security and stuff, that would otherwise be done via .htaccess, in any case.

@tanghus
Copy link
Contributor

tanghus commented Nov 6, 2012

I was a bit premature when I assigned the issue to me I must admit. @georgehrke @bartv2 and @eMerzh have much more knowledge about this area. ownCloud "should" work with all web servers, and using .htaccess is afaik only a means of making it work for most web servers.
Android sync shouldn't be affected - at least it isn't with my installations (Homeycomb/ICS).
I will wait for comments from the more knowledgeable supporters here ;)

@georgehrke
Copy link
Contributor

I guess this issue is obsolete, as all remote services are piped thru /remote.php. Am i right?

@bartv2
Copy link
Contributor

bartv2 commented Feb 8, 2013

this is about legacy urls, what to do with those. We supported both in 4.5, +1 for removing support in OC5

@tanghus
Copy link
Contributor

tanghus commented Feb 8, 2013

KDE Akonadi 4.10 just released still uses legacy urls for ownCloud :-/

@tanghus
Copy link
Contributor

tanghus commented Feb 11, 2013

this is about legacy urls, what to do with those. We supported both in 4.5, +1 for removing support in OC5

And they don't work anymore in master. Unless someone's volunteering to fix the legacy support I agree with @bartv2 that it might as well be dropped.

@karlitschek
Copy link
Contributor

This legacy urls are no longer supported.

@tanghus
Copy link
Contributor

tanghus commented Feb 28, 2013

@karlitschek Maybe you have more kudos here than I https://bugs.kde.org/show_bug.cgi?id=314711
;)

@karlitschek
Copy link
Contributor

@tanghus I confirmed this in the KDE bugtracker. Let's hope one of the Akonadi developers can look into that. :-)

@tanghus
Copy link
Contributor

tanghus commented Feb 28, 2013

@karlitschek yeah I saw. Thanks :)

@karlitschek
Copy link
Contributor

Let's hope someone fixes it :-)

@AdamWill
Copy link
Contributor

Heh, I just ran into what looks like exactly this. OC 8 is still carrying the rewrite rules in its shipped .htaccess file, despite the above discussion about dropping them. So I decided to check if they actually work for the Fedora package I maintain, and they don't - I hit this issue.

Indeed the 'contacts' and 'calendar' apps are in a separate apps directory, because we keep main ownCloud in /usr/share/owncloud but it is not allowed to write there, so app store apps are downloaded to /var/lib/owncloud/apps , which is configured as an extra apps directory with the URI apps-appstore. So that sounds very much like this bug.

If you really don't want to support the legacy URLs any more, maybe drop the .htaccess entries and the workaround in calendar / contacts appinfo?

@DeepDiver1975
Copy link
Member

Wow - thanks Adam. Can I ask you to send pull requests to fix this? Thx

@AdamWill
Copy link
Contributor

If by 'fix' you mean 'drop', sure :)

@brianjmurrell
Copy link

@AdamWill Did you ever send a pull request for this? I was redirected here by the comment in /etc/httpd/conf.d/owncloud-defaults.inc in the EPEL 7 packaging.

Would be good to get this loose end tied up.

@AdamWill
Copy link
Contributor

nope, sorry. i woke up one day and forswore ever having anything to do with this giant ball of PHP-branded pain ever again, so I don't maintain it or have any interest in doing anything to it any more. :P

@lock
Copy link

lock bot commented Aug 4, 2019

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.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants