You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2010/03/22 17:09:44 UTC
svn commit: r926151 - in /subversion/trunk/subversion:
libsvn_fs_base/bdb/changes-table.c tests/libsvn_fs_base/changes-test.c
Author: cmpilato
Date: Mon Mar 22 16:09:44 2010
New Revision: 926151
URL: http://svn.apache.org/viewvc?rev=926151&view=rev
Log:
Plug another invalid-changes-ordering hole in the BDB backend.
* subversion/libsvn_fs_base/bdb/changes-table.c
(fold_change): Protect against another form of invalid sequence,
where an 'add' follows other non-delete/reset changes on the same
node.
* subversion/tests/libsvn_fs_base/changes-test.c
(changes_bad_sequences): New test.
(test_funcs): Add reference to new test.
Suggested by: glasser
Modified:
subversion/trunk/subversion/libsvn_fs_base/bdb/changes-table.c
subversion/trunk/subversion/tests/libsvn_fs_base/changes-test.c
Modified: subversion/trunk/subversion/libsvn_fs_base/bdb/changes-table.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_base/bdb/changes-table.c?rev=926151&r1=926150&r2=926151&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_base/bdb/changes-table.c (original)
+++ subversion/trunk/subversion/libsvn_fs_base/bdb/changes-table.c Mon Mar 22 16:09:44 2010
@@ -168,6 +168,15 @@ fold_change(apr_hash_t *changes,
(SVN_ERR_FS_CORRUPT, NULL,
_("Invalid change ordering: non-add change on deleted path"));
+ /* Sanity check: an add can't follow anything except
+ a delete or reset. */
+ if ((change->kind == svn_fs_path_change_add)
+ && (old_change->change_kind != svn_fs_path_change_delete)
+ && (old_change->change_kind != svn_fs_path_change_reset))
+ return svn_error_create
+ (SVN_ERR_FS_CORRUPT, NULL,
+ _("Invalid change ordering: add change on preexisting path"));
+
/* Now, merge that change in. */
switch (change->kind)
{
Modified: subversion/trunk/subversion/tests/libsvn_fs_base/changes-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs_base/changes-test.c?rev=926151&r1=926150&r2=926151&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_fs_base/changes-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_fs_base/changes-test.c Mon Mar 22 16:09:44 2010
@@ -698,6 +698,204 @@ changes_fetch_ordering(const svn_test_op
}
+static svn_error_t *
+changes_bad_sequences(const svn_test_opts_t *opts,
+ apr_pool_t *pool)
+{
+ svn_fs_t *fs;
+ apr_pool_t *subpool = svn_pool_create(pool);
+ svn_error_t *err;
+
+ /* Create a new fs and repos */
+ SVN_ERR(svn_test__create_bdb_fs
+ (&fs, "test-repo-changes-bad-sequences", opts,
+ pool));
+
+ /* Test changes bogus because a path's node-rev-ID changed
+ unexpectedly. */
+ svn_pool_clear(subpool);
+ {
+ static const char *bogus_changes[][6]
+ /* KEY PATH NODEREVID KIND TEXT PROP */
+ = { { "x", "/foo", "1.0.0", "add", 0 , 0 },
+ { "x", "/foo", "1.0.0", "modify", 0 , "1" },
+ { "x", "/foo", "2.0.0", "modify", "1", "1" } };
+ int num_changes = sizeof(bogus_changes) / sizeof(const char *) / 6;
+ struct changes_args args;
+ int i;
+
+ for (i = 0; i < num_changes; i++)
+ {
+ change_t change;
+
+ /* Set up the current change item. */
+ change.path = bogus_changes[i][1];
+ change.noderev_id = svn_fs_parse_id(bogus_changes[i][2],
+ strlen(bogus_changes[i][2]),
+ subpool);
+ change.kind = string_to_kind(bogus_changes[i][3]);
+ change.text_mod = bogus_changes[i][4] ? 1 : 0;
+ change.prop_mod = bogus_changes[i][5] ? 1 : 0;
+
+ /* Set up transaction baton. */
+ args.fs = fs;
+ args.key = "x";
+ args.change = &change;
+
+ /* Write new changes to the changes table. */
+ SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_changes_add, &args,
+ TRUE, subpool));
+ }
+
+ /* Now read 'em back, looking for an error. */
+ args.fs = fs;
+ args.key = "x";
+ err = svn_fs_base__retry_txn(args.fs, txn_body_changes_fetch, &args,
+ TRUE, subpool);
+ if (!err)
+ {
+ return svn_error_create(SVN_ERR_TEST_FAILED, 0,
+ "Expected SVN_ERR_FS_CORRUPT, got no error.");
+ }
+ else if (err->apr_err != SVN_ERR_FS_CORRUPT)
+ {
+ return svn_error_create(SVN_ERR_TEST_FAILED, err,
+ "Expected SVN_ERR_FS_CORRUPT, got a different error.");
+ }
+ else
+ {
+ svn_error_clear(err);
+ }
+
+ /* Post-test cleanup. */
+ SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_changes_delete, &args,
+ TRUE, subpool));
+ }
+
+ /* Test changes bogus because there's a change other than an
+ add-type changes on a deleted path. */
+ svn_pool_clear(subpool);
+ {
+ static const char *bogus_changes[][6]
+ /* KEY PATH NODEREVID KIND TEXT PROP */
+ = { { "x", "/foo", "1.0.0", "delete", 0 , 0 },
+ { "x", "/foo", "1.0.0", "modify", "1", 0 } };
+ int num_changes = sizeof(bogus_changes) / sizeof(const char *) / 6;
+ struct changes_args args;
+ int i;
+
+ for (i = 0; i < num_changes; i++)
+ {
+ change_t change;
+
+ /* Set up the current change item. */
+ change.path = bogus_changes[i][1];
+ change.noderev_id = svn_fs_parse_id(bogus_changes[i][2],
+ strlen(bogus_changes[i][2]),
+ subpool);
+ change.kind = string_to_kind(bogus_changes[i][3]);
+ change.text_mod = bogus_changes[i][4] ? 1 : 0;
+ change.prop_mod = bogus_changes[i][5] ? 1 : 0;
+
+ /* Set up transaction baton. */
+ args.fs = fs;
+ args.key = "x";
+ args.change = &change;
+
+ /* Write new changes to the changes table. */
+ SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_changes_add, &args,
+ TRUE, subpool));
+ }
+
+ /* Now read 'em back, looking for an error. */
+ args.fs = fs;
+ args.key = "x";
+ err = svn_fs_base__retry_txn(args.fs, txn_body_changes_fetch, &args,
+ TRUE, subpool);
+ if (!err)
+ {
+ return svn_error_create(SVN_ERR_TEST_FAILED, 0,
+ "Expected SVN_ERR_FS_CORRUPT, got no error.");
+ }
+ else if (err->apr_err != SVN_ERR_FS_CORRUPT)
+ {
+ return svn_error_create(SVN_ERR_TEST_FAILED, err,
+ "Expected SVN_ERR_FS_CORRUPT, got a different error.");
+ }
+ else
+ {
+ svn_error_clear(err);
+ }
+
+ /* Post-test cleanup. */
+ SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_changes_delete, &args,
+ TRUE, subpool));
+ }
+
+ /* Test changes bogus because there's an add on a path that's got
+ previous non-delete changes on it. */
+ svn_pool_clear(subpool);
+ {
+ static const char *bogus_changes[][6]
+ /* KEY PATH NODEREVID KIND TEXT PROP */
+ = { { "x", "/foo", "1.0.0", "modify", "1", 0 },
+ { "x", "/foo", "1.0.0", "add", "1", 0 } };
+ int num_changes = sizeof(bogus_changes) / sizeof(const char *) / 6;
+ struct changes_args args;
+ int i;
+
+ for (i = 0; i < num_changes; i++)
+ {
+ change_t change;
+
+ /* Set up the current change item. */
+ change.path = bogus_changes[i][1];
+ change.noderev_id = svn_fs_parse_id(bogus_changes[i][2],
+ strlen(bogus_changes[i][2]),
+ subpool);
+ change.kind = string_to_kind(bogus_changes[i][3]);
+ change.text_mod = bogus_changes[i][4] ? 1 : 0;
+ change.prop_mod = bogus_changes[i][5] ? 1 : 0;
+
+ /* Set up transaction baton. */
+ args.fs = fs;
+ args.key = "x";
+ args.change = &change;
+
+ /* Write new changes to the changes table. */
+ SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_changes_add, &args,
+ TRUE, subpool));
+ }
+
+ /* Now read 'em back, looking for an error. */
+ args.fs = fs;
+ args.key = "x";
+ err = svn_fs_base__retry_txn(args.fs, txn_body_changes_fetch, &args,
+ TRUE, subpool);
+ if (!err)
+ {
+ return svn_error_create(SVN_ERR_TEST_FAILED, 0,
+ "Expected SVN_ERR_FS_CORRUPT, got no error.");
+ }
+ else if (err->apr_err != SVN_ERR_FS_CORRUPT)
+ {
+ return svn_error_create(SVN_ERR_TEST_FAILED, err,
+ "Expected SVN_ERR_FS_CORRUPT, got a different error.");
+ }
+ else
+ {
+ svn_error_clear(err);
+ }
+
+ /* Post-test cleanup. */
+ SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_changes_delete, &args,
+ TRUE, subpool));
+ }
+
+ return SVN_NO_ERROR;
+}
+
+
/* The test table. */
@@ -714,5 +912,7 @@ struct svn_test_descriptor_t test_funcs[
"fetch compressed changes from the changes table"),
SVN_TEST_OPTS_PASS(changes_fetch_ordering,
"verify ordered-ness of fetched compressed changes"),
+ SVN_TEST_OPTS_PASS(changes_bad_sequences,
+ "verify that bad change sequences raise errors"),
SVN_TEST_NULL
};