Re: svn commit: r1585609 - /httpd/httpd/trunk/modules/aaa/mod_authz_core.c

[Yann Ylavic]

On Tue, Apr 8, 2014 at 12:38 AM,  <bre...@apache.org> wrote:
> Author: breser
> Date: Mon Apr  7 22:38:53 2014
> New Revision: 1585609
>
> URL: http://svn.apache.org/r1585609
> Log:
> Allow Require expr to work when the expression is quoted.
>
> For example as appears in our documentation:
> Require expr "%{TIME_HOUR} -ge 9 && %{TIME_HOUR} -le 17"
>
> PR: 56235
>
> Modified:
>     httpd/httpd/trunk/modules/aaa/mod_authz_core.c
>
> Modified: httpd/httpd/trunk/modules/aaa/mod_authz_core.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_authz_core.c?rev=1585609&r1=1585608&r2=1585609&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/aaa/mod_authz_core.c (original)
> +++ httpd/httpd/trunk/modules/aaa/mod_authz_core.c Mon Apr  7 22:38:53 2014
> @@ -1062,6 +1062,16 @@ static const char *expr_parse_config(cmd
>      const char *expr_err = NULL;
>      struct require_expr_info *info = apr_pcalloc(cmd->pool, sizeof(*info));
>
> +    /* if the expression happens to be surrounded by quotes, skip them */
> +    if (require_line[0] == '"') {
> +        apr_size_t len = strlen(require_line);
> +
> +        if (require_line[len-1] == '"')
> +            require_line = apr_pstrndup(cmd->temp_pool,
> +                                        require_line + 1,
> +                                        len - 2);
> +    }
> +
>      apr_pool_userdata_setn(info, REQUIRE_EXPR_NOTE, apr_pool_cleanup_null,
>                            cmd->temp_pool);
>      info->expr = ap_expr_parse_cmd(cmd, require_line, 0, &expr_err,
>
>

This fix does not handle inner quotes (ie. \-escaping), nor other
providers (than core_authz_expr) which handle exprs or multiple
trailing words (eg. mod_authnz_ldap,
mod_authz_{dbd,groupfile,host,user,...}).

How about handling the case in the Require directive parser itself
(add_authz_provider)?

Something like:

Index: modules/aaa/mod_authz_core.c
===================================================================
--- modules/aaa/mod_authz_core.c    (revision 1674046)
+++ modules/aaa/mod_authz_core.c    (working copy)
@@ -393,7 +393,13 @@ static const char *add_authz_provider(cmd_parms *c
         section->negate = 1;
     }

-    section->provider_args = args;
+    if (args && (args[0] == '"' || args[0] == '\'')
+             && (args[strlen(args) - 1] == args[0])) {
+        section->provider_args = ap_getword_conf(cmd->pool, &args);
+    }
+    else {
+        section->provider_args = args;
+    }

     /* lookup and cache the actual provider now */
     section->provider = ap_lookup_provider(AUTHZ_PROVIDER_GROUP,
@@ -425,7 +431,7 @@ static const char *add_authz_provider(cmd_parms *c
                                AUTHZ_PROVIDER_NAME_NOTE,
                                apr_pool_cleanup_null,
                                cmd->temp_pool);
-        err = section->provider->parse_require_line(cmd, args,
+        err = section->provider->parse_require_line(cmd,
section->provider_args,
                                               &section->provider_parsed_args);

         if (err)
@@ -1069,16 +1075,6 @@ static const char *expr_parse_config(cmd_parms *cm
     const char *expr_err = NULL;
     struct require_expr_info *info = apr_pcalloc(cmd->pool, sizeof(*info));

-    /* if the expression happens to be surrounded by quotes, skip them */
-    if (require_line[0] == '"') {
-        apr_size_t len = strlen(require_line);
-
-        if (require_line[len-1] == '"')
-            require_line = apr_pstrndup(cmd->temp_pool,
-                                        require_line + 1,
-                                        len - 2);
-    }
-
     apr_pool_userdata_setn(info, REQUIRE_EXPR_NOTE, apr_pool_cleanup_null,
                           cmd->temp_pool);
     info->expr = ap_expr_parse_cmd(cmd, require_line, 0, &expr_err,
--

(the second hunk is a revert of this commit).