C. Michael Pilato wrote:
> 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.
Filed issue #3381 -
http://subversion.tigris.org/issues/show_bug.cgi?id=3381
--
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=1343127
Received on 2009-03-17 21:07:14 CET