Skip to content

Conversation

mholt
Copy link
Member

@mholt mholt commented Feb 1, 2016

Some fastcgi backends don't use standard HTTP verbs, especially file-management-related software and CalDAV, etc. Basically, this change treats any other request types as a POST request but with the method name carried through.

@hubertbanas and @TribuneX - could either/both of you verify if this fix works for you? Let me know if you need help building from source -- or just ask on gitter dev chat.

This is a start for fixing #497 but also any WebDAV stuff in general, I think.

@mholt mholt mentioned this pull request Feb 1, 2016
@DenBeke
Copy link

DenBeke commented Feb 8, 2016

Regarding ownCloud: it could successfully connect with WebDav, but now the owncloud client gives the following error:

Error downloading https://domain.com/remote.php/webdav/ - server replied: Not Found (Path not found)

In the access log I got:

192.168.0.1 - [08/Feb/2016:22:07:46 +0100] "PROPFIND /remote.php HTTP/1.1" 404 269

WebDav problem, or rewrite rules problem?

@mholt
Copy link
Member Author

mholt commented Feb 23, 2016

I'm not sure. Somewhere between WebDAV and Caddy the status code is getting lost or Caddy thinks the status code is 200 when it should be one of those weird ones like 207. I've got my hands full at the moment but somebody please look into this if you can. 😅

@dstapp
Copy link

dstapp commented Mar 6, 2016

As far as I can see the "Not Found" issue refers to some strange behavior with the Caddy/FastCGI combination I was not able find out...

Assuming you have fix_pathinfo=1 (which you'll need for ownCloud to work but this is not more an security concern today...), running the following requests leads to the following PATH_INFO values:

/remote.php/webdav => /webdav
/remote.php/webdav/ => webdav/

As you can see, the leading slash is gone, which is strange as this does not happen when using Apache and mod_php. I didn't reproduce this on nginx but I think it's worth giving it a try as this may need to be fixed in the future anyway...

Because of the crappy ownCloud code that "tries" to sanitize the PATH_INFO content this way:

$request = \OC::$server->getRequest();
        $pathInfo = $request->getPathInfo();
        if ($pathInfo === false || $pathInfo === '') {
                throw new RemoteException('Path not found', OC_Response::STATUS_NOT_FOUND);
        }
        if (!$pos = strpos($pathInfo, '/', 1)) {
                $pos = strlen($pathInfo);
        }
        $service=substr($pathInfo, 1, $pos-1);

What happens is: $service has the value ebdav which leads to the RemoteException "Path not found".

@DenBeke
I worked around this issue by ignoring the trailing / when rewriting. Please take a look into my Caddyfile and try if it works for you now:

https://gist.github.com/dprandzioch/1aab06b82797acdaeab0

I also read your PR for the "ownCloud on Caddy" blogpost... Please keep in mind that because the ownClouds data/ is within the docroot, the way your Caddyfile works, exposes all private user contents without authorization to anyone... Feel free to use my Caddyfile to include at least this:

rewrite {
    r  ^/(?:\.htaccess|data|config|db_structure\.xml|README)
    status 403
}

To ensure noone has access to the data/ directory.

And now.. let's go and take this upstream, I'm looking forward to set up ownCloud on Caddy the first time ;-)

@DenBeke
Copy link

DenBeke commented Mar 6, 2016

@dprandzioch this works perfectly!

@mholt mholt changed the title fastcgi: Accept any other methods as a POST-style request (fixes #497) fastcgi: Accept any other methods as a POST-style request Mar 7, 2016
# Conflicts:
#	middleware/fastcgi/fastcgi.go
@mholt mholt merged commit 741d768 into master Mar 8, 2016
@mholt
Copy link
Member Author

mholt commented Mar 8, 2016

Thanks @dprandzioch for your insights! And @DenBeke for the feedback/experimentation. This is an overall improvement I think, so I've merged it in.

@mholt mholt deleted the fastcgi-methods branch March 8, 2016 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants