subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Julian Foad <julian.f...@wandisco.com>
Subject Re: Input validation observations
Date Fri, 03 Dec 2010 12:17:00 GMT
Noorul Islam K M wrote:
> Julian Foad <julian.foad@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.

> Index: subversion/libsvn_client/add.c
> ===================================================================
> --- subversion/libsvn_client/add.c	(revision 1041293)
> +++ subversion/libsvn_client/add.c	(working copy)
> @@ -875,9 +875,28 @@
>                    svn_client_ctx_t *ctx,
>                    apr_pool_t *pool)
>  {
> +  svn_boolean_t wc_present = FALSE, url_present = FALSE;
> +  int i;
> +
>    if (! paths->nelts)
>      return SVN_NO_ERROR;
>  
> +  /* Check to see if at least one of our paths is a working copy
> +     path or a repository url. */
> +  for (i = 0; i < paths->nelts; ++i)
> +    {
> +      const char *path = APR_ARRAY_IDX(paths, i, const char *);
> +      if (! svn_path_is_url(path))
> +       wc_present = TRUE;
> +      else
> +       url_present = TRUE;
> +    }
> +
> +  if (url_present && wc_present)
> +    return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL,
> +                             _("Cannot mix repository and working copy "
> +                               "targets"));

Does this validation make the check at the beginning of mkdir_urls()
redundant?  If so, we should get rid of it.

- Julian



Mime
View raw message