On Fri, Nov 18, 2011 at 01:37:24PM +0000, Philip Martin wrote:
> phi...@apache.org writes:
>
> > Author: philip
> > Date: Fri Nov 18 09:09:56 2011
> > New Revision: 1203546
>
> > Also, a nested Location inside an SVNPath Location is ambiguous so
> > prevent access and log the problem.
>
> > + if (parent->fs_path)
> > + {
> > + /* Nesting inside SVNPath is ambiguous so prevent access. */
> > + newconf->fs_path = newconf->fs_parent_path = NULL;
> > +
> > + ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL,
> > + "mod_dav_svn: invalid nested Location '%s' inside "
> > + "SVNPath Location '%s'",
> > + child->root_dir, parent->root_dir);
> > + }
>
> I'm having second thoughts about this bit. Given
>
> <Location /foo>
> ...
> SVNPath /some/foo
> </Location>
> <Location /foo/bar>
> ...
> SVNPath /some/bar
> </Location>
>
> there is obvious ambiguity about the URL /foo/bar. Is it the root of
> the repository at /foo/bar or is it /bar in the repository at /foo?
> However provided I'm not trying to use /bar inside the /foo repository
> then checkouts, updates and commits work for both repositories. (Some
> operations even work on /bar inside /foo, although not enough to be
> useful.)
>
> What that means is that some people may be using this setup, either
> intentionally or accidentally. Should we continue to support them? If
> we choose not to support them in 1.8 can we make the change in 1.7.2 as
> well?
I didn't notice this when approving the fix but we should really be using
APLOG_WARNING instead of APLOG_ERR. It's not supposed to be an error,
just a warning.
I think logging a warning is entirely appropriate.
There is a potential problem here, and admins might not be aware of it.
We might want to rephrase the message to make its intention clear.
Suggestion (not tested);
ap_log_error(APLOG_MARK, APLOG_WARNING, 0, NULL,
"mod_dav_svn: nested Location '%s' inside "
"SVNPath Location '%s'; this hinders access to a file "
"or directory '%s' inside the repository '%s'",
child->root_dir, parent->root_dir,
svn_dirent_basename(child->root_dir, scratch_pool),
parent->fs_path);
