On 04/08/2010 09:23 PM, Philip Martin wrote:
> Kamesh Jayachandran<kamesh_at_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.
>
>
Yes, exactly.
>> 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?
>
Yes done. Attached patch has this.
>
>> + 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?
>
As said in my other email this works without any issue, the comment
added in the patch details the same.
> I think this check would make more sense in access_checker rather than
> req_check_access.
>
May be, I prefer req_check_access as there are 3 callers for this function.
> The code needs a comment to say why no access control is neccessary in
> this case.
>
>
Check my new comment in the attached patch.
The attached patch fixes one segfault(introduced in my earlier patch) also.
With regards
Kamesh Jayachandran
Received on 2010-04-09 18:05:46 CEST