subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From s...@apache.org
Subject svn commit: r1461701 - in /subversion/trunk/subversion: libsvn_fs/editor.c libsvn_fs/fs-loader.c libsvn_fs_fs/tree.c tests/libsvn_fs/fs-test.c
Date Wed, 27 Mar 2013 16:48:00 GMT
Author: stsp
Date: Wed Mar 27 16:48:00 2013
New Revision: 1461701

URL: http://svn.apache.org/r1461701
Log:
For issue #4330, relax the check added in r1461580 to cover newlines instead
of arbitrary control characters. Also move the check from the FS layer into
the FSFS layer.

This allows BDB filesystems to store filenames which contain newlines again.

It also allows FSFS filesystems to store filenames which contain control
characters other than newlines (I'm not entirely sure this is safe,
but we can solve the problem one character at a time).

* subversion/libsvn_fs/editor.c
  (can_create): Remove svn_path_check_valid() call.

* subversion/libsvn_fs/fs-loader.c
  (svn_fs__path_valid): Remove svn_path_check_valid() call. 

* subversion/libsvn_fs_fs/tree.c
  (escape_newline): New, based on illegal_path_escape from libsvn_subr/path.c.
  (check_newline): New, based on svn_path_check_valid().
  (fs_make_dir, fs_copy, fs_make_file): Prevent new paths which contain
   newlines from being added to the filesystem.

Modified:
    subversion/trunk/subversion/libsvn_fs/editor.c
    subversion/trunk/subversion/libsvn_fs/fs-loader.c
    subversion/trunk/subversion/libsvn_fs_fs/tree.c
    subversion/trunk/subversion/tests/libsvn_fs/fs-test.c

Modified: subversion/trunk/subversion/libsvn_fs/editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/editor.c?rev=1461701&r1=1461700&r2=1461701&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs/editor.c (original)
+++ subversion/trunk/subversion/libsvn_fs/editor.c Wed Mar 27 16:48:00 2013
@@ -288,9 +288,6 @@ can_create(svn_fs_root_t *txn_root,
   svn_node_kind_t kind;
   const char *cur_fspath;
 
-  /* Reject paths which contain control characters (see issue #4340). */
-  SVN_ERR(svn_path_check_valid(fspath, scratch_pool));
-
   SVN_ERR(svn_fs_check_path(&kind, txn_root, fspath, scratch_pool));
   if (kind == svn_node_none)
     return SVN_NO_ERROR;

Modified: subversion/trunk/subversion/libsvn_fs/fs-loader.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/fs-loader.c?rev=1461701&r1=1461700&r2=1461701&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs/fs-loader.c (original)
+++ subversion/trunk/subversion/libsvn_fs/fs-loader.c Wed Mar 27 16:48:00 2013
@@ -396,9 +396,6 @@ svn_fs__path_valid(const char *path, apr
                                path);
     }
 
-  /* No control characters (see issue #4340). */
-  SVN_ERR(svn_path_check_valid(path, pool));
-
   /* That's good enough. */
   return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1461701&r1=1461700&r2=1461701&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Wed Mar 27 16:48:00 2013
@@ -2176,6 +2176,70 @@ fs_dir_entries(apr_hash_t **table_p,
   return svn_fs_fs__dag_dir_entries(table_p, node, pool);
 }
 
