On 01/26/2011 12:38 PM, C. Michael Pilato wrote:
> On 01/24/2011 08:52 PM, Justin Erenkrantz wrote:
>> On Mon, Jan 24, 2011 at 2:22 PM, C. Michael Pilato <cmpilato_at_collab.net> wrote:
>>> [Using dev@ as a public TODO list to avoid pushing stack on a task.]
>>>
>>> In mod_dav_svn/mirror.c:dav_svn__location_body_filter() and
>>> dav_svn__location_in_filter() are code blocks like this:
>>>
>>> if (uri.path)
>>> canonicalized_uri = svn_urlpath__canonicalize(uri.path, r->pool);
>>> else
>>> canonicalized_uri = uri.path;
>>> if (strcmp(canonicalized_uri, root_dir) == 0) {
>>> [...]
>>>
>>> So ... if uri.path == NULL, then canonicalized_uri is set to NULL, and then
>>> that NULL is used in a strcmp(). Won't that SEGFAULT?
>>
>> It'd be difficult (if not outright impossible) to hit that else case.
>> Follow apr_uri_parse and apr_pstrmemdup. Also know that we don't hit
>> this code block if master_uri isn't set. The original code I wrote
>> was just a straight strcmp - I believe the check for null is spurious.
>
> My reading of the code says that uri.path can be NULL if the master_uri is
> something like "http://server". I'll see if I can verify that.
Yep. Definitely SEGFAULTs here if I set SVNMasterURI to something like the
above. I wondered if maybe setting canonicalized_uri to the empty string
here would serve as a workaround, but unfortunately, when I tried to setup a
configuration to test this, I found myself in some endless loop, leaking
memory like a sieve and rushing towards a machine crash. That's fun.
Maybe setting it to "/" would work better?
Or perhaps not. After all, these filters aim to replace instances of the
slave's root path with the master's (and vice-versa). A string replace of
"" seems kinda hopeless (and maybe that's why I saw the endless loop). A
string replace of "/" seems likewise pretty awful.
I'm not sure how to move forward. Should we require that SVNMasterURI
values point to something other than the server root?
--
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Received on 2011-01-26 19:29:01 CET