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
> 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
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.
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
Patch by: Bernd Rinn <firstname.lastname@example.org>
(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: email@example.com
For additional commands, e-mail: firstname.lastname@example.org
Received on Sat Jul 23 19:11:44 2005