subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Noorul Islam K M <noo...@collab.net>
Subject Re: Input validation observations
Date Fri, 03 Dec 2010 09:34:26 GMT
Julian Foad <julian.foad@wandisco.com> writes:

> On Thu, 2010-12-02 at 13:58 +0530, Noorul Islam K M wrote:
>
>> Julian Foad <julianfoad@btopenworld.com> writes:
>> 
>> > On Tue, 2010-11-30 at 18:42 +0530, Noorul Islam K M wrote:
>> >
>> >> Julian Foad <julian.foad@wandisco.com> writes:
>> >> 
>> >> > I tried some potentially invalid inputs to "svn" a week or two ago
and
>> >> > made notes on what I found.  Just posting here in case anyone wants
to
>> >> > do something about one or more of them.
>> >> >
>> >> > Noorul, I'm including you in the "To" addresses because you said you
>> >> > were looking for more small tasks to do, so feel free to pick one of
>> >> > these if you want to.
>> >> 
>> >> Thank you! I will start working on these one by one.
>> >
>> > Great!
>> >
>> >
>> >> > Where I end with a question mark, it means I'm not sure if we want
this
>> >> > change, it's just a suggestion.
>> >> >
>> >> >   * "svn checkout ^/ ^/y" -> "A asf/cxf, A asf/cxf/utils, ...".
 (Don't
>> >> > try this without being ready on the Ctrl-C or Ctrl-\!)  It seems to
>> >> > ignore "^/y" and create ./(basename(^/)); should fail: "'^/y' is not
a
>> >> > WC path".
>> >> >
>> >> >   * "svn checkout ^/subversion/trunk/build ^/y" -> "Checked out
revision
>> >> > 1040465. URL 'https://svn.apache.org/repos/asf/y' doesn't exist".
>> >> > Bleach - that's just crazy.  Should fail: "'^/y' is not a WC path".
>> >> >
>> >> >   * "svn copy a ^/b c" doesn't detect the mixed source types in cl,
only
>> >> > in lib; should reject them at CLI level?
>> >> >
>> >> >   * "svn info ^/b" -> "Not a valid URL"; should be "path '/b' not
found
>> >> > in revision REV"?
>> >> >
>> >> >   * "svn mkdir ^/ a" -> "Illegal repository URL 'a'"; should say
"can't
>> >> > mix URL and local targets"?
>> >> >
>> >> >   * "svn mkdir a ^/" -> "Can't create directory
>> >> > 'https:/svn.apache.org/repos/asf'"; should not print the URL as if
it's
>> >> > a local path.
>> >> >
>> >> >   * "svn mv ^/ ^/" -> "Cannot move path
>> >> > 'https:/svn.apache.org/repos/asf' into itself"; should not print the
URL
>> >> > as if it's a local path.
>> >> >
>> >> >   * "svn update ^/a" -> "Skipped
>> >> > 'https://svn.apache.org/repos/asf/a' ..."; should fail: "'^/a' is not
a
>> >> > WC path"?
>> >> >
>> >> 
>> >> svn help update says that the argument should be a PATH. I think it is
>> >> right to force the user to enter local PATH.
>> >
>> > Good.  Thanks for finding that.  I agree.  That change will make some of
>> > my recent patch (r1040232) redundant: it will no longer need to remove
>> > URL targets from the array of targets, since it will just return an
>> > error if it finds any, like you already did in some of the other
>> > subcomands.
>> >
>> 
>> I further looked into the code, since update accepts multiple targets,
>> the program skips URL targets assuming that there might one or more
>> local paths as part of targets list. The skipping part is intentional
>> and I am not sure whether we should change this behavior. 
>
> I think we should change this behaviour and make "svn update" throw an
> error if any target is a URL.
>

Attached is the patch for same.

Log 
[[[

Make 'svn update' verify that URLs are not passed as targets.

* subversion/svn/update-cmd.c, 
  subversion/libsvn_client/update.c:
  (svn_cl__update, svn_client_update4): Raise an error if a URL was
  passed. Remove code that notifies 'Skipped' message for URL targets.

* subversion/tests/cmdline/input_validation_tests.py
  (invalid_update_targets, test_list): New test

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

]]]

Thanks and Regards
Noorul


Mime
View raw message