subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From stef...@apache.org
Subject svn commit: r1662585 - in /subversion/trunk/subversion: libsvn_fs/fs-loader.c tests/libsvn_fs/fs-test.c
Date Thu, 26 Feb 2015 22:02:46 GMT
Author: stefan2
Date: Thu Feb 26 22:02:46 2015
New Revision: 1662585

URL: http://svn.apache.org/r1662585
Log:
Globally prevent paths with newlines in them from being created at the FS
layer.  Various higher-level functionality can't handle them ATM and this
is the most central / convenient place to prevent those paths.

Our svn: property formats that contain paths (svn:mergeinfo, svn:externals
and svn:ignore) don't support newlines in those paths.  For the latter two
properties, this is simply a feature limitation.  Mergeinfo, however, gets
added by SVN to paths at its own discretion and "sticks" after renames etc.
So, that would be bound to break eventually.

The fix for issue #4340 did only address the storage lager itself (FSFS)
but not the impact of such paths on other parts of the system.  Keep the
FSFS-private check in place.  Once the remainder of SVN handles these paths
correctly, the FS-global ban may be lifted again.

* subversion/libsvn_fs/fs-loader.c
  (svn_fs__path_valid): Reject the introduction of paths with \n in them.

* subversion/tests/libsvn_fs/fs-test.c
  (filename_trailing_newline): Even if the backend could handle \n paths,
                               ATM there is no way to create them.

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

Modified: subversion/trunk/subversion/libsvn_fs/fs-loader.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/fs-loader.c?rev=1662585&r1=1662584&r2=1662585&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs/fs-loader.c (original)
+++ subversion/trunk/subversion/libsvn_fs/fs-loader.c Thu Feb 26 22:02:46 2015
@@ -441,6 +441,8 @@ default_warning_func(void *baton, svn_er
 svn_error_t *
 svn_fs__path_valid(const char *path, apr_pool_t *pool)
 {
+  char *c;
+
   /* UTF-8 encoded string without NULs. */
   if (! svn_utf__cstring_is_valid(path))
     {
@@ -457,6 +459,18 @@ svn_fs__path_valid(const char *path, apr
                                path);
     }
 
+  /* Raise an error if PATH contains a newline because svn:mergeinfo and
+     friends can't handle them.  Issue #4340 describes a similar problem
+     in the FSFS code itself.
+   */
+  c = strchr(path, '\n');
+  if (c)
+    {
+      return svn_error_createf(SVN_ERR_FS_PATH_SYNTAX, NULL,
+               _("Invalid control character '0x%02x' in path '%s'"),
+               (unsigned char)*c, svn_path_illegal_path_escape(path, pool));
+    }
+
   /* That's good enough. */
   return SVN_NO_ERROR;
 }

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=1662585&r1=1662584&r2=1662585&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_fs/fs-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_fs/fs-test.c Thu Feb 26 22:02:46 2015
@@ -5100,15 +5100,11 @@ 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 legacy_backend;
-  static const char contents[] = "foo\003bar";
-
-  /* The FS API wants \n to be permitted, but FSFS never implemented that,
-   * so for FSFS we expect errors rather than successes in some of the commits.
-   * Use a blacklist approach so that new FSes default to implementing the API
-   * as originally defined. */
-  legacy_backend = (!strcmp(opts->fs_type, SVN_FS_TYPE_FSFS));
 
+  /* The FS API wants \n to be permitted, but FSFS never implemented that.
+   * Moreover, formats like svn:mergeinfo and svn:externals don't support
+   * it either.  So, we can't have newlines in file names in any FS.
+   */
   SVN_ERR(svn_test__create_fs(&fs, "test-repo-filename-trailing-newline",
                               opts, pool));
 
@@ -5120,64 +5116,20 @@ 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 on FSFS. */
+  /* Attempt to copy /foo to "/bar\n". This should fail. */
   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);
-  if (!legacy_backend)
-    SVN_TEST_ASSERT(err == SVN_NO_ERROR);
-  else
-    SVN_TEST_ASSERT_ERROR(err, SVN_ERR_FS_PATH_SYNTAX);
+  SVN_TEST_ASSERT_ERROR(err, SVN_ERR_FS_PATH_SYNTAX);
 
-  /* Attempt to create a file /foo/baz\n. This should fail on FSFS. */
+  /* Attempt to create a file /foo/baz\n. This should fail. */
   err = svn_fs_make_file(txn_root, "/foo/baz\n", subpool);
-  if (!legacy_backend)
-    SVN_TEST_ASSERT(err == SVN_NO_ERROR);
-  else
-    SVN_TEST_ASSERT_ERROR(err, SVN_ERR_FS_PATH_SYNTAX);
-
-
-  /* Create another file, with contents. */
-  if (!legacy_backend)
-    {
-      SVN_ERR(svn_fs_make_file(txn_root, "/bar\n/baz\n", subpool));
-      SVN_ERR(svn_test__set_file_contents(txn_root, "bar\n/baz\n",
-                                          contents, pool));
-    }
-
-  if (!legacy_backend)
-    {
-      svn_revnum_t after_rev;
-      static svn_test__tree_entry_t expected_entries[] = {
-        { "foo", NULL },
-        { "bar\n", NULL },
-        { "foo/baz\n", "" },
-        { "bar\n/baz\n", contents },
-        { NULL, NULL }
-      };
-      const char *expected_changed_paths[] = {
-        "/bar\n",
-        "/foo/baz\n",
-        "/bar\n/baz\n",
-        NULL
-      };
-      apr_hash_t *expected_changes = apr_hash_make(pool);
-      int i;
-
-      SVN_ERR(svn_fs_commit_txn(NULL, &after_rev, txn, subpool));
-      SVN_TEST_ASSERT(SVN_IS_VALID_REVNUM(after_rev));
-
-      /* Validate the DAG. */
-      SVN_ERR(svn_fs_revision_root(&root, fs, after_rev, pool));
-      SVN_ERR(svn_test__validate_tree(root, expected_entries, 4, pool));
-
-      /* Validate changed-paths, where the problem originally occurred. */
-      for (i = 0; expected_changed_paths[i]; i++)
-        svn_hash_sets(expected_changes, expected_changed_paths[i],
-                      "undefined value");
-      SVN_ERR(svn_test__validate_changes(root, expected_changes, pool));
-    }
+  SVN_TEST_ASSERT_ERROR(err, SVN_ERR_FS_PATH_SYNTAX);
+
+  /* Attempt to create a directory /foo/bang\n. This should fail. */
+  err = svn_fs_make_dir(txn_root, "/foo/bang\n", subpool);
+  SVN_TEST_ASSERT_ERROR(err, SVN_ERR_FS_PATH_SYNTAX);
 
   return SVN_NO_ERROR;
 }



Mime
View raw message