Kamesh Jayachandran <kam...@collab.net> writes:

> [issue2753] Fix issue 2753.

Let me see if I understand: The issue is that when SVNListParentPath
and AuthzSVNAccessFile are configured then GET requests for the parent
path get passed through the authz stuff.  This is a bug because the
authz file doesn't control parent path.

Your patch recognises this request and avoids doing the authz check.

> Relax requests aimed at the repo Parent path from authz control.
>
> * subversion/mod_authz_svn/mod_authz_svn.c
>   (req_check_access): When canonicalized 'uri' and 'root_path' are same
>    allow the request.
> ]]]
>
> If there are no objections will commit this in next couple of days.
>
> Thanks
> With regards
> Kamesh Jayachandran
>
> Index: subversion/mod_authz_svn/mod_authz_svn.c
> ===================================================================
> --- subversion/mod_authz_svn/mod_authz_svn.c  (revision 931820)
> +++ subversion/mod_authz_svn/mod_authz_svn.c  (working copy)
> @@ -210,6 +210,8 @@
>    svn_authz_t *access_conf = NULL;
>    svn_error_t *svn_err;
>    char errbuf[256];
> +  const char *canonicalized_uri;
> +  const char *canonicalized_root_path;
>    const char *username_to_authorize = get_username_to_authorize(r, conf);
>  
>    switch (r->method_number)
> @@ -249,6 +251,15 @@
>          break;
>      }
>  
> +  canonicalized_uri = svn_uri_canonicalize(r->uri, r->pool);
> +  canonicalized_root_path = svn_uri_canonicalize(conf->base_path, r->pool);

Can conf->base_path be canonicalised once in
create_authz_svn_dir_config rather than for every request?

> +  if (strcmp(canonicalized_uri, canonicalized_root_path) == 0)
> +    {
> +      /*Do no access control when root_path(as configured in <Location>) and 
> +       given uri are same.*/
> +      return OK;
> +    }

What happens if SVNParentPath is not being used?  Is base_path is the
root of the repository?  Does this disable authz on the root of that
repository? Perhaps you should be checking dav_svn__get_list_parentpath?

I think this check would make more sense in access_checker rather than
req_check_access.

The code needs a comment to say why no access control is neccessary in
this case.

> +
>    dav_err = dav_svn_split_uri(r,
>                                r->uri,
>                                conf->base_path,

-- 
Philip

Reply via email to