You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by co...@apache.org on 2014/01/13 02:42:12 UTC
svn commit: r1557640 - in /httpd/httpd/trunk: CHANGES
docs/manual/mod/mod_dir.xml modules/mappers/mod_dir.c
Author: covener
Date: Mon Jan 13 01:42:12 2014
New Revision: 1557640
URL: http://svn.apache.org/r1557640
Log:
restore http://svn.apache.org/viewvc?view=revision&revision=233369
under a configurable option: don't run mod_dir if r->handler is already set.
PR53794
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/docs/manual/mod/mod_dir.xml
httpd/httpd/trunk/modules/mappers/mod_dir.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1557640&r1=1557639&r2=1557640&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Mon Jan 13 01:42:12 2014
@@ -1,6 +1,10 @@
-*- coding: utf-8 -*-
Changes with Apache 2.5.0
+
+ *) mod_dir: Add DirectoryCheckHandler to allow a 2.2-like behavior, skipping
+ execution when a handler is already set. PR53929. [Eric Covener]
+
*) mod_rewrite: Protect against looping with the [N] flag by enforcing a
default limit of 10000 iterations, and allowing each rule to change its
limit. [Eric Covener]
Modified: httpd/httpd/trunk/docs/manual/mod/mod_dir.xml
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_dir.xml?rev=1557640&r1=1557639&r2=1557640&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/manual/mod/mod_dir.xml (original)
+++ httpd/httpd/trunk/docs/manual/mod/mod_dir.xml Mon Jan 13 01:42:12 2014
@@ -274,5 +274,31 @@ a directory</description>
</highlight>
</usage>
</directivesynopsis>
+<directivesynopsis>
+<name>DirectoryCheckHandler</name>
+<description>Toggle how this module responds when another handler is configured</description>
+<syntax>DirectoryCheckHandler On|Off</syntax>
+<default>DirectorySlash Off</default>
+<contextlist><context>server config</context><context>virtual host</context>
+<context>directory</context><context>.htaccess</context></contextlist>
+<override>Indexes</override>
+<compatibility>Available in 2.4.8 and later. Releases prior to 2.4 implicitly
+act as if "DirectorySlash Off" was specified.</compatibility>
+<usage>
+ <p>The <directive>DirectoryCheckHandler</directive> directive determines
+ whether <module>mod_dir</module> should check for directory indexes or
+ add trailing slashes when some other handler has been configured for
+ the current URL. Handlers can be set by directives such as
+ <directive module="core">SetHandler</directive> or by other modules,
+ such as <module>mod_rewrite</module> during per-directory substitutions.
+ </p>
+
+ <p> In releases prior to 2.4, this module did not take any action if any
+ other handler was configured for a URL. This allows directory indexes to
+ be served even when a <directive>SetHandler</directive> directive is
+ specified for an entire directory, but it can also result in some conflicts
+ with modules such as <directive>mod_rewrite</directive>.</p>
+</usage>
+</directivesynopsis>
</modulesynopsis>
Modified: httpd/httpd/trunk/modules/mappers/mod_dir.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_dir.c?rev=1557640&r1=1557639&r2=1557640&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/mappers/mod_dir.c (original)
+++ httpd/httpd/trunk/modules/mappers/mod_dir.c Mon Jan 13 01:42:12 2014
@@ -44,6 +44,7 @@ typedef enum {
typedef struct dir_config_struct {
apr_array_header_t *index_names;
moddir_cfg do_slash;
+ moddir_cfg checkhandler;
int redirect_index;
const char *dflt;
} dir_config_rec;
@@ -86,6 +87,13 @@ static const char *configure_slash(cmd_p
d->do_slash = arg ? MODDIR_ON : MODDIR_OFF;
return NULL;
}
+static const char *configure_checkhandler(cmd_parms *cmd, void *d_, int arg)
+{
+ dir_config_rec *d = d_;
+
+ d->checkhandler = arg ? MODDIR_ON : MODDIR_OFF;
+ return NULL;
+}
static const char *configure_redirect(cmd_parms *cmd, void *d_, const char *arg1)
{
dir_config_rec *d = d_;
@@ -123,6 +131,8 @@ static const command_rec dir_cmds[] =
"a list of file names"),
AP_INIT_FLAG("DirectorySlash", configure_slash, NULL, DIR_CMD_PERMS,
"On or Off"),
+ AP_INIT_FLAG("DirectoryCheckHandler", configure_checkhandler, NULL, DIR_CMD_PERMS,
+ "On or Off"),
AP_INIT_TAKE1("DirectoryIndexRedirect", configure_redirect,
NULL, DIR_CMD_PERMS, "On, Off, or a 3xx status code."),
@@ -135,6 +145,7 @@ static void *create_dir_config(apr_pool_
new->index_names = NULL;
new->do_slash = MODDIR_UNSET;
+ new->checkhandler = MODDIR_UNSET;
new->redirect_index = REDIRECT_UNSET;
return (void *) new;
}
@@ -148,6 +159,8 @@ static void *merge_dir_configs(apr_pool_
new->index_names = add->index_names ? add->index_names : base->index_names;
new->do_slash =
(add->do_slash == MODDIR_UNSET) ? base->do_slash : add->do_slash;
+ new->checkhandler =
+ (add->checkhandler == MODDIR_UNSET) ? base->checkhandler : add->checkhandler;
new->redirect_index=
(add->redirect_index == REDIRECT_UNSET) ? base->redirect_index : add->redirect_index;
new->dflt = add->dflt ? add->dflt : base->dflt;
@@ -260,6 +273,10 @@ static int fixup_dir(request_rec *r)
return HTTP_MOVED_PERMANENTLY;
}
+ if (d->checkhandler == MODDIR_ON && strcmp(r->handler, DIR_MAGIC_TYPE)) {
+ return DECLINED;
+ }
+
if (d->index_names) {
names_ptr = (char **)d->index_names->elts;
num_names = d->index_names->nelts;
Re: svn commit: r1557640 - in /httpd/httpd/trunk: CHANGES
docs/manual/mod/mod_dir.xml modules/mappers/mod_dir.c
Posted by Eric Covener <co...@gmail.com>.
> At least one of the "DirectorySlash" occurrences is wrong. (I guess both
> are.)
got 'em in r1562170
> I wonder if there is a more intuitive directive name. What about
> DirectoryOverrideHandler? (with reversed sense of on/off) Is that more
> obvious than DirectoryCheckHandler?
The options are all pretty awkward. I think DirectoryOverrideHandler
might be a little better. Maybe something better will spring up in
the thread.
Re: svn commit: r1557640 - in /httpd/httpd/trunk: CHANGES
docs/manual/mod/mod_dir.xml modules/mappers/mod_dir.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Sun, Jan 12, 2014 at 8:42 PM, <co...@apache.org> wrote:
> Author: covener
> Date: Mon Jan 13 01:42:12 2014
> New Revision: 1557640
>
> URL: http://svn.apache.org/r1557640
> Log:
> restore http://svn.apache.org/viewvc?view=revision&revision=233369
> under a configurable option: don't run mod_dir if r->handler is already
> set.
>
> PR53794
>
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/docs/manual/mod/mod_dir.xml
> httpd/httpd/trunk/modules/mappers/mod_dir.c
>
> Modified: httpd/httpd/trunk/CHANGES
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1557640&r1=1557639&r2=1557640&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Mon Jan 13 01:42:12 2014
> @@ -1,6 +1,10 @@
> -*- coding:
> utf-8 -*-
> Changes with Apache 2.5.0
>
> +
> + *) mod_dir: Add DirectoryCheckHandler to allow a 2.2-like behavior,
> skipping
> + execution when a handler is already set. PR53929. [Eric Covener]
> +
> *) mod_rewrite: Protect against looping with the [N] flag by enforcing a
> default limit of 10000 iterations, and allowing each rule to change
> its
> limit. [Eric Covener]
>
> Modified: httpd/httpd/trunk/docs/manual/mod/mod_dir.xml
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_dir.xml?rev=1557640&r1=1557639&r2=1557640&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/docs/manual/mod/mod_dir.xml (original)
> +++ httpd/httpd/trunk/docs/manual/mod/mod_dir.xml Mon Jan 13 01:42:12 2014
> @@ -274,5 +274,31 @@ a directory</description>
> </highlight>
> </usage>
> </directivesynopsis>
> +<directivesynopsis>
> +<name>DirectoryCheckHandler</name>
> +<description>Toggle how this module responds when another handler is
> configured</description>
> +<syntax>DirectoryCheckHandler On|Off</syntax>
> +<default>DirectorySlash Off</default>
> +<contextlist><context>server config</context><context>virtual
> host</context>
> +<context>directory</context><context>.htaccess</context></contextlist>
> +<override>Indexes</override>
> +<compatibility>Available in 2.4.8 and later. Releases prior to 2.4
> implicitly
> +act as if "DirectorySlash Off" was specified.</compatibility>
> +<usage>
> + <p>The <directive>DirectoryCheckHandler</directive> directive
> determines
> + whether <module>mod_dir</module> should check for directory indexes or
> + add trailing slashes when some other handler has been configured for
> + the current URL. Handlers can be set by directives such as
> + <directive module="core">SetHandler</directive> or by other modules,
> + such as <module>mod_rewrite</module> during per-directory
> substitutions.
> + </p>
> +
> + <p> In releases prior to 2.4, this module did not take any action if
> any
> + other handler was configured for a URL. This allows directory indexes
> to
> + be served even when a <directive>SetHandler</directive> directive is
> + specified for an entire directory, but it can also result in some
> conflicts
> + with modules such as <directive>mod_rewrite</directive>.</p>
> +</usage>
> +</directivesynopsis>
>
At least one of the "DirectorySlash" occurrences is wrong. (I guess both
are.)
I wonder if there is a more intuitive directive name. What about
DirectoryOverrideHandler? (with reversed sense of on/off) Is that more
obvious than DirectoryCheckHandler?
> </modulesynopsis>
>
> Modified: httpd/httpd/trunk/modules/mappers/mod_dir.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_dir.c?rev=1557640&r1=1557639&r2=1557640&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/modules/mappers/mod_dir.c (original)
> +++ httpd/httpd/trunk/modules/mappers/mod_dir.c Mon Jan 13 01:42:12 2014
> @@ -44,6 +44,7 @@ typedef enum {
> typedef struct dir_config_struct {
> apr_array_header_t *index_names;
> moddir_cfg do_slash;
> + moddir_cfg checkhandler;
> int redirect_index;
> const char *dflt;
> } dir_config_rec;
> @@ -86,6 +87,13 @@ static const char *configure_slash(cmd_p
> d->do_slash = arg ? MODDIR_ON : MODDIR_OFF;
> return NULL;
> }
> +static const char *configure_checkhandler(cmd_parms *cmd, void *d_, int
> arg)
> +{
> + dir_config_rec *d = d_;
> +
> + d->checkhandler = arg ? MODDIR_ON : MODDIR_OFF;
> + return NULL;
> +}
> static const char *configure_redirect(cmd_parms *cmd, void *d_, const
> char *arg1)
> {
> dir_config_rec *d = d_;
> @@ -123,6 +131,8 @@ static const command_rec dir_cmds[] =
> "a list of file names"),
> AP_INIT_FLAG("DirectorySlash", configure_slash, NULL, DIR_CMD_PERMS,
> "On or Off"),
> + AP_INIT_FLAG("DirectoryCheckHandler", configure_checkhandler, NULL,
> DIR_CMD_PERMS,
> + "On or Off"),
> AP_INIT_TAKE1("DirectoryIndexRedirect", configure_redirect,
> NULL, DIR_CMD_PERMS, "On, Off, or a 3xx status code."),
>
> @@ -135,6 +145,7 @@ static void *create_dir_config(apr_pool_
>
> new->index_names = NULL;
> new->do_slash = MODDIR_UNSET;
> + new->checkhandler = MODDIR_UNSET;
> new->redirect_index = REDIRECT_UNSET;
> return (void *) new;
> }
> @@ -148,6 +159,8 @@ static void *merge_dir_configs(apr_pool_
> new->index_names = add->index_names ? add->index_names :
> base->index_names;
> new->do_slash =
> (add->do_slash == MODDIR_UNSET) ? base->do_slash : add->do_slash;
> + new->checkhandler =
> + (add->checkhandler == MODDIR_UNSET) ? base->checkhandler :
> add->checkhandler;
> new->redirect_index=
> (add->redirect_index == REDIRECT_UNSET) ? base->redirect_index :
> add->redirect_index;
> new->dflt = add->dflt ? add->dflt : base->dflt;
> @@ -260,6 +273,10 @@ static int fixup_dir(request_rec *r)
> return HTTP_MOVED_PERMANENTLY;
> }
>
> + if (d->checkhandler == MODDIR_ON && strcmp(r->handler,
> DIR_MAGIC_TYPE)) {
> + return DECLINED;
> + }
> +
> if (d->index_names) {
> names_ptr = (char **)d->index_names->elts;
> num_names = d->index_names->nelts;
>
>
>
--
Born in Roswell... married an alien...
http://emptyhammock.com/
Re: svn commit: r1557640 - in /httpd/httpd/trunk: CHANGES
docs/manual/mod/mod_dir.xml modules/mappers/mod_dir.c
Posted by Eric Covener <co...@gmail.com>.
> For users of mod_dav, though, this means that a working 2.2.x
> configuration will still break after an upgrade to 2.4.8; they'd
> need to add in "DirectoryCheckHandler On".
For the rewrite PR's, which are relatively old, I just wanted to get a
safe non-default change into 2.4.x.
I subsequently [next revision] decided it was safe enough to whitelist
mod_rewrites redirect-handler, and to do it by default. One of the
good reasons for this is that since it's trying to do an internal
redirect, mod_dir will get a second chance at it in the new request.
>
> I'd like to my additional line of logic, if you agree:
>
> + if (r->method_number != M_GET && r->method_number != M_POST) {
> + return DECLINED;
> + }
>
> which would ensure that for mod_dir takes no effect when the method
> is neither GET nor POST, which seems reasonable to me (since it has
> no real meaning with other methods), and has the effect that 2.2.x DAV
> setups will "just work" again with 2.4.x, once the patch is back-ported.
>
> Does that seem acceptable to you? I can just follow CTR, I know, but
> I thought I'd ask first since you're clearly looking at similar issues
> at the same time.
I don't really know enough about the risk on this one.
Re: svn commit: r1557640 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_dir.xml
modules/mappers/mod_dir.c
Posted by Chris Darroch <ch...@pearsoncmg.com>.
covener@apache.org wrote:
> restore http://svn.apache.org/viewvc?view=revision&revision=233369
> under a configurable option: don't run mod_dir if r->handler is already set.
> PR53794
I like this, and it's certainly a more configurable solution to
the problem with mod_dav which came up last week:
http://marc.info/?l=apache-httpd-dev&m=138842871710848&w=2
The only problem I see here (without having testing the new changes)
is that I believe the default will be to preserve the prior 2.4.x
behaviour:
> + new->checkhandler = MODDIR_UNSET;
> + if (d->checkhandler == MODDIR_ON && strcmp(r->handler, DIR_MAGIC_TYPE)) {
> + return DECLINED;
> + }
For users of mod_dav, though, this means that a working 2.2.x
configuration will still break after an upgrade to 2.4.8; they'd
need to add in "DirectoryCheckHandler On".
I'd like to my additional line of logic, if you agree:
+ if (r->method_number != M_GET && r->method_number != M_POST) {
+ return DECLINED;
+ }
which would ensure that for mod_dir takes no effect when the method
is neither GET nor POST, which seems reasonable to me (since it has
no real meaning with other methods), and has the effect that 2.2.x DAV
setups will "just work" again with 2.4.x, once the patch is back-ported.
Does that seem acceptable to you? I can just follow CTR, I know, but
I thought I'd ask first since you're clearly looking at similar issues
at the same time.
Chris.
--
GPG Key ID: 088335A9
GPG Key Fingerprint: 86CD 3297 7493 75BC F820 6715 F54F E648 0883 35A9