[PATCH] Fix bug in recursive authz logic (Was: Re: problem with AUTHZ_SVN_RECURSIVE in mod_authz_svn.c)

I'm crossposting this to dev@ for review of my patches. Any followup
discussion can probably stay on dev@ (remove users@ if replying, but
keep Cc-ing to Bernd).

Bernd Rinn wrote:
> I think that I have found a bug in mod_authz_svn.c of svn 1.2.1 with
> respect to operations that require AUTHZ_SVN_RECURSIVE access.

Thanks for your report and the effort you put into identifying the cause
of the problem! You are indeed right, this is a nasty bug in the authz
algorithm.

I have recently being altering the authz code quite wildly in trunk. As
such, your patch no longer applies at all (the relevant code is now in
libsvn_repos). I have ported your fix to be applicable to trunk, and
ask, if it is satisfactory to the commiters, that the fix be nominated
for inclusion in the upcoming 1.2.2 release.

As for your concerns about separators, there is no problem: paths in
authz files are pieces of URIs, and as such always use forward slashes,
whatever the platform.

Commiters: I include two patches that should be commited separately:
  - bernd_rinn_authz_bug_1.2.patch corrects the bug in a way similar to
Bernd Rinn's proposed fix, and should be nominated for inclusion in 1.2.2 .
  - bernd_rinn_authz_bug_1.3.patch alters the code of the previous patch
to use svn_path_is_ancestor, an new API of libsvn_subr introduced for
svn 1.3 . The previous patch basically reimplements this function
internal to authz.c; this patch undoes that and uses the new API.

Here are the two commit messages:

[[[
Fix a bug in the authz recursive lookup logic, reported at
http://subversion.tigris.org/servlets/ReadMsg?list=users&msgNo=35734 .

Suggested by: Bernd Rinn <bernd@sdf.lonestar.org>
Patch by: David Anderson <david.anderson@calixo.net>

* subversion/libsvn_repos/authz.c
   (authz_path_is_ancestor): New internal function.
   (authz_parse_section): use authz_path_is_ancestor to establish
     relationships between paths instead of just strncmp.
* subversion/tests/libsvn_repos/repos-test.c
   (authz): New regression test.
]]]

[[[
Port rXXXXX to the 1.3 API.

Patch by: David Anderson <david.anderson@calixo.net>

* subversion/libsvn_repos/authz.c
   (authz_path_is_ancestor): Delete internal function.
   (authz_parse_section): use svn_path_is_ancestor instead of
     authz_path_is_ancestor.
]]]

Thanks again for your help!
- Dave.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org