You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Stefan Fritsch <sf...@sfritsch.de> on 2010/01/29 08:42:41 UTC
mod_dav inconsistent behaviour for GET requests
Hi,
mod_dav doesn't handle GET requests in a consistent way: If a repos
provider has handle_get == 1, mod_dav will handle GET requests by
itself. This means no other handler will get a chance to interpret the
file as script or SSI. On the other hand, if the repos provider has
handle_get == 0, mod_dav will do nothing and another handler like
mod_php or mod_include may handle scripts, causing the output of the
script execution to be sent to the client instead of the script
source.
The documentation
http://httpd.apache.org/docs/trunk/mod/mod_dav.html#complex suggests
using ForceType 'text/plain' to circumvent this. Apart from the fact
that this is not enough and one may need 'SetHandler none' as well, I
think this is a kludge. mod_dav should make sure that the GET request
is handled by the default handler by setting r->handler in its fixup
hook. If nobody disagrees, I will change this in trunk.
Cheers,
Stefan
Re: mod_dav inconsistent behaviour for GET requests
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Sunday 31 January 2010, Justin Erenkrantz wrote:
> On Sun, Jan 31, 2010 at 3:01 AM, Stefan Fritsch <sf...@sfritsch.de>
wrote:
> > On Sat, 30 Jan 2010, Justin Erenkrantz wrote:
> >> I don't see how your patch would "intentionally" break - the
> >> failure mechanism is that the source scripts are served - not
> >> that a configuration error stops the server from running. --
> >> justin
> >
> > Surely a fatal server error is not a necessary condition for
> > something to be broken?
>
> When it has the capability of exposing source that would not
> otherwise be served, absolutely.
>
> The fundamental flaw with this patch is that dav_fixups runs after
> core_override_type - so the "none" handler simply won't get
> reassigned by the rest of the applicable configs - ie set to CGI
> or PHP or whatnot. So, it would simply fall through and go to the
> default handler. Ouch. -- justin
That's exactly what the patch is supposed to do. Therefore I would not
call it flawed.
I think that the auth changes from 2.2 to trunk are so large that
anyone upgrading will have to carefully review and test his
configuration for security problems anyway. An additional change in
the behaviour of mod_dav wouldn't create much of an additional problem
(if it is documented correctly).
But since quite a few people disagree with me here, an alternative
could be an additional directive (or second argument to 'Dav') that
allows to configure the behaviour. For example
DavHandleMethods all
DavHandleMethods exceptPOST
DavHandleMethods exceptGET,POST
For 2.4, one could then leave the default at exceptGET,POST /
exceptPOST (depending on the dav provider), just like it is for 2.2.x.
But if the user does not specify DavHandleMethods explicitly, httpd
could log a notice saying:
"DavHandleMethods defaulting to 'exceptGET,POST'. The default will
change to 'all' with the next major release of httpd. Please specify
DavHandleMethods explicitly."
Or one could even make the new directive mandatory, with httpd
refusing to start without it.
Would this address your concerns?
Cheers,
Stefan
Re: mod_dav inconsistent behaviour for GET requests
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Sun, Jan 31, 2010 at 3:01 AM, Stefan Fritsch <sf...@sfritsch.de> wrote:
> On Sat, 30 Jan 2010, Justin Erenkrantz wrote:
>>
>> I don't see how your patch would "intentionally" break - the failure
>> mechanism is that the source scripts are served - not that a
>> configuration error stops the server from running. -- justin
>
> Surely a fatal server error is not a necessary condition for something to be
> broken?
When it has the capability of exposing source that would not otherwise
be served, absolutely.
The fundamental flaw with this patch is that dav_fixups runs after
core_override_type - so the "none" handler simply won't get reassigned
by the rest of the applicable configs - ie set to CGI or PHP or
whatnot. So, it would simply fall through and go to the default
handler. Ouch. -- justin
Re: mod_dav inconsistent behaviour for GET requests
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Sat, 30 Jan 2010, Justin Erenkrantz wrote:
> I don't see how your patch would "intentionally" break - the failure
> mechanism is that the source scripts are served - not that a
> configuration error stops the server from running. -- justin
Surely a fatal server error is not a necessary condition for something to
be broken?
But let's put it in a different way: It's a matter of precedence of
different configuration directives, on the one hand
AddHandler/AddType/SetHandler, on the other hand 'Dav'.
The current behaviour is this: 'Dav' takes precedence over
AddHandler/AddType/SetHandler for all methods except GET and POST. Except
if the Dav provider handles GET by itself, then 'Dav' takes precedence
over AddHandler/AddType/SetHandler for all methods except POST.
I think this is illogical, difficult to understand, and makes debugging
problems harder than necessary. IMHO, 'Dav' should take precedence for all
methods (though my patch doesn't touch POST yet).
Cheers,
Stefan
Re: mod_dav inconsistent behaviour for GET requests
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Sat, Jan 30, 2010 at 2:04 AM, Stefan Fritsch <sf...@sfritsch.de> wrote:
> The recommended setup is to map separate URLs for DAV and script
> execution to the content. It has been like this since at least 2.0.
>
> The patch intentionally breaks existing configs that rely on the
> ability to use the same URLs for DAV and script execution. Is this not
> an acceptable change from 2.2 to 2.4 (if properly documented), as it
> makes life a lot easier for people who use the recommended setup?
I don't see how your patch would "intentionally" break - the failure
mechanism is that the source scripts are served - not that a
configuration error stops the server from running. -- justin
Re: mod_dav inconsistent behaviour for GET requests
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Sat, 30 Jan 2010, Graham Leggett wrote:
> On 30 Jan 2010, at 12:04 PM, Stefan Fritsch wrote:
> I don't follow how this makes it easier to use the recommended setup?
>
> In your example config, you defined /dav as being handled by mod_dav, and
> then you defined a FilesMatch (as I recall) that defined mod_php to be used
> by all URLs in the complete URL space that ended with ".php". In so doing
> you're creating two configs that both overlap and contradict themselves, and
> this is specifically discouraged by the recommended setup.
It's not that easy to enable mod_php globally except for one subdir, at
least for the casual admin. For the FilesMatch, one would need some
advanced regexp foo. The same is true if you have AddType'd various script
extensions.
The example config at
http://httpd.apache.org/docs/2.2/mod/mod_dav.html#complex recommends using
'ForceType text/plain' to override the 'AddType application/x-httpd-php
.php' that most users have somewhere else in their config. This is a
pretty bad hack, IMHO. Most files in the dav directory will be delivered
with the wrong content type.
> I also don't like the idea that mod_dav is treated differently to other
> handlers. If you want to really solve this problem, you need to do so
> generically.
mod_dav is not treated differently, is should just handle the request. The
fact that it delegates this to the default handler for some providers
should not concern the average user. Or how do you explain to a user that
a config that works fine with 'Dav svn' does not work with 'Dav on'? (Or
the other way round, depending on what you are trying to achieve.)
Re: mod_dav inconsistent behaviour for GET requests
Posted by Graham Leggett <mi...@sharp.fm>.
On 30 Jan 2010, at 12:04 PM, Stefan Fritsch wrote:
> The recommended setup is to map separate URLs for DAV and script
> execution to the content. It has been like this since at least 2.0.
>
> The patch intentionally breaks existing configs that rely on the
> ability to use the same URLs for DAV and script execution. Is this not
> an acceptable change from 2.2 to 2.4 (if properly documented), as it
> makes life a lot easier for people who use the recommended setup?
I don't follow how this makes it easier to use the recommended setup?
In your example config, you defined /dav as being handled by mod_dav,
and then you defined a FilesMatch (as I recall) that defined mod_php
to be used by all URLs in the complete URL space that ended with
".php". In so doing you're creating two configs that both overlap and
contradict themselves, and this is specifically discouraged by the
recommended setup.
I also don't like the idea that mod_dav is treated differently to
other handlers. If you want to really solve this problem, you need to
do so generically.
Regards,
Graham
--
Re: mod_dav inconsistent behaviour for GET requests
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Saturday 30 January 2010, Roy T. Fielding wrote:
> > */
> > if (!conf->provider->repos->handle_get) {
> > + if (r->finfo.filetype != APR_DIR)
> > + r->handler = "none";
> > return DECLINED;
> > }
> > }
>
> It looks to me like that would introduce a security hole for
> existing configs that expect a handler to run on GET (PHP/CGI
> scripts that are authorable via DAV). -1 if so.
The recommended setup is to map separate URLs for DAV and script
execution to the content. It has been like this since at least 2.0.
The patch intentionally breaks existing configs that rely on the
ability to use the same URLs for DAV and script execution. Is this not
an acceptable change from 2.2 to 2.4 (if properly documented), as it
makes life a lot easier for people who use the recommended setup?
Re: mod_dav inconsistent behaviour for GET requests
Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On Jan 29, 2010, at 10:46 AM, Stefan Fritsch wrote:
> On Friday 29 January 2010, Plüm, Rüdiger, VF-Group wrote:
>> Thanks for clarification. I guess I understand your intension
>> better now. So basicly you want those providers that do not
>> implement GET by themselves to enforce the usage of the default
>> handler, correct?
>> Mind to sent a patch to the list for better review?
>
> Exactly. The patch below works with 2.2 (haven't tested with trunk
> due to lack of mod_php).
>
> BTW, I found PR 13025, which seems to suggest that being able to mix
> script execution and DAV on the same URL is a feature. I am still for
> removing this 'feature' in trunk, though. But I would be against a
> backport to 2.2.x.
>
>
> --- a/modules/dav/main/mod_dav.c
> +++ b/modules/dav/main/mod_dav.c
> @@ -4803,12 +4803,13 @@ static int dav_fixups(request_rec *r)
>
> /*
> * If the repository hasn't indicated that it will handle the
> - * GET method, then just punt.
> - *
> - * ### this isn't quite right... taking over the response can break
> - * ### things like mod_negotiation. need to look into this some more.
> + * GET method, then we let the default handler do it. Set the handler
> + * explicitly to ensure that no other handler takes the request.
> + * We don't care about directories, though.
> */
> if (!conf->provider->repos->handle_get) {
> + if (r->finfo.filetype != APR_DIR)
> + r->handler = "none";
> return DECLINED;
> }
> }
It looks to me like that would introduce a security hole for existing
configs that expect a handler to run on GET (PHP/CGI scripts that are
authorable via DAV). -1 if so.
....Roy
Re: mod_dav inconsistent behaviour for GET requests
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Friday 29 January 2010, Plüm, Rüdiger, VF-Group wrote:
> Thanks for clarification. I guess I understand your intension
> better now. So basicly you want those providers that do not
> implement GET by themselves to enforce the usage of the default
> handler, correct?
> Mind to sent a patch to the list for better review?
Exactly. The patch below works with 2.2 (haven't tested with trunk
due to lack of mod_php).
BTW, I found PR 13025, which seems to suggest that being able to mix
script execution and DAV on the same URL is a feature. I am still for
removing this 'feature' in trunk, though. But I would be against a
backport to 2.2.x.
--- a/modules/dav/main/mod_dav.c
+++ b/modules/dav/main/mod_dav.c
@@ -4803,12 +4803,13 @@ static int dav_fixups(request_rec *r)
/*
* If the repository hasn't indicated that it will handle the
- * GET method, then just punt.
- *
- * ### this isn't quite right... taking over the response can break
- * ### things like mod_negotiation. need to look into this some more.
+ * GET method, then we let the default handler do it. Set the handler
+ * explicitly to ensure that no other handler takes the request.
+ * We don't care about directories, though.
*/
if (!conf->provider->repos->handle_get) {
+ if (r->finfo.filetype != APR_DIR)
+ r->handler = "none";
return DECLINED;
}
}
RE: mod_dav inconsistent behaviour for GET requests
Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
> -----Original Message-----
> From: Stefan Fritsch
> Sent: Freitag, 29. Januar 2010 11:26
> To: dev@httpd.apache.org
> Subject: Re: mod_dav inconsistent behaviour for GET requests
>
> It seems my mail was not so clear.
>
> On Friday 29 January 2010, Plüm, Rüdiger, VF-Group wrote:
> > > The documentation
> > > http://httpd.apache.org/docs/trunk/mod/mod_dav.html#complex
> > > suggests using ForceType 'text/plain' to circumvent this. Apart
> > > from the fact that this is not enough and one may need
> > > 'SetHandler none' as well, I think this is a kludge. mod_dav
> > > should make sure that the GET request is handled by the default
> > > handler by setting r->handler in its fixup hook. If nobody
> > > disagrees, I will change this in trunk.
> >
> > Please do not. Some mod_dav providers need to implement their own
> > GET handler because the resource requested might not be a plain
> > file (e.g. it is stored in the database).
>
> If this was not clear, I only want to change the behaviour for
> providers that do not implement their own GET handler, i.e.
> handle_get
> == 0.
>
> > Apart from Julians comments, SSI is a filter and can be applied to
> > these resources as well (no problem) and the same is true for PHP
> > if mod_php is build as a filter.
>
> Ok, filters are a different problem, then. But my argument still
> stands for mod_php as handler.
>
> > But usually: Use a different URL for WebDAV than for live access.
>
> That's what I want. But mod_dav makes it more difficult than
> necessary. Consider a configuration with mod_php like this:
>
> <FilesMatch \.php$>
> SetHandler application/x-httpd-php
> </Files>
>
> <Location /dav>
> Dav on
> </Location>
>
> Then I want mod_dav to serve the script source for /dav/test.php but
> instead I will get the script output from mod_php. To get the script
> source, I currently need:
>
> <Location /dav>
> Dav on
> SetHandler none
> </Location>
>
> In contrast, if I use a dav provider that handles GETs (like
> mod_dav_svn), I don't need the 'SetHandler none'. This is
> inconsistent.
>
> IMNSHO, if I set 'Dav on' for some Location, mod_dav has to ensure
> that it works, even for providers that don't handle GETs.
Thanks for clarification. I guess I understand your intension better now.
So basicly you want those providers that do not implement GET by themselves
to enforce the usage of the default handler, correct?
Mind to sent a patch to the list for better review?
Regards
Rüdiger
Re: mod_dav inconsistent behaviour for GET requests
Posted by Stefan Fritsch <sf...@sfritsch.de>.
It seems my mail was not so clear.
On Friday 29 January 2010, Plüm, Rüdiger, VF-Group wrote:
> > The documentation
> > http://httpd.apache.org/docs/trunk/mod/mod_dav.html#complex
> > suggests using ForceType 'text/plain' to circumvent this. Apart
> > from the fact that this is not enough and one may need
> > 'SetHandler none' as well, I think this is a kludge. mod_dav
> > should make sure that the GET request is handled by the default
> > handler by setting r->handler in its fixup hook. If nobody
> > disagrees, I will change this in trunk.
>
> Please do not. Some mod_dav providers need to implement their own
> GET handler because the resource requested might not be a plain
> file (e.g. it is stored in the database).
If this was not clear, I only want to change the behaviour for
providers that do not implement their own GET handler, i.e. handle_get
== 0.
> Apart from Julians comments, SSI is a filter and can be applied to
> these resources as well (no problem) and the same is true for PHP
> if mod_php is build as a filter.
Ok, filters are a different problem, then. But my argument still
stands for mod_php as handler.
> But usually: Use a different URL for WebDAV than for live access.
That's what I want. But mod_dav makes it more difficult than
necessary. Consider a configuration with mod_php like this:
<FilesMatch \.php$>
SetHandler application/x-httpd-php
</Files>
<Location /dav>
Dav on
</Location>
Then I want mod_dav to serve the script source for /dav/test.php but
instead I will get the script output from mod_php. To get the script
source, I currently need:
<Location /dav>
Dav on
SetHandler none
</Location>
In contrast, if I use a dav provider that handles GETs (like
mod_dav_svn), I don't need the 'SetHandler none'. This is
inconsistent.
IMNSHO, if I set 'Dav on' for some Location, mod_dav has to ensure
that it works, even for providers that don't handle GETs.
Cheers,
Stefan
RE: mod_dav inconsistent behaviour for GET requests
Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
> -----Original Message-----
> From: Stefan Fritsch
> Sent: Freitag, 29. Januar 2010 08:43
> To: dev@httpd.apache.org
> Subject: mod_dav inconsistent behaviour for GET requests
>
> Hi,
>
> mod_dav doesn't handle GET requests in a consistent way: If a repos
> provider has handle_get == 1, mod_dav will handle GET requests by
> itself. This means no other handler will get a chance to
> interpret the
> file as script or SSI. On the other hand, if the repos provider has
> handle_get == 0, mod_dav will do nothing and another handler like
> mod_php or mod_include may handle scripts, causing the output of the
> script execution to be sent to the client instead of the script
> source.
>
> The documentation
> http://httpd.apache.org/docs/trunk/mod/mod_dav.html#complex suggests
> using ForceType 'text/plain' to circumvent this. Apart from the fact
> that this is not enough and one may need 'SetHandler none' as well, I
> think this is a kludge. mod_dav should make sure that the GET request
> is handled by the default handler by setting r->handler in its fixup
> hook. If nobody disagrees, I will change this in trunk.
Please do not. Some mod_dav providers need to implement their own GET handler
because the resource requested might not be a plain file (e.g. it is stored
in the database).
Apart from Julians comments, SSI is a filter and can be applied to these
resources as well (no problem) and the same is true for PHP if mod_php
is build as a filter.
But usually: Use a different URL for WebDAV than for live access.
Regards
Rüdiger
Re: mod_dav inconsistent behaviour for GET requests
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Friday 29 January 2010, Julian Reschke wrote:
> And if you really really have to, consider using the MS extension
> request header "Translate".
Not all DAV clients send this header. Therefore it is not an option.
Re: mod_dav inconsistent behaviour for GET requests
Posted by Julian Reschke <ju...@gmx.de>.
Stefan Fritsch wrote:
> Hi,
>
> mod_dav doesn't handle GET requests in a consistent way: If a repos
> provider has handle_get == 1, mod_dav will handle GET requests by
> itself. This means no other handler will get a chance to interpret the
> file as script or SSI. On the other hand, if the repos provider has
> handle_get == 0, mod_dav will do nothing and another handler like
> mod_php or mod_include may handle scripts, causing the output of the
> script execution to be sent to the client instead of the script
> source.
> ...
Lesson: don't serve the script's output and the script itself from the
same URI.
And if you really really have to, consider using the MS extension
request header "Translate".
BR, Julian