On Thu, 2010-02-25 at 20:45 +0530, Kamesh Jayachandran wrote:
> On 02/25/2010 08:29 PM, Julian Foad wrote:
> > kame...@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').
> 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.
- Julian
