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

Re: Input validation observations

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Mon, 06 Dec 2010 15:57:13 +0000

On Sat, 2010-12-04 at 13:01 +0530, Noorul Islam K M wrote:
> Julian Foad <julian.foad_at_wandisco.com> writes:
>
> > Noorul Islam K M wrote:
> >
> >> Julian Foad <julian.foad_at_wandisco.com> writes:
> >> > * "svn mkdir ^/ a" -> "Illegal repository URL 'a'"; should say "can't
> >> > mix URL and local targets"?
> > [...]
> >> Make 'svn mkdir' verify that both working copy paths and URLs are
> >> not passed.
> >>
> >> * subversion/svn/mkdir-cmd.c,
> >> subversion/libsvn_client/add.c
> >> (svn_cl__mkdir, svn_client_mkdir4): Raise an error if both working
> >> copy paths and URLs are passed.
> >>
> >> * subversion/tests/cmdline/input_validation_tests.py
> >> (invalid_mkdir_targets, test_list): New test
> > [...]
> >> Index: subversion/svn/mkdir-cmd.c
> >> ===================================================================
> >> --- subversion/svn/mkdir-cmd.c (revision 1041293)
> >> +++ subversion/svn/mkdir-cmd.c (working copy)
> >> @@ -48,6 +48,8 @@
> >> + svn_boolean_t wc_present = FALSE, url_present = FALSE;
> >> + int i;
> >> @@ -56,6 +58,22 @@
> >> + /* Check to see if at least one of our paths is a working copy
> >> + path or a repository url. */
> >> + for (i = 0; i < targets->nelts; ++i)
> >> + {
> >> + const char *target = APR_ARRAY_IDX(targets, i, const char *);
> >> + if (! svn_path_is_url(target))
> >> + wc_present = TRUE;
> >> + else
> >> + url_present = TRUE;
> >> + }
> >> +
> >> + if (url_present && wc_present)
> >> + return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> >> + _("Cannot mix repository and working copy "
> >> + "targets"));
> >
> > This is fine.
> >
> > The same code already exists in three other files and equivalent but
> > different code also exists in at least delete-cmd.c and probably other
> > files. I think it is time to factor it out. We can do that in a
> > subsequent patch.
>
> Do you mean we need to come up with new function that will do this check
> and return the error message?

Yes please.

- Julian
Received on 2010-12-06 16:57:56 CET

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.