httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Shane Holden <dpej...@apache.org>
Subject Re: svn commit: r667651 - /httpd/httpd/trunk/modules/aaa/mod_authz_core.c
Date Fri, 11 Jul 2008 06:26:27 GMT
I tried to build Apache from trunk tonight and noticed that this patch 
broke something.  I'm getting a 403 error when trying to browse to a 
clean install.  I'm by no means an expert here, but I noticed a few 
things which are noted below...

bnicholes@apache.org wrote:
> Author: bnicholes
> Date: Fri Jun 13 13:59:10 2008
> New Revision: 667651
>
> URL: http://svn.apache.org/viewvc?rev=667651&view=rev
> Log:
> Switch the default base authz logic operation to 'AND' rather than 'OR'.  This should
allow directory authz rules merging to be more restrictive in sub-directories
>
> 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=667651&r1=667650&r2=667651&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/aaa/mod_authz_core.c (original)
> +++ httpd/httpd/trunk/modules/aaa/mod_authz_core.c Fri Jun 13 13:59:10 2008
> @@ -111,13 +111,16 @@
>  static const char *merge_authz_provider(authz_core_dir_conf *conf, authz_provider_list
*newp);
>  static void walk_merge_provider_list(apr_pool_t *a, authz_core_dir_conf *conf, authz_provider_list
*providers);
>  
> +#define BASE_REQ_STATE AUTHZ_REQSTATE_ALL
> +#define BASE_REQ_LEVEL 0
> +
>  static void *create_authz_core_dir_config(apr_pool_t *p, char *dummy)
>  {
>      authz_core_dir_conf *conf =
>              (authz_core_dir_conf *)apr_pcalloc(p, sizeof(authz_core_dir_conf));
>  
> -    conf->req_state = AUTHZ_REQSTATE_ONE;
> -    conf->req_state_level = 0;
> +    conf->req_state = BASE_REQ_STATE;
> +    conf->req_state_level = BASE_REQ_LEVEL;
>      conf->merge_rules = 1;
>      return (void *)conf;
>  }
>   
Not sure if this was intentional... but the default went from 
authz_reqstate_one to authz_reqstate_all.  If I change base_req_state to 
authz_reqstate_one the 403 disappears, but since I don't know much about 
how this is suppose to work it might not be the correct fix.
> @@ -180,11 +183,21 @@
>  
>      /* Walk all of the elements recursively to allow each existing
>          element to be copied and merged into the final configuration.*/
> -    if (providers->one_next) {
> -        walk_merge_provider_list (a, conf, providers->one_next);
> +    if (BASE_REQ_STATE == AUTHZ_REQSTATE_ONE) {
> +        if (providers->one_next) {
> +            walk_merge_provider_list (a, conf, providers->one_next);
> +        }
> +        if (providers->all_next) {
> +            walk_merge_provider_list (a, conf, providers->all_next);
> +        }
>      }
> -    if (providers->all_next) {
> -        walk_merge_provider_list (a, conf, providers->all_next);
> +    else {
> +        if (providers->all_next) {
> +            walk_merge_provider_list (a, conf, providers->all_next);
> +        }
> +        if (providers->one_next) {
> +            walk_merge_provider_list (a, conf, providers->one_next);
> +        }
>      }
>   
base_req_state == authz_reqstate_one will always fail.  was this 
comparison suppose to be conf->req_state == authz_reqstate_one?
>  
>      return;
> @@ -200,18 +213,30 @@
>          authz_provider_list *last = conf->providers;
>          int level = conf->req_state_level;
>  
> -        /* if the level is 0 then take care of the implicit 'or'
> +        /* if the level is the base level then take care of the implicit 
>           * operation at this level. 
>           */
> -        if (level == 0) {
> -            /* Just run through the Require_one list and add the
> -             * node 
> -             */
> -            while (last->one_next) {
> -                last = last->one_next;
> +        if (level == BASE_REQ_LEVEL) {
> +            if (conf->req_state == AUTHZ_REQSTATE_ONE) {
> +                /* Just run through the Require_one list and add the
> +                 * node 
> +                 */
> +                while (last->one_next) {
> +                    last = last->one_next;
> +                }
> +                last->one_next = newp;
> +            }
> +            else {
> +                /* Just run through the Require_all list and add the
> +                 * node 
> +                 */
> +                while (last->all_next) {
> +                    last = last->all_next;
> +                }
> +                last->all_next = newp;
>              }
> -            last->one_next = newp;
>          } 
> +
>          /* if the last nodes level is greater than the new nodes 
>           *  level, then we need to insert the new node at this
>           *  point.  The req_state of the new node determine
>
>
>
>
>   


Mime
View raw message