[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: Noorul Islam K M <noorul_at_collab.net>
Date: Fri, 10 Dec 2010 15:03:56 +0530

Noorul Islam K M <noorul_at_collab.net> writes:

> Julian Foad <julian.foad_at_wandisco.com> writes:
>
>> 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.
>
> Attached is the patch which has the new functions to replace the
> existing blocks. All tests pass when tested with 'make check'. No need
> for test cases as they are already available. Also I have not modified
> libsvn_client/copy.c and svn/diff-cmd.c because they are not straight
> forward. I will go through them again and will come up with follow-up
> patch.
>

Attached is the patch for svn/diff-cmd.c. All tests pass.

Log
[[[
Follow-up to r1044028. Make use of new function.

* subversion/svn/diff-cmd.c
  (svn_cl__diff): Make use of new function
    svn_cl__assert_homogeneous_target_type(). Tweak existing logic which
    checks to see whether working copy path is present in targets.

Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
]]]

Thanks and Regards
Noorul

Index: subversion/svn/diff-cmd.c
===================================================================
--- subversion/svn/diff-cmd.c (revision 1044185)
+++ subversion/svn/diff-cmd.c (working copy)
@@ -260,7 +260,7 @@
     }
   else
     {
- svn_boolean_t working_copy_present = FALSE, url_present = FALSE;
+ svn_boolean_t working_copy_present = FALSE;
 
       /* The 'svn diff [-r N[:M]] [TARGET[@REV]...]' case matches. */
 
@@ -272,22 +272,20 @@
       old_target = "";
       new_target = "";
 
+ svn_cl__assert_homogeneous_target_type(targets);
+
       /* Check to see if at least one of our paths is a working copy
          path. */
       for (i = 0; i < targets->nelts; ++i)
         {
           const char *path = APR_ARRAY_IDX(targets, i, const char *);
           if (! svn_path_is_url(path))
- working_copy_present = TRUE;
- else
- url_present = TRUE;
+ {
+ working_copy_present = TRUE;
+ break;
+ }
         }
 
- if (url_present && working_copy_present)
- return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
- _("Cannot mix repository and working copy "
- "targets"));
-
       if (opt_state->start_revision.kind == svn_opt_revision_unspecified
           && working_copy_present)
           opt_state->start_revision.kind = svn_opt_revision_base;
Received on 2010-12-10 10:36:36 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.