Bernd Rinn wrote:
> Thanks for your quick response! I guess the patch for svn 1.2.x should be for
> mod_authz_svn/mod_authz_svn.c instead of libsvn_repos/authz.c (since this
> doesn't exist in branches/1.2.x). For convenience I have attached a new
> version of your patch bernd_rinn_authz_bug_1.2.patch.
You are correct. My patch was assuming that my changes to the authz
routines would go in the next 1.2 micro release, but since they add
public APIs, I doubt they will.
Thanks for producing the correct patch for this. Included at the end of
my mail is the corresponding commit log for review and inclusion in the
1.2.x branch.
> there is one case that I do not understand: when path1 == "/foo/" and path2 ==
> "/foo", the function will return FALSE. On the other hand, when path1 ==
> "/foo" and path2 == "/foo/", the function will return TRUE. Is this on
> purpose?
Well, /foo/ is beneath /foo, so the second case is logical. The first
case failing is in my opinion due to one of an oversight or conforming
interpretation to what a / means in the context of an URI/path, meaning
that you can't just say "well /dir/ is the same as /dir". I'm in favour
of the latter:
> in AuthzSVNAccessFile does not apply to repos_path == "/dir1/" (at least if there isn't some
> mangling that removes trailing slashes from repos_path that I have
> overlooked). This looks like unexpected behavior to me.
That case can never occur, as the repos_path is a piece of a
canonicalized URI, which iirc has no trailing slash. So both sections
named [/dir/] and [/dir] will match using svn_path_is_ancestor. This is
what brings me to believe that the behaviour of svn_path_is_ancestor is
intentional and conforms with URI naming standards.
kfogel and other commiters reviewing: Bernd's new patch for 1.2 is good
for me. There is no regression test included, but adding such a test
with authz completely integrated in mod_authz_svn as it is in 1.2 would
be bothersome and probably not worth the effort if the test is in trunk.
I would however like an experienced second opinion concerning the
behaviour of svn_path_is_ancestor.
- Dave
[[[
Fix the same bug that was fixed on trunk in rXXXXX. The bug made authz
incorrectly consider some authz sections which didn't apply during
recursive lookups.
Patch by: Bernd Rinn <bernd@sdf.lonestar.org>
* subversion/mod_authz_svn/mod_authz_svn.c
(authz_path_is_ancestor): New internal function. Implements the
missing public API svn_path_is_ancestor, introduced in trunk.
(parse_authz_section): use authz_path_is_ancestor to establish
relationships between paths instead of just strncmp.
]]]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Jul 23 19:11:44 2005