[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1203546 - /subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c

From: Stefan Sperling <stsp_at_elego.de>
Date: Fri, 18 Nov 2011 14:52:44 +0100

On Fri, Nov 18, 2011 at 01:37:24PM +0000, Philip Martin wrote:
> philip_at_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);
Received on 2011-11-18 14:53:21 CET

This is an archived mail posted to the Subversion Dev mailing list.