On Oct 6, 2004, at 8:47 PM, Ben Collins-Sussman wrote:
> I finally got around to reading the r11211 patch, which is supposed to
> fix the 'svn ls' over http:// performance problem.
>
> I'm worried: if a certain child-node isn't readable, then does the
> whole RA->get_dir() fail now? If so, that's a new behavior. We
> should either be showing it (i.e. leaking the existence child, a known
> bug), or omitting it.
>
Aha, sorry. I understand the patch now. (It's always easier to
understand when looking at it in a graphical diff tool!) If the
child-path isn't readable, we just return an 'undefined' value for the
author and date attributes. The path is still returned as a child of
the directory, as always.
But I've discovered a theoretical problem with the patch, and have
talked with Tobias about it in IRC.
The problem is that we're now testing the readability of the child-path
in whatever revision of the parent directory we happen to be fetching
(most likely HEAD). That's not correct. The issue here is whether or
not the child-path is readable in it's *created-rev*. If you think
about it, this is a very clever solution: the child-path is guaranteed
to exist in its created-rev's list of changed-paths. Therefore, if
(created-rev, child-path) is readable, we know that it's safe to return
the svn:author and svn:date props attached to created-rev. If the pair
is unreadable, then we know we cannot return those revprops. Instead
of having to search down the *entire* list of changed-paths in the
creative-rev, we're checking exactly one changed-path. Very nice!
Tobias is going to fix this later tonight; he just needs to tweak the
patch so that he's calling dav_svn_authz_read() on the child's
created-rev, not the rev opened by the RA session. Then he'll add this
change to the 1.1.1 STATUS file for voting.
Again, I point out that this is a theoretical problem only. Packagers
who have already distributed this patch don't need to revoke anything.
You see, we've written everything to assume that authz systems are
checking readability of (rev, path). In reality, no such authz system
exists yet. mod_authz_svn only controls access to paths, not (rev,
path) pairs. That's why Tobias' patch is working fine. Still,
somebody *could* write such an authz system someday, so we still have
to invoke the authz_read callback correctly, as if particular revisions
mattered.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Oct 7 16:57:25 2004