Re: svn commit: r1561447 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_dir.c

[Eric Covener <co...@gmail.com>]

Is the DirectorySlash side of things ever useful for these methods?

On Sat, Jan 25, 2014 at 11:09 PM,  <ch...@apache.org> wrote:
> Author: chrisd
> Date: Sun Jan 26 04:09:45 2014
> New Revision: 1561447
>
> URL: http://svn.apache.org/r1561447
> Log:
> Skip DirectoryIndex execution unless method is GET or POST, restoring
> 2.2 behavior when using mod_dav. PR 54914.
>
> Otherwise, variable behavior results: if no DirectoryIndex file is found,
> mod_dav's r->handler runs as expected.  But if an index file is found,
> r->handler will be changed by ap_internal_fast_redirect() to something
> other than mod_dav's r->handler, while r->method is left unchanged,
> usually leading to a 405 response.
>
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/mappers/mod_dir.c
>
> Modified: httpd/httpd/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1561447&r1=1561446&r2=1561447&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Sun Jan 26 04:09:45 2014
> @@ -1,6 +1,9 @@
>                                                           -*- coding: utf-8 -*-
>  Changes with Apache 2.5.0
>
> +  *) mod_dir: Default to 2.2-like behavior and skip execution when method is
> +     neither GET nor POST, such as for DAV requests. PR 54914. [Chris Darroch]
> +
>    *) mod_rewrite: Rename the handler that does per-directory internal
>       redirects to "rewrite-redirect-handler" from "redirect-handler" so
>       it is less ambiguous and less likely to be reused. [Eric Covener]
>
> Modified: httpd/httpd/trunk/modules/mappers/mod_dir.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_dir.c?rev=1561447&r1=1561446&r2=1561447&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/mappers/mod_dir.c (original)
> +++ httpd/httpd/trunk/modules/mappers/mod_dir.c Sun Jan 26 04:09:45 2014
> @@ -274,6 +274,10 @@ static int fixup_dir(request_rec *r)
>          return HTTP_MOVED_PERMANENTLY;
>      }
>
> +    if (r->method_number != M_GET && r->method_number != M_POST) {
> +        return DECLINED;
> +    }
> +
>      if (d->checkhandler == MODDIR_ON && strcmp(r->handler, DIR_MAGIC_TYPE)) {
>          return DECLINED;
>      }
>
>



-- 
Eric Covener
covener@gmail.com

Re: svn commit: r1561447 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_dir.c

[Chris Darroch <ch...@pearsoncmg.com>]

Eric:

> Is the DirectorySlash side of things ever useful for these methods?

   You're thinking that maybe the return DECLINED statement could move
up to the top of fixup_dir()?  I've pondered that myself, but I think
that's a much bigger change than just restoring the 2.2.x behaviour
which should resolve this PR issue.

   For one thing, mod_dav is only one module -- other third-party
modules may expect the 2.2 behaviour here as well.  So I'd hesitate
to make a more substantial change.

   As for mod_dav, the spec says, "If clients do not use the trailing
slash form the client needs to be prepared to see a redirect response."
It also notes that the server MAY choose to operate on a collection URL
lacking a trailing slash, and if so, it SHOULD return with a Content-Location
header.  So far as I know, mod_dav isn't ready to do this -- it likely
operates OK on collection URLs lacking a trailing slash, but I don't
think it returns the Content-Location header.  That's not required, but
it would be nice to have before we made this kind of change.

   Practically speaking, since mod_dir and DirectorySlash are enabled
in the default httpd configuration, I suspect most mod_dav deployments
will have DirectorySlash enabled, and therefore most DAV clients will
be able to operate with the existing 2.2 redirection behaviour.  (And
some may even flail if they don't see the redirection, although that's
technically their problem.)

   So for the moment, I'd be inclined to just restore the 2.2 behaviour
and close the PR.  I've got a stack of other mod_dav fixups should
try to push into trunk ... I'll try to steal some time to chip away
at the pile.  No rash promises, though.  :-/

   Thanks for taking a look at this; cheers,

Chris.

-- 
GPG Key ID: 088335A9
GPG Key Fingerprint: 86CD 3297 7493 75BC F820  6715 F54F E648 0883 35A9