Hello,
This is a first cut at solving issue 1954 (). The code is based on Jon
Foster's detailed patch - [RFC] Checking paths are legal
(http://svn.haxx.se/dev/archive-2004-09/0047.shtml). I have not picked
up all of Jon's Win32 code since I don't yet have a Windows XP box to
test the changes.
Please let me know if the changes are generally in the right direction.
make check passes after tab-related test in commit_uri_unsafe() in
commit.py was moved to tab_test(). It is at a coarse level using XFail,
instead of looking for specific error messages. Any pointers to the
right test functions to use?
Thanks
Sameer
First cut at issue 1954 - We should error out on pathnames added or
imported that we don't accept.
Added checks based on svn_ctype_isutf8(), svn_ctype_iscntrl().
* subversion/include/svn_path.h
subversion/libsvn_subr/path.c
(svn_path_is_valid_in_svn): New function
* General comment - added calls to svn_path_is_valid_in_svn()
in functions listed below
* subversion/libsvn_wc/adm_ops.c
modified svn_wc_add()
* subversion/libsvn_ra_local/ra_plugin.c
modified svn_ra_local__do_check_path()
* subversion/libsvn_client/copy.c
modified wc_to_wc_copy()
* subversion/libsvn_client/add.c
modified add_dir_recursive(), add_file()
* subversion/libsvn_client/commit.c
modified import_file(), import_dir()
* subversion/tests/clients/cmdline/commit_tests.py
pulled tab test code from commit_uri_unsafe() into tab_test()
Index: subversion/include/svn_path.h
===================================================================
--- subversion/include/svn_path.h (revision 12179)
+++ subversion/include/svn_path.h (working copy)
@@ -338,6 +338,22 @@
svn_boolean_t svn_path_is_backpath_present (const char *path);
+/**
+ * @since New in 1.2.
+ *
+ * Test to see if a path is allowed in a Subversion repository.
+ * First check whether all characters are valid in UTF8
+ * Next check whether all characters are non-control characters
+ * Currently, all other paths are allowed.
+ *
+ * This is OS-independent - it is used to decide what paths may be
+ * added to a repository. Adding occurs either using a file on disk
+ * (svn import/add) or one not on disk (svn mv/copy URL1 URL2).
+ */
+svn_error_t *svn_path_is_valid_in_svn (const char *path,
+ apr_size_t len,
+ apr_pool_t *pool);
+
/** Test if @a path2 is a child of @a path1.
* If not, return @c NULL.
* If so, return a copy of the remainder path, allocated in @a pool.
Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/adm_ops.c (revision 12179)
+++ subversion/libsvn_wc/adm_ops.c (working copy)
@@ -879,6 +879,10 @@
svn_node_kind_t kind;
apr_uint32_t modify_flags = 0;
svn_wc_adm_access_t *adm_access;
+
+ SVN_ERR (svn_path_is_valid_in_svn (path,
+ strlen(path),
+ pool));
/* Make sure something's there. */
SVN_ERR (svn_io_check_path (path, &kind, pool));
Index: subversion/libsvn_subr/path.c
===================================================================
--- subversion/libsvn_subr/path.c (revision 12179)
+++ subversion/libsvn_subr/path.c (working copy)
@@ -29,6 +29,7 @@
#include "svn_private_config.h" /* for SVN_PATH_LOCAL_SEPARATOR */
#include "svn_utf.h"
#include "svn_io.h" /* for svn_io_stat() */
+#include "svn_ctype.h" /* for svn_ctype_() */
/* The canonical empty path. Can this be changed? Well, change the empty
@@ -1248,3 +1249,25 @@
else
return svn_utf_cstring_to_utf8 (path_utf8, path_apr, pool);
}
+
+svn_error_t*
+svn_path_is_valid_in_svn (const char *path,
+ apr_size_t len,
+ apr_pool_t *pool)
+{
+ int i;
+ for (i = 0; i < len; i++)
+ {
+ if (!svn_ctype_isutf8(path[i]))
+ return svn_error_createf(SVN_ERR_FS_PATH_SYNTAX, NULL,
+ "Invalid UTF8 character in '%s'",
+ svn_path_local_style (path, pool));
+
+ if (svn_ctype_iscntrl(path[i]))
+ return svn_error_createf(SVN_ERR_FS_PATH_SYNTAX, NULL,
+ "Invalid control character in '%s'",
+ svn_path_local_style (path, pool));
+ }
+
+ return SVN_NO_ERROR;
+}
Index: subversion/libsvn_ra_local/ra_plugin.c
===================================================================
--- subversion/libsvn_ra_local/ra_plugin.c (revision 12179)
+++ subversion/libsvn_ra_local/ra_plugin.c (working copy)
@@ -687,7 +687,15 @@
svn_ra_local__session_baton_t *sbaton = session_baton;
svn_fs_root_t *root;
const char *abs_path = sbaton->fs_path;
+ svn_error_t *err;
+ if (path && (err = svn_path_is_valid_in_svn (path,
+ strlen(path),
+ pool)))
+ return svn_error_createf (SVN_ERR_FS_PATH_SYNTAX, err,
+ _("Invalid pathname '%s'"),
+ svn_path_local_style (path, pool));
+
/* ### Not sure if this counts as a workaround or not. The
session baton uses the empty string to mean root, and not
sure that should change. However, it would be better to use
Index: subversion/libsvn_client/copy.c
===================================================================
--- subversion/libsvn_client/copy.c (revision 12179)
+++ subversion/libsvn_client/copy.c (working copy)
@@ -74,6 +74,10 @@
svn_wc_adm_access_t *adm_access, *src_access;
svn_error_t *err;
+ /* Verify path has no invalid characters */
+ SVN_ERR (svn_path_is_valid_in_svn (src_path, strlen(src_path),
+ pool));
+
/* Verify that SRC_PATH exists. */
SVN_ERR (svn_io_check_path (src_path, &src_kind, pool));
if (src_kind == svn_node_none)
Index: subversion/libsvn_client/add.c
===================================================================
--- subversion/libsvn_client/add.c (revision 12179)
+++ subversion/libsvn_client/add.c (working copy)
@@ -209,6 +209,11 @@
svn_node_kind_t kind;
svn_boolean_t is_special;
+ /* Check for invalid pathname characters */
+ SVN_ERR (svn_path_is_valid_in_svn (path,
+ strlen(path),
+ pool));
+
/* add the file */
SVN_ERR (svn_wc_add (path, adm_access, NULL, SVN_INVALID_REVNUM,
ctx->cancel_func, ctx->cancel_baton,
@@ -276,6 +281,11 @@
svn_wc_adm_access_t *dir_access;
apr_array_header_t *ignores;
+ /* Check for invalid pathname characters */
+ SVN_ERR (svn_path_is_valid_in_svn (dirname,
+ strlen(dirname),
+ pool));
+
/* Check cancellation; note that this catches recursive calls too. */
if (ctx->cancel_func)
SVN_ERR (ctx->cancel_func (ctx->cancel_baton));
Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c (revision 12179)
+++ subversion/libsvn_client/commit.c (working copy)
@@ -176,6 +176,9 @@
svn_node_kind_t kind;
svn_boolean_t is_special;
+ /* validate pathname characters */
+ SVN_ERR (svn_path_is_valid_in_svn (path, strlen (path), pool));
+
SVN_ERR (svn_io_check_special_path (path, &kind, &is_special, pool));
if (kind == svn_node_unknown)
@@ -276,6 +279,9 @@
apr_hash_index_t *hi;
apr_array_header_t *ignores;
+ /* validate pathname characters */
+ SVN_ERR (svn_path_is_valid_in_svn (path, strlen (path), pool));
+
SVN_ERR (svn_wc_get_default_ignores (&ignores, ctx->config, pool));
SVN_ERR (svn_io_get_dirents (&dirents, path, pool));
@@ -427,6 +433,7 @@
apr_array_header_t *batons = NULL;
const char *edit_path = "";
+ SVN_ERR (svn_io_check_path (path, &kind, pool));
/* Get a root dir baton. We pass an invalid revnum to open_root
to mean "base this on the youngest revision". Should we have an
SVN_YOUNGEST_REVNUM defined for these purposes? */
Index: subversion/tests/clients/cmdline/commit_tests.py
===================================================================
--- subversion/tests/clients/cmdline/commit_tests.py (revision 12179)
+++ subversion/tests/clients/cmdline/commit_tests.py (working copy)
@@ -857,6 +857,43 @@
#----------------------------------------------------------------------
+def tab_test(sbox):
+ "tab testing"
+
+ # ripped out of commit_uri_unsafe - currently must be used with an XFail
+ # add test for directory
+
+ sbox.build()
+ wc_dir = sbox.wc_dir
+
+ if svntest.main.windows or sys.platform == 'cygwin':
+ tab_name = 'tab-path'
+ else:
+ tab_name = "tab\tpath"
+
+ tab_path = os.path.join(wc_dir, 'A', tab_name)
+ svntest.main.file_append(tab_path, "This path has a tab in it.")
+ svntest.main.run_svn(None, 'add', '--non-recursive', tab_path)
+
+ expected_output = svntest.wc.State(wc_dir, {
+ 'A/' + tab_name : Item(verb='Adding'),
+ })
+
+ # Items in the status list are all at rev 1
+ expected_status.tweak(wc_rev=1)
+
+ # Items in our add list will be at rev 2
+ for item in expected_output.desc.keys():
+ expected_status.add({ item : Item(wc_rev=2, repos_rev=2, status=' ') })
+
+ svntest.actions.run_and_verify_commit (wc_dir,
+ expected_output,
+ expected_status,
+ None, None, None, None, None,
+ wc_dir)
+
+#----------------------------------------------------------------------
+
def hook_test(sbox):
"hook testing"
@@ -1069,17 +1106,17 @@
if svntest.main.windows or sys.platform == 'cygwin':
angle_name = '_angle_'
nasty_name = '#![]{}()__%'
- tab_name = 'tab-path'
+ #tab_name = 'tab-path'
else:
angle_name = '<angle>'
nasty_name = '#![]{}()<>%'
- tab_name = "tab\tpath"
+ #tab_name = "tab\tpath"
# Make some convenient paths.
hash_dir = os.path.join(wc_dir, '#hash#')
nasty_dir = os.path.join(wc_dir, nasty_name)
space_path = os.path.join(wc_dir, 'A', 'D', 'space path')
- tab_path = os.path.join(wc_dir, 'A', 'D', 'G', tab_name)
+ #tab_path = os.path.join(wc_dir, 'A', 'D', 'G', tab_name)
bang_path = os.path.join(wc_dir, 'A', 'D', 'H', 'bang!')
bracket_path = os.path.join(wc_dir, 'A', 'D', 'H', 'bra[ket')
brace_path = os.path.join(wc_dir, 'A', 'D', 'H', 'bra{e')
@@ -1091,7 +1128,7 @@
os.mkdir(hash_dir)
os.mkdir(nasty_dir)
svntest.main.file_append(space_path, "This path has a space in it.")
- svntest.main.file_append(tab_path, "This path has a tab in it.")
+ #svntest.main.file_append(tab_path, "This path has a tab in it.")
svntest.main.file_append(bang_path, "This path has a bang in it.")
svntest.main.file_append(bracket_path, "This path has a bracket in it.")
svntest.main.file_append(brace_path, "This path has a brace in it.")
@@ -1103,7 +1140,7 @@
add_list = [hash_dir,
nasty_dir, # not xml-safe
space_path,
- tab_path,
+ #tab_path,
bang_path,
bracket_path,
brace_path,
@@ -1119,7 +1156,7 @@
'#hash#' : Item(verb='Adding'),
nasty_name : Item(verb='Adding'),
'A/D/space path' : Item(verb='Adding'),
- 'A/D/G/' + tab_name : Item(verb='Adding'),
+ #'A/D/G/' + tab_name : Item(verb='Adding'),
'A/D/H/bang!' : Item(verb='Adding'),
'A/D/H/bra[ket' : Item(verb='Adding'),
'A/D/H/bra{e' : Item(verb='Adding'),
@@ -1891,6 +1928,7 @@
hudson_part_1_variation_2,
hudson_part_2,
hudson_part_2_1,
+ XFail(tab_test),
XFail(hook_test),
merge_mixed_revisions,
commit_uri_unsafe,
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Dec 6 12:08:29 2004