Hello,
This is still incomplete, but it should have covered all the review
points.
Thanks
Sameer
Resolve issue #1954: Error on add or import of a path that is
invalid in Subversion.
####################################################
### ###
### Note: This is a draft patch for review only, ###
### please do not commit it. ###
### ###
####################################################
* subversion/include/svn_path.h
(svn_path_is_valid, svn_path_error_if_invalid): New functions.
* subversion/include/svn_utf.h
(svn_utf_check_cstring_utf8): New function.
* subversion/libsvn_subr/path.c
(svn_path_is_valid, svn_path_error_if_invalid,
validate_svn_path): New functions.
* subversion/libsvn_wc/adm_ops.c
(svn_wc_add): Call svn_path_error_if_invalid().
* subversion/libsvn_ra_local/ra_plugin.c
(svn_ra_local__do_check_path): Same.
* subversion/libsvn_client/copy.c
(wc_to_wc_copy): Same.
* subversion/libsvn_client/add.c
(add_dir_recursive, add_file): Same.
* subversion/libsvn_client/commit.c
(import_file, import_dir): Same.
* subversion/tests/clients/cmdline/commit_tests.py
(tab_test): New test.
(test_list): Run it XFail.
(commit_uri_unsafe): Removed tab test
Index: subversion/include/svn_path.h
===================================================================
--- subversion/include/svn_path.h (revision 12257)
+++ subversion/include/svn_path.h (working copy)
@@ -356,6 +356,39 @@
const char *path2,
apr_pool_t *pool);
+/**
+ * @since New in 1.2.
+ *
+ * Check whether @a path is a valid Subversion path
+ * @a pool is used for temporary allocation
+ * @a check_utf8 turns on additional checks
+ *
+ * A valid Subversion pathname is a UTF-8 string without control
+ * characters, except for SPACE (0x20). "Valid" means Subversion
+ * can store the pathname in a repository. There may be other
+ * OS-specific limitations on what paths can be represented in
+ * a working copy.
+ */
+svn_boolean_t svn_path_is_valid (const char *path,
+ apr_pool_t *pool,
+ svn_boolean_t check_utf8);
+
+/**
+ * @since New in 1.2.
+ *
+ * Check whether @a path is a valid Subversion path and generate
+ * error if invalid. See docstring of svn_path_is_valid() for
+ * definition of valid pathname
+ * @a pool is used for temporary allocation
+ * @a check_utf8 turns on additional checks
+ *
+ * returns SVN_NO_ERROR if valid
+ * returns SVN_ERR_FS_PATH_SYNTAX if invalid
+ */
+svn_error_t *svn_path_error_if_invalid (const char *path,
+ apr_pool_t *pool,
+ svn_boolean_t check_utf8);
+
/** URI/URL stuff
*
Index: subversion/include/svn_utf.h
===================================================================
--- subversion/include/svn_utf.h (revision 12257)
+++ subversion/include/svn_utf.h (working copy)
@@ -171,6 +171,16 @@
const svn_string_t *src,
apr_pool_t *pool);
+/** @since New in 1.2
+ *
+ * Public interface around check_cstring_utf8, which checks
+ * whether the NULL-terminated sequence @a DATA is valid UTF-8
+ *
+ * returns SVN_NO_ERROR if valid
+ * returns APR_EINVAL if invalid
+ */
+svn_error_t *svn_utf_check_cstring_utf8 (const char *data, apr_pool_t *pool);
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/adm_ops.c (revision 12257)
+++ subversion/libsvn_wc/adm_ops.c (working copy)
@@ -879,6 +879,8 @@
svn_node_kind_t kind;
apr_uint32_t modify_flags = 0;
svn_wc_adm_access_t *adm_access;
+
+ SVN_ERR (svn_path_error_if_invalid (path, pool, FALSE));
/* Make sure something's there. */
SVN_ERR (svn_io_check_path (path, &kind, pool));
Index: subversion/libsvn_subr/utf.c
===================================================================
--- subversion/libsvn_subr/utf.c (revision 12257)
+++ subversion/libsvn_subr/utf.c (working copy)
@@ -476,6 +476,19 @@
return SVN_NO_ERROR;
}
+/* @since New in 1.2
+ *
+ * Public interface around check_cstring_utf8, which checks
+ * whether the NULL-terminated sequence @a DATA is valid UTF-8
+ *
+ * returns SVN_NO_ERROR if valid
+ * returns APR_EINVAL if invalid
+ */
+svn_error_t *
+svn_utf_check_cstring_utf8 (const char *data, apr_pool_t *pool)
+{
+ return (check_cstring_utf8 (data, pool));
+}
svn_error_t *
svn_utf_stringbuf_to_utf8 (svn_stringbuf_t **dest,
Index: subversion/libsvn_subr/path.c
===================================================================
--- subversion/libsvn_subr/path.c (revision 12257)
+++ 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"
/* The canonical empty path. Can this be changed? Well, change the empty
@@ -1248,3 +1249,69 @@
else
return svn_utf_cstring_to_utf8 (path_utf8, path_apr, pool);
}
+
+/* catchall function that performs various checks on path
+ * and fills out params based on switches.
+ * If check_utf8 is TRUE, validates path for utf8 correctness
+ * Otherwise, only checks for existence of control chars.
+ * If generate_error is TRUE and path is invalid, an error message
+ * is returned in err
+ * (created to avoid repeating code in two functions,
+ * one which returns an error message and one which doesn't)
+ */
+static svn_boolean_t
+validate_svn_path (const char *path, apr_pool_t *pool,
+ svn_boolean_t check_utf8,
+ svn_boolean_t generate_error,
+ svn_error_t *err)
+{
+ const char *c = path;
+
+ if (check_utf8)
+ {
+ svn_error_t *utf_err = svn_utf_check_cstring_utf8 (path, pool);
+ if (utf_err)
+ {
+ if (generate_error)
+ err = svn_error_createf (SVN_ERR_FS_PATH_SYNTAX, utf_err,
+ "Invalid UTF-8 in path: '%s'",
+ svn_path_local_style (path, pool));
+ else
+ svn_error_clear (utf_err);
+
+ return FALSE;
+ }
+ }
+
+ for (; *c; c++)
+ if (svn_ctype_iscntrl(*c))
+ {
+ if (generate_error)
+ err = svn_error_createf (SVN_ERR_FS_PATH_SYNTAX, NULL,
+ "Invalid character '%x' in path '%s'",
+ *c,
+ svn_path_local_style (path, pool));
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
+svn_boolean_t
+svn_path_is_valid (const char *path, apr_pool_t *pool,
+ svn_boolean_t check_utf8)
+{
+ svn_error_t *err = SVN_NO_ERROR;
+
+ return validate_svn_path (path, pool, check_utf8, FALSE, err);
+}
+
+svn_error_t*
+svn_path_error_if_invalid (const char *path, apr_pool_t *pool,
+ svn_boolean_t check_utf8)
+{
+ svn_error_t* err = SVN_NO_ERROR;
+
+ validate_svn_path (path, pool, check_utf8, TRUE, err);
+ return err;
+}
Index: subversion/libsvn_ra_local/ra_plugin.c
===================================================================
--- subversion/libsvn_ra_local/ra_plugin.c (revision 12257)
+++ subversion/libsvn_ra_local/ra_plugin.c (working copy)
@@ -688,6 +688,9 @@
svn_fs_root_t *root;
const char *abs_path = sbaton->fs_path;
+ if (path)
+ SVN_ERR (svn_path_error_if_invalid (path, pool, TRUE));
+
/* ### 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 12257)
+++ subversion/libsvn_client/copy.c (working copy)
@@ -74,6 +74,8 @@
svn_wc_adm_access_t *adm_access, *src_access;
svn_error_t *err;
+ SVN_ERR (svn_path_error_if_invalid (src_path, pool, FALSE));
+
/* 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 12257)
+++ subversion/libsvn_client/add.c (working copy)
@@ -209,6 +209,8 @@
svn_node_kind_t kind;
svn_boolean_t is_special;
+ SVN_ERR (svn_path_error_if_invalid (path, pool, FALSE));
+
/* add the file */
SVN_ERR (svn_wc_add (path, adm_access, NULL, SVN_INVALID_REVNUM,
ctx->cancel_func, ctx->cancel_baton,
@@ -276,6 +278,8 @@
svn_wc_adm_access_t *dir_access;
apr_array_header_t *ignores;
+ SVN_ERR (svn_path_error_if_invalid (dirname, pool, FALSE));
+
/* 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 12257)
+++ subversion/libsvn_client/commit.c (working copy)
@@ -176,6 +176,8 @@
svn_node_kind_t kind;
svn_boolean_t is_special;
+ SVN_ERR (svn_path_error_if_invalid (path, pool, FALSE));
+
SVN_ERR (svn_io_check_special_path (path, &kind, &is_special, pool));
if (kind == svn_node_unknown)
@@ -276,6 +278,8 @@
apr_hash_index_t *hi;
apr_array_header_t *ignores;
+ SVN_ERR (svn_path_error_if_invalid (path, pool, FALSE));
+
SVN_ERR (svn_wc_get_default_ignores (&ignores, ctx->config, pool));
SVN_ERR (svn_io_get_dirents (&dirents, path, pool));
Index: subversion/tests/clients/cmdline/commit_tests.py
===================================================================
--- subversion/tests/clients/cmdline/commit_tests.py (revision 12257)
+++ 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 Thu Dec 9 07:25:15 2004