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 <cmpil...@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 <cmpil...@collab.net> CollabNet <> www.collab.net <> Distributed Development On Demand
signature.asc
Description: OpenPGP digital signature
