Re: svn commit: r916286 - in /subversion/trunk/subversion/mod_dav_svn: dav_svn.h mirror.c mod_dav_svn.c

[Kamesh Jayachandran <ka...@collab.net>]

On 02/26/2010 08:59 PM, Kamesh Jayachandran wrote:
> On 02/25/2010 11:36 PM, Julian Foad wrote:
>> On Thu, 2010-02-25 at 20:45 +0530, Kamesh Jayachandran wrote:
>>> On 02/25/2010 08:29 PM, Julian Foad wrote:
>>>> kameshj@apache.org wrote:
>> [...]
>>>>> -    if (strcmp(uri.path, root_dir) == 0) {
>>>>> +    if (uri.path)
>>>>> +        canonicalized_uri = svn_dirent_canonicalize(uri.path, 
>>>>> r->pool);
>>>>>
>>>> Oops, you called "dirent_canonicalize" on a URI.
>>> Is there any uri canonicalize function?.
>> svn_uri_canonicalize() if it's a URI (in which non-URI characters must
>> be escaped as '%XX').
>>
>
> Ok, I wanted to see the failure in my eyes before attempting to fix 
> the same.
>
> Following are my observations,
>
> <Location "/svn 1/">
>   DAV svn
>   SVNParentPath /repositories
> </Location>
> <Location "/svn 2/">
>   DAV svn
>   SVNParentPath /repositories-slave
>   SVNMasterURI "http://localhost/svn 1"
> </Location>
>
> I could not see the difference between "svn_uri_canonicalize()" and  
> svn_dirent_canonicalize() for the above configuration, by the way both 
> fails while proxying.

Fixed the svn_dirent_canonicalize->svn_uri_canonicalize in r917512.



Fixed the write through proxy *not* proxying the request to master if 
the configured location has the encodable content in r917523

With regards
Kamesh Jayachandran
>
> I have an upcoming local patch in progress attempting to fix the same.
>
>
>>> FWIW here uri.path is the PATH portion of the URL, i.e something like
>>> /svn/blah/blah
>> svn_relpath_canonicalize() if it's a "relpath" (see<svn_dirent_uri.h>
>> for definitions).
>>
>> It's definitely wrong to use a "dirent" function on the path portion of
>> a URL.
>>
>> [...]
>>
>>>>> Modified: subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c
>>>>> URL: 
>>>>> http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c?rev=916286&r1=916285&r2=916286&view=diff 
>>>>>
>>>>> ============================================================================== 
>>>>>
>>>>> --- subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c (original)
>>>>> +++ subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c Thu Feb 
>>>>> 25 13:40:22 2010
>>>>> @@ -169,7 +169,8 @@
>>>>>      /* NOTE: dir==NULL creates the default per-dir config */
>>>>>      dir_conf_t *conf = apr_pcalloc(p, sizeof(*conf));
>>>>>
>>>>> -  conf->root_dir = dir;
>>>>> +  if (dir)
>>>>> +    conf->root_dir = svn_dirent_canonicalize(dir, p);
>>>>>
>>>> And is this "root_dir" meant to be a disk path or a URI?  I'm not 
>>>> sure,
>>>> myself.
>>>>
>>> May be the disk path if the context is driven by<Directory
>>> /some/path/to/repo>, though I never tried that way.
>>>
>>> For the<Location /svn/>  configuration root_dir is '/svn'(after
>>> canonicaliztion.
>> We need to know what type of path it is and use the correct
>> canonicalization function(s).  Maybe it requires two different functions
>> depending on ... something.
>>
>
> IIUC only way to configure the SVN url is via <Location>, so no issues.
>
> With regards
> Kamesh Jayachandran
>> - Julian
>>
>>
>