On 7/21/2011 3:32 AM, Igor Galić wrote:
> I think you're missing an MMN bump, regarding backporting - or API in
> general, the wrapper is the right way to go.
> Also: Why not patch against trunk?
>
> i
>
> Daniel Ruggeri <[email protected]> wrote:
>
>> All;
>> I am attaching a patch that will allow for a comma separated list of
>> directives permissable for override. I am doing this because the
>> existing AllowOverride list of override-able directives sometimes has
>> too many things, sometimes allows more than is documented and it also
>> forces third party modules into one of the four predefined lists. With
>> this patch, an AllowOverrideList directive is added for the server admin
>> to specify individual directives for override. No other functionality
>> changes. FWIW, In my particular use case, I would like to grant a
>> content admin the ability to implement a server-side redirect in
>> .htacess, but would NOT like to grant them the ability to stand up a
>> proxy via "RewriteRule / http://someOtherLocation/ [P]" (which is what
>> would happen if FileInfo was set in AllowOverride). Note how Redirect
>> and friends are not documented as directives in core.html#allowoverride
>> for FileInfo... another patch, another day :) .
>>
>>
>> With that in mind, I'd like to request comments on the idea and the
>> implementation supplied. I can see that there could be efficiency gained
>> without having to "tolower" each directive before comparison in
>> invoke_cmd, but I could not find a way outside of server/config.c
>> (server/core.c constructs the list I am using) to look up a directive's
>> normal case like ReDiREcT becomes "Redirect" before the call to
>> invoke_cmd as a result of ml = apr_hash_get(ap_config_hash, dir,
>> APR_HASH_KEY_STRING);.
>>
>>
>> The second thing I'd like to bring up is that I see a lot of use in
>> backporting this, but I have made a change that I think would be
>> considered an API change (added param to ap_parse_htaccess in
>> include/http_config.h). Since I know this is not allowed, would it be
>> acceptable to rename the new ap_parse_htaccess function and implement a
>> wrapper as ap_parse_htaccess? I would foresee that such a wrapper would
>> issue a deprecation warning when called, but will call ap_parse_htaccess
>> with a NULL in place of the (new) override_list.
>>
>> cheers!
>> --
>> --
>> Daniel Ruggeri
Attached is the final cut of the patch including doco and MMN bump as
you brought up. I plan to commit this on Monday, time permitting (and of
course in the absence of objections). I'll cobble something together
afterwards for a 2.2 backport.
Regarding the question, Igor, there was not much thought put into which
source tree I started hacking on - I wanted to get something together
before this weekend hit and already had this version configured/built.
--
--
Daniel Ruggeri
Index: httpd-trunk.AllowOverrideList/docs/manual/mod/core.xml
===================================================================
--- httpd-trunk.AllowOverrideList/docs/manual/mod/core.xml (revision
1149429)
+++ httpd-trunk.AllowOverrideList/docs/manual/mod/core.xml (working copy)
@@ -327,7 +327,8 @@
<directive type="section" module="core">Files</directive> sections.
</note>
- <p>When this directive is set to <code>None</code>, then
+ <p>When this directive is set to <code>None</code> and <directive
+ module="core">AllowOverrideList</directive> is not set, then
<a href="#accessfilename">.htaccess</a> files are completely ignored.
In this case, the server will not even attempt to read
<code>.htaccess</code> files in the filesystem.</p>
@@ -442,7 +443,60 @@
</note>
</usage>
+<directivesynopsis>
+<name>AllowOverrideList</name>
+<description>Individual directives that are allowed in
+<code>.htaccess</code> files</description>
+<syntax>AllowOverrideList directive1,directve2,...</syntax>
+<contextlist><context>directory</context></contextlist>
+
+<usage>
+ <p>When the server finds an <code>.htaccess</code> file (as
+ specified by <directive module="core">AccessFileName</directive>)
+ it needs to know which directives declared in that file can override
+ earlier configuration directives.</p>
+
+ <note><title>Only available in <Directory> sections</title>
+ <directive>AllowOverrideList</directive> is valid only in
+ <directive type="section" module="core">Directory</directive>
+ sections specified without regular expressions, not in <directive
+ type="section" module="core">Location</directive>, <directive
+ module="core" type="section">DirectoryMatch</directive> or
+ <directive type="section" module="core">Files</directive> sections.
+ </note>
+
+ <p>When this directive is not set and <directive module="core">
+ AllowOverride</directive> is set to <code>None</code>, then
+ <a href="#accessfilename">.htaccess</a> files are completely ignored.
+ In this case, the server will not even attempt to read
+ <code>.htaccess</code> files in the filesystem.</p>
+
+ <p>Example:</p>
+
+ <example>
+ AllowOverride None
+ AllowOverrideList "Redirect,RedirectMatch"
+ </example>
+
+ <p>In the example above only the <code>Redirect</code> and
+ <code>RedirectMatch</code> directives are allowed. All others will
+ cause an internal server error.</p>
+
+ <example>
+ AllowOverride AuthConfig
+ AllowOverrideList "CookieTracking,CookieName"
+ </example>
+
+ <p>In the example above <directive module="core">AllowOverride
+ </directive> grants permission to the <code>AuthConfig</code>
+ directive grouping and <directive>AllowOverrideList</directive> grants
+ permission to only two directves from the <code>FileInfo</code> directive
+ grouping. All others will cause an internal server error.</p>
+</usage>
+
+
<seealso><directive module="core">AccessFileName</directive></seealso>
+<seealso><directive module="core">AllowOverride</directive></seealso>
<seealso><a href="../configuring.html">Configuration Files</a></seealso>
<seealso><a href="../howto/htaccess.html">.htaccess Files</a></seealso>
</directivesynopsis>
Index: httpd-trunk.AllowOverrideList/server/config.c
===================================================================
--- httpd-trunk.AllowOverrideList/server/config.c (revision 1149429)
+++ httpd-trunk.AllowOverrideList/server/config.c (working copy)
@@ -838,10 +838,20 @@
static const char *invoke_cmd(const command_rec *cmd, cmd_parms *parms,
void *mconfig, const char *args)
{
+ int override_list_ok = 0;
char *w, *w2, *w3;
const char *errmsg = NULL;
- if ((parms->override & cmd->req_override) == 0)
+ /** Have we been provided a list of acceptable directives? */
+ if(parms->override_list != NULL) {
+ char *directivelc=apr_pstrdup(parms->pool,cmd->name);
+ ap_str_tolower(directivelc);
+
+ if(apr_table_get(parms->override_list, directivelc) != NULL)
+ override_list_ok = 1;
+ }
+
+ if ((parms->override & cmd->req_override) == 0 && !override_list_ok)
return apr_pstrcat(parms->pool, cmd->name, " not allowed here", NULL);
parms->info = cmd->cmd_data;
@@ -1506,7 +1516,7 @@
*/
static cmd_parms default_parms =
-{NULL, 0, 0, -1, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL};
+{NULL, 0, 0, NULL, -1, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL};
AP_DECLARE(char *) ap_server_root_relative(apr_pool_t *p, const char *file)
{
@@ -2005,7 +2015,7 @@
AP_CORE_DECLARE(int) ap_parse_htaccess(ap_conf_vector_t **result,
request_rec *r, int override,
- int override_opts,
+ int override_opts, apr_table_t
*override_list,
const char *d, const char *access_name)
{
ap_configfile_t *f = NULL;
@@ -2027,6 +2037,7 @@
parms = default_parms;
parms.override = override;
parms.override_opts = override_opts;
+ parms.override_list = override_list;
parms.pool = r->pool;
parms.temp_pool = r->pool;
parms.server = r->server;
Index: httpd-trunk.AllowOverrideList/server/core.c
===================================================================
--- httpd-trunk.AllowOverrideList/server/core.c (revision 1149429)
+++ httpd-trunk.AllowOverrideList/server/core.c (working copy)
@@ -236,6 +236,10 @@
conf->override_opts = new->override_opts;
}
+ if (conf->override_list == NULL) {
+ conf->override_list = new->override_list;
+ }
+
if (conf->response_code_strings == NULL) {
conf->response_code_strings = new->response_code_strings;
}
@@ -1608,6 +1612,33 @@
return NULL;
}
+static const char *set_override_list(cmd_parms *cmd, void *d_, const char *arg)
+{
+ core_dir_config *d = d_;
+
+ /* Throw a warning if we're in <Location> or <Files> */
+ if (ap_check_cmd_context(cmd, NOT_IN_LOCATION | NOT_IN_FILES)) {
+ ap_log_error(APLOG_MARK, APLOG_WARNING, 0, cmd->server,
+ "Useless use of AllowOverrideList in line %d of %s.",
+ cmd->directive->line_num, cmd->directive->filename);
+ }
+
+ char *directive;
+ char *tok_state;
+ char *arglc=apr_pstrdup(cmd->temp_pool, arg);
+
+ ap_str_tolower(arglc);
+ d->override_list = apr_table_make(cmd->pool, 1);
+
+ directive = apr_strtok(arglc, ", ", &tok_state);
+ while (directive != NULL) {
+ apr_table_set(d->override_list, directive, "1");
+ directive = apr_strtok(NULL, ", ", &tok_state);
+ }
+
+ return NULL;
+}
+
static const char *set_options(cmd_parms *cmd, void *d_, const char *l)
{
core_dir_config *d = d_;
@@ -3742,6 +3773,9 @@
AP_INIT_RAW_ARGS("AllowOverride", set_override, NULL, ACCESS_CONF,
"Controls what groups of directives can be configured by per-directory "
"config files"),
+AP_INIT_TAKE1("AllowOverrideList", set_override_list, NULL, ACCESS_CONF,
+ "Controls what individual directives can be configured by per-directory "
+ "config files as a comma-separated list"),
AP_INIT_RAW_ARGS("Options", set_options, NULL, OR_OPTIONS,
"Set a number of attributes for a given directory"),
AP_INIT_TAKE1("DefaultType", set_default_type, NULL, OR_FILEINFO,
Index: httpd-trunk.AllowOverrideList/server/request.c
===================================================================
--- httpd-trunk.AllowOverrideList/server/request.c (revision 1149429)
+++ httpd-trunk.AllowOverrideList/server/request.c (working copy)
@@ -486,6 +486,7 @@
allow_options_t remove;
overrides_t override;
overrides_t override_opts;
+ apr_table_t *override_list;
} core_opts_t;
static void core_opts_merge(const ap_conf_vector_t *sec, core_opts_t *opts)
@@ -513,6 +514,11 @@
opts->override = this_dir->override;
opts->override_opts = this_dir->override_opts;
}
+
+ if (this_dir->override_list != NULL) {
+ opts->override_list = this_dir->override_list;
+ }
+
}
@@ -740,6 +746,7 @@
opts.remove = this_dir->opts_remove;
opts.override = this_dir->override;
opts.override_opts = this_dir->override_opts;
+ opts.override_list = this_dir->override_list;
/* Set aside path_info to merge back onto path_info later.
* If r->filename is a directory, we must remerge the path_info,
@@ -946,12 +953,13 @@
/* No htaccess in an incomplete root path,
* nor if it's disabled
*/
- if (seg < startseg || !opts.override) {
+ if (seg < startseg || (!opts.override && opts.override_list ==
NULL)) {
break;
}
+
res = ap_parse_htaccess(&htaccess_conf, r, opts.override,
- opts.override_opts,
+ opts.override_opts, opts.override_list,
apr_pstrdup(r->pool, r->filename),
sconf->access_name);
if (res) {
Index: httpd-trunk.AllowOverrideList/include/http_config.h
===================================================================
--- httpd-trunk.AllowOverrideList/include/http_config.h (revision 1149429)
+++ httpd-trunk.AllowOverrideList/include/http_config.h (working copy)
@@ -28,6 +28,7 @@
#include "util_cfgtree.h"
#include "ap_config.h"
+#include "apr_tables.h"
#ifdef __cplusplus
extern "C" {
@@ -282,6 +283,8 @@
int override;
/** Which allow-override-opts bits are set */
int override_opts;
+ /** Table of directives allowed per AllowOverrideList */
+ apr_table_t *override_list;
/** Which methods are <Limit>ed */
apr_int64_t limited;
/** methods which are limited */
@@ -1065,6 +1068,7 @@
* @param r The request currently being served
* @param override Which overrides are active
* @param override_opts Which allow-override-opts bits are set
+ * @param override_list Table of directives allowed for override
* @param path The path to the htaccess file
* @param access_name The list of possible names for .htaccess files
* int The status of the current request
@@ -1073,6 +1077,7 @@
request_rec *r,
int override,
int override_opts,
+ apr_table_t *override_list,
const char *path,
const char *access_name);
Index: httpd-trunk.AllowOverrideList/include/ap_mmn.h
===================================================================
--- httpd-trunk.AllowOverrideList/include/ap_mmn.h (revision 1149429)
+++ httpd-trunk.AllowOverrideList/include/ap_mmn.h (working copy)
@@ -341,14 +341,17 @@
* ap_expr_parse_cmd_mi() function, add
ap_expr_str_*() functions,
* rename AP_EXPR_FLAGS_* -> AP_EXPR_FLAG_*
* 20110702.1 (2.3.14-dev) Add ap_scan_script_header_err*_ex functions
+ * 20110721.0 (2.3.14-dev) Add override_list as parameter to ap_parse_htaccess
+ * Add member override_list to cmd_parms_struct,
+ * core_dir_config and htaccess_result
*/
#define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
#ifndef MODULE_MAGIC_NUMBER_MAJOR
-#define MODULE_MAGIC_NUMBER_MAJOR 20110702
+#define MODULE_MAGIC_NUMBER_MAJOR 20110721
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 1 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 0 /* 0...n */
/**
* Determine if the server's current MODULE_MAGIC_NUMBER is at least a
Index: httpd-trunk.AllowOverrideList/include/httpd.h
===================================================================
--- httpd-trunk.AllowOverrideList/include/httpd.h (revision 1149429)
+++ httpd-trunk.AllowOverrideList/include/httpd.h (working copy)
@@ -706,6 +706,8 @@
int override;
/** the override options allowed for the .htaccess file */
int override_opts;
+ /** Table of allowed directives for override */
+ apr_table_t *override_list;
/** the configuration directives */
struct ap_conf_vector_t *htaccess;
/** the next one, or NULL if no more; N.B. never change this */
Index: httpd-trunk.AllowOverrideList/include/http_core.h
===================================================================
--- httpd-trunk.AllowOverrideList/include/http_core.h (revision 1149429)
+++ httpd-trunk.AllowOverrideList/include/http_core.h (working copy)
@@ -31,6 +31,7 @@
#include "apr_optional.h"
#include "util_filter.h"
#include "ap_expr.h"
+#include "apr_tables.h"
#include "http_config.h"
@@ -601,6 +602,9 @@
/** per-dir log config */
struct ap_logconf *log;
+ /** Table of directives allowed per AllowOverrideList */
+ apr_table_t *override_list;
+
} core_dir_config;
/* macro to implement off by default behaviour */
