sussman@tigris.org writes:
> Author: sussman
> Date: Thu Jul 28 15:18:00 2005
> New Revision: 15463
>
> Modified:
> trunk/subversion/libsvn_repos/authz.c
> trunk/subversion/tests/libsvn_repos/repos-test.c
>
> Log:
> Bugfix for overly-restrict rejection by recursive authz lookup algorithm.
I have some questions about this change, see below. Also, note that I
tweaked "overly-restrict" to "overly-strict" in the log message.
> Original bug report here:
> http://subversion.tigris.org/servlets/ReadMsg?listName=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_parse_section): use svn_path_is_ancestor to establish
> relationships between paths instead of just strncmp.
>
> * subversion/tests/libsvn_repos/repos-test.c
> (authz): New regression test.
>
> --- trunk/subversion/libsvn_repos/authz.c (original)
> +++ trunk/subversion/libsvn_repos/authz.c Thu Jul 28 15:18:00 2005
> @@ -214,10 +214,10 @@
> svn_boolean_t conclusive;
>
> /* Does the section apply to us? */
> - if (strncmp (section_name, b->qualified_repos_path,
> - strlen (b->qualified_repos_path)) != 0
> - && strncmp (section_name, b->repos_path,
> - strlen (b->repos_path)) != 0)
> + if (svn_path_is_ancestor (b->qualified_repos_path,
> + section_name) == FALSE
> + && svn_path_is_ancestor (b->repos_path,
> + section_name) == FALSE)
> return TRUE;
At first, I didn't understand why we test both b->qualified_repos_path
and b->repos_path (both in the original code and now). The
authz_lookup_baton (that's b's type) documentation says:
/* The path in the repository to authorize. */
const char *repos_path;
/* repos_path prefixed by the repository name. */
const char *qualified_repos_path;
I thought: does this mean we allow people to specify authz
restrictions in two ways, and that since there's no way to tell which
way they chose, we have to check for both? That would seem an overly
permissive format, if so.
But I think I'm working out the answer as I type...
(Feel free to ignore the rest of this mail, but if you're feeling
generous and want to check my logic [cough Ben Collins-Sussman cough],
that'd be great.)
http://svnbook.red-bean.com/en/1.1/svn-book.html#svn-ch-6-sect-4.4.2
describes the AuthzSVNAccessFile format, saying:
"... the value of the section-names are either of the form
[repos-name:path] or the form [path]. If you're using the
SVNParentPath directive, then it's important to specify the
repository names in your sections. If you omit them, then a
section like [/some/dir] will match the path /some/dir in every
repository. If you're using the SVNPath directive, however, then
it's fine to only define paths in your sections -- after all,
there's only one repository."
The code in libsvn_repos/authz.c is asking "Could this section
*possibly* apply to this path? If not, just return TRUE now, because
this section cannot cause access to be denied if it doesn't even apply
to this path. Otherwise, continue on to the real checks."
Is that right?
On the assumption that it is right, I have voted for it in STATUS :-).
(That is, I have voted for the equivalent backport change.)
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Aug 5 07:32:08 2005