I am top posting since my mails are not getting through with my mail client. I
am using web based client which lacks quoting feature.
This is in response to
* "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.
Attached is the patch which improves the error message. Also added missing
test. All the tests pass with 'make check'.
Log
[[[
Make 'svn move' display correct error message while moving any
path/URL onto or into itself. Add missing test.
* subversion/libsvn_client/copy.c
(try_copy): Display error message based on source type,
(repos_to_repos_copy): Remove redundant code.
* subversion/tests/cmdline/copy_tests.py
(move_wc_and_repo_dir_to_itself, test_list): New test
Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
]]]
Thanks and Regards
Noorul
-----Original Message-----
From: Julian Foad [mailto:julian.f...@wandisco.com]
Sent: Tue 11/30/2010 5:18 PM
To: Subversion Development; noorul Islam. Kamal Malmiyoda
Subject: Input validation observations
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.
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"?
- Julian
Index: subversion/tests/cmdline/copy_tests.py
===================================================================
--- subversion/tests/cmdline/copy_tests.py (revision 1045067)
+++ subversion/tests/cmdline/copy_tests.py (working copy)
@@ -4942,7 +4942,22 @@
Item(status=' ', wc_rev='-', copied='+')})
svntest.actions.run_and_verify_status(sbox.wc_dir, expected_status)
+def move_wc_and_repo_dir_to_itself(sbox):
+ "move wc and repo dir to itself"
+ sbox.build(read_only = True)
+ wc_dir = os.path.join(sbox.wc_dir, 'A')
+ repo_url = sbox.repo_url + '/A'
+ # try to move wc dir to itself
+ svntest.actions.run_and_verify_svn(None, [],
+ '.*Cannot move path.* into itself.*',
+ 'move', wc_dir, wc_dir)
+
+ # try to move repo dir to itself
+ svntest.actions.run_and_verify_svn(None, [],
+ '.*Cannot move URL.* into itself.*',
+ 'move', repo_url, repo_url)
+
########################################################################
# Run the tests
@@ -5045,6 +5060,7 @@
copy_repos_over_deleted_other_kind,
copy_wc_over_deleted_same_kind,
copy_wc_over_deleted_other_kind,
+ move_wc_and_repo_dir_to_itself,
]
if __name__ == '__main__':
Index: subversion/libsvn_client/copy.c
===================================================================
--- subversion/libsvn_client/copy.c (revision 1045067)
+++ subversion/libsvn_client/copy.c (working copy)
@@ -983,10 +983,6 @@
}
else if (strcmp(pair->src_abspath_or_url, top_url) == 0)
{
- if (is_move)
- return svn_error_createf(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
- _("Cannot move URL '%s' into itself"),
- pair->src_abspath_or_url);
src_rel = "";
SVN_ERR(svn_ra_check_path(ra_session, src_rel, pair->src_revnum,
&info->src_kind, pool));
@@ -2077,7 +2073,7 @@
"supported"));
}
- /* Disallow moving any path onto or into itself. */
+ /* Disallow moving any path/URL onto or into itself. */
for (i = 0; i < copy_pairs->nelts; i++)
{
svn_client__copy_pair_t *pair = APR_ARRAY_IDX(copy_pairs, i,
@@ -2085,10 +2081,14 @@
if (strcmp(pair->src_abspath_or_url,
pair->dst_abspath_or_url) == 0)
- return svn_error_createf
- (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
- _("Cannot move path '%s' into itself"),
- svn_dirent_local_style(pair->src_abspath_or_url, pool));
+ return svn_error_createf(
+ SVN_ERR_UNSUPPORTED_FEATURE, NULL,
+ srcs_are_urls ?
+ _("Cannot move URL '%s' into itself") :
+ _("Cannot move path '%s' into itself"),
+ srcs_are_urls ?
+ pair->src_abspath_or_url :
+ svn_dirent_local_style(pair->src_abspath_or_url, pool));
}
}
else