+/* Return a copy of PATH, allocated from POOL, for which newlines
+   have been escaped using the form \NNN (where NNN is the
+   octal representation of the byte's ordinal value).  */
+static const char *
+escape_newline(const char *path, apr_pool_t *pool)
+{
+  svn_stringbuf_t *retstr;
+  apr_size_t i, copied = 0;
+  int c;
+
+  /* At least one control character:
+      strlen - 1 (control) + \ + N + N + N + null . */
+  retstr = svn_stringbuf_create_ensure(strlen(path) + 4, pool);
+  for (i = 0; path[i]; i++)
+    {
+      c = (unsigned char)path[i];
+      if (c != '\n')
+        continue;
+
+      /* First things first, copy all the good stuff that we haven't
+         yet copied into our output buffer. */
+      if (i - copied)
+        svn_stringbuf_appendbytes(retstr, path + copied,
+                                  i - copied);
+
+      /* Make sure buffer is big enough for '\' 'N' 'N' 'N' (and NUL) */
+      svn_stringbuf_ensure(retstr, retstr->len + 4);
+      /*### The backslash separator doesn't work too great with Windows,
+         but it's what we'll use for consistency with invalid utf8
+         formatting (until someone has a better idea) */
+      apr_snprintf(retstr->data + retstr->len, 5, "\\%03o", (unsigned char)c);
+      retstr->len += 4;
+
+      /* Finally, update our copy counter. */
+      copied = i + 1;
+    }
+
+  /* Anything left to copy? */
+  if (i - copied)
+    svn_stringbuf_appendbytes(retstr, path + copied, i - copied);
+
+  /* retstr is null-terminated either by apr_snprintf or the svn_stringbuf
+     functions. */
+
+  return retstr->data;
+}
+
+/* Raise an error if PATH contains a newline because FSFS cannot handle
+ * such paths. See issue #4340. */
+static svn_error_t *
+check_newline(const char *path, apr_pool_t *pool)
+{
+  const char *c;
+
+  for (c = path; *c; c++)
+    {
+      if (*c == '\n')
+        return svn_error_createf(SVN_ERR_FS_PATH_SYNTAX, NULL,
+           _("Invalid control character '0x%02x' in path '%s'"),
+           (unsigned char)*c, escape_newline(path, pool));
+    }
+
+  return SVN_NO_ERROR;
+}
 
 /* Create a new directory named PATH in ROOT.  The new directory has
    no entries, and no properties.  ROOT must be the root of a
@@ -2190,6 +2254,8 @@ fs_make_dir(svn_fs_root_t *root,
   dag_node_t *sub_dir;
   const char *txn_id = root->txn;
 
+  SVN_ERR(check_newline(path, pool));
+
   path = svn_fs__canonicalize_abspath(path, pool);
   SVN_ERR(open_path(&parent_path, root, path, open_path_last_optional,
                     txn_id, pool));
@@ -2442,6 +2508,8 @@ fs_copy(svn_fs_root_t *from_root,
         const char *to_path,
         apr_pool_t *pool)
 {
+  SVN_ERR(check_newline(to_path, pool));
+
   return svn_error_trace(copy_helper(from_root,
                                      svn_fs__canonicalize_abspath(from_path,
                                                                   pool),
@@ -2540,6 +2608,8 @@ fs_make_file(svn_fs_root_t *root,
   dag_node_t *child;
   const char *txn_id = root->txn;
 
+  SVN_ERR(check_newline(path, pool));
+
   path = svn_fs__canonicalize_abspath(path, pool);
   SVN_ERR(open_path(&parent_path, root, path, open_path_last_optional,
                    txn_id, pool));

Modified: subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs/fs-test.c?rev=1461701&r1=1461700&r2=1461701&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_fs/fs-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_fs/fs-test.c Wed Mar 27 16:48:00 2013
@@ -4945,6 +4945,12 @@ filename_trailing_newline(const svn_test
   svn_fs_root_t *txn_root, *root;
   svn_revnum_t youngest_rev = 0;
   svn_error_t *err;
+  svn_boolean_t allow_newlines;
+  
+  /* Some filesystem implementations can handle newlines in filenames
+   * and can be white-listed here.
+   * Currently, only BDB supports \n in filenames. */
+  allow_newlines = (strcmp(opts->fs_type, "bdb") == 0);
 
   SVN_ERR(svn_test__create_fs(&fs, "test-repo-filename-trailing-newline",
                               opts, pool));
@@ -4957,16 +4963,22 @@ filename_trailing_newline(const svn_test
   SVN_TEST_ASSERT(SVN_IS_VALID_REVNUM(youngest_rev));
   svn_pool_clear(subpool);
 
-  /* Attempt to copy /foo to "/bar\n". This should fail. */
+  /* Attempt to copy /foo to "/bar\n". This should fail on FSFS. */
   SVN_ERR(svn_fs_begin_txn(&txn, fs, youngest_rev, subpool));
   SVN_ERR(svn_fs_txn_root(&txn_root, txn, subpool));
   SVN_ERR(svn_fs_revision_root(&root, fs, youngest_rev, subpool));
   err = svn_fs_copy(root, "/foo", txn_root, "/bar\n", subpool);
-  SVN_TEST_ASSERT_ERROR(err, SVN_ERR_FS_PATH_SYNTAX);
+  if (allow_newlines)
+    SVN_TEST_ASSERT(err == SVN_NO_ERROR);
+  else
+    SVN_TEST_ASSERT_ERROR(err, SVN_ERR_FS_PATH_SYNTAX);
 
-  /* Attempt to create a file /foo/baz\n. This should fail. */
+  /* Attempt to create a file /foo/baz\n. This should fail on FSFS. */
   err = svn_fs_make_file(txn_root, "/foo/baz\n", subpool);
-  SVN_TEST_ASSERT_ERROR(err, SVN_ERR_FS_PATH_SYNTAX);
+  if (allow_newlines)
+    SVN_TEST_ASSERT(err == SVN_NO_ERROR);
+  else
+    SVN_TEST_ASSERT_ERROR(err, SVN_ERR_FS_PATH_SYNTAX);
 
   return SVN_NO_ERROR;
 }
@@ -5053,6 +5065,6 @@ struct svn_test_descriptor_t test_funcs[
     SVN_TEST_OPTS_PASS(delete_fs,
                        "test svn_fs_delete_fs"),
     SVN_TEST_OPTS_PASS(filename_trailing_newline,
-                       "filenames with trailing \\n should be rejected"),
+                       "filenames with trailing \\n might be rejected"),
     SVN_TEST_NULL
   };



Mime
View raw message