Stefan Sperling wrote:
> On Mon, Aug 04, 2008 at 10:13:05AM -0400, C. Michael Pilato wrote:
>> stsp_at_tigris.org wrote:
>>> Author: stsp
>>> Date: Mon Aug 4 03:14:02 2008
>>> New Revision: 32360
>>>
>>> Log:
>>> Fix a bug causing mod_dav_svn to eat all memory during a merge
>>> operation when SVNPathAuthz is set to 'short_circuit'.
>>> According to the reporter, this bug is only triggered when
>>> using a 1.5 client.
>>>
>>> * subversion/libsvn_repos/rev_hunt.c
>>> (svn_repos_node_location_segments): authz_read requires that
>>> its path argument starts with a leading slash, so make sure
>>> the path passed to authz_read starts with '/'.
>> You know, if authz_read requires as much, I would *strongly* suggest having
>> that function (or at least the implementations thereof which we provide)
>> assert that the input is valid, perhaps using Julian's new
>> assert-is-sometimes-an-error macro.
>
> Oh, absolutely, exactly my thoughts.
> In fact, I've already proposed doing this in IRC while you
> weren't around :)
>
> I have no time right now to do that, though. Can you do it?
> Otherwise I'll keep it on my mental TODO list, or even better
> open an issue in the tracker.
>
> Note that the docstring of the callback in svn_repos.h does
> not mention this restriction either, so we'll probably want to patch
> the docs as well.
> Thereby possibly restricting public API semantics retroactively... :-/
>
> Stefan
I took at glance at this today, adding assert() statements in the
authz_read_func callback implementations. I got a few test failures. That
led me to question whether or not this really is a requirement of the authz
subsystem. So far I've not seen anything to back that claim, which is
really weird.
For example, given the fixed format of the authz files used by mod_authz_svn
and svnserve, I would have expected to see some kind of documentation that
stated that incoming paths to be checked for access had to be canonical
absolute FS paths, but I saw no such thing. Nor did I see code inside the
implementation that attempted to verify as much or automatically correct
paths that weren't absolute. Anyway, I've backed off of this again, and
will file an issue that demands some documentation clarity and input
validation in these codepaths.
--
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1343078
Received on 2009-03-17 20:53:15 CET