You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by hw...@apache.org on 2010/03/31 19:44:10 UTC
svn commit: r929634 - in /subversion/branches/1.6.x: ./ CHANGES STATUS
subversion/libsvn_fs_base/bdb/changes-table.c subversion/libsvn_fs_fs/fs_fs.c
subversion/tests/libsvn_fs_base/changes-test.c
Author: hwright
Date: Wed Mar 31 17:44:10 2010
New Revision: 929634
URL: http://svn.apache.org/viewvc?rev=929634&view=rev
Log:
Reintegrate the 1.6.x-r926151 branch:
* r926151, r926167
Add an additional check for bogus `changes' table item sequencing.
Apparently the custom Google backend managed to trigger some badness
along these lines.
Notes:
r926151 adds a test and fix for BDB; r926167 ports the change to FSFS.
Justification:
Safety first.
Branch:
r876470 has not been backported to 1.6.x, so the test added in r926151
must use the old-style SVN_TEST_PASS macro rather than the
SVN_TEST_OPTS_PASS macro.
^/subversion/branches/1.6.x-r926151
Votes:
+1: cmpilato, pburba, gstein
Modified:
subversion/branches/1.6.x/ (props changed)
subversion/branches/1.6.x/CHANGES (props changed)
subversion/branches/1.6.x/STATUS
subversion/branches/1.6.x/subversion/libsvn_fs_base/bdb/changes-table.c
subversion/branches/1.6.x/subversion/libsvn_fs_fs/fs_fs.c
subversion/branches/1.6.x/subversion/tests/libsvn_fs_base/changes-test.c
Propchange: subversion/branches/1.6.x/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Mar 31 17:44:10 2010
@@ -35,6 +35,7 @@
/subversion/branches/1.6.x-r898963:899874-915098
/subversion/branches/1.6.x-r905326:905545-923537
/subversion/branches/1.6.x-r923389:927445-929628
+/subversion/branches/1.6.x-r926151:927922-929631
/subversion/branches/1.6.x-wc-ng-check-override:910213-923776
/subversion/branches/bdb-reverse-deltas:872050-872529
/subversion/branches/diff-callbacks3:870059-870761
@@ -66,4 +67,4 @@
/subversion/branches/tc_url_rev:874351-874483
/subversion/branches/tree-conflicts:868291-873154
/subversion/branches/tree-conflicts-notify:873926-874008
-/subversion/trunk:875965,875968,876004,876012,876017,876019,876022,876024,876041-876042,876048,876051,876055-876056,876059,876083,876091,876097,876101,876109,876123-876125,876129,876132,876138,876160,876167,876175,876180,876185,876205,876223-876225,876230,876233,876252,876256,876283,876287,876312,876326-876327,876330,876366,876372,876374,876376,876383,876386,876442,876456-876457,876462-876464,876467,876469,876480,876486,876495-876497,876516-876518,876524,876526,876583,876601,876614,876628,876633,876641,876659,876687,876689,876705,876715,876726,876760,876763,876794,876804,876815-876816,876821,876825,876837,876840-876841,876843,876849,876857-876858,876862,876873,876890,876897,876905,876908,876925,876931,876934,876948-876949,876953,876987,876993,877011,877016,877028-877029,877038,877119,877127,877146,877157,877191,877195,877203,877211,877230,877234,877237,877243,877249,877259,877261,877304,877319,877407,877437,877441-877442,877453,877459,877472,877544,877553,877565,877568,87757
3,877593,877595,877597,877601,877612,877665,877667,877681,877692,877696,877701,877720,877730,877784,877793,877797,877809,877815,877819,877821,877842,877848,877853,877867,877869,877873,877901,877909,877916,877931,877942,877953,877964,877968,877970,877981-877982,878005,878013,878015,878020,878046,878053,878062,878074,878080,878089,878091,878093,878095,878127,878129,878131,878142,878173-878176,878216,878240,878242,878255,878269,878272,878279,878296-878297,878303,878321,878335,878338,878341,878343,878353,878364,878367-878368,878385,878399,878423,878426,878462,878484,878491,878498,878532,878595,878646,878659,878673,878682-878683,878690-878691,878693,878723,878760-878761,878873,878875,878877,878879,878905,878915,878924-878925,878946,878949,878955,878960,878970,878981,879001,879033,879056,879074,879076,879081-879082,879093,879105,879126,879148,879170,879198-879199,879201,879271,879293,879357,879375-879376,879403,879631,879635-879636,879688,879709-879711,879747,879954,879961,880082,
880095,880105,880162,880226,880274-880275,880370,880450,880474,880525-880526,880552,881905,884842,886164,886197,888715,888979,889081,889840,891672,892050,892085,895514,895653,896522,896915,898963,899826,899828,900797,901752,902093,904301,904394,904594,905303,905326,906256,906305,908980-908981,917640,918211,922516,923389,923391,927323,927328
+/subversion/trunk:875965,875968,876004,876012,876017,876019,876022,876024,876041-876042,876048,876051,876055-876056,876059,876083,876091,876097,876101,876109,876123-876125,876129,876132,876138,876160,876167,876175,876180,876185,876205,876223-876225,876230,876233,876252,876256,876283,876287,876312,876326-876327,876330,876366,876372,876374,876376,876383,876386,876442,876456-876457,876462-876464,876467,876469,876480,876486,876495-876497,876516-876518,876524,876526,876583,876601,876614,876628,876633,876641,876659,876687,876689,876705,876715,876726,876760,876763,876794,876804,876815-876816,876821,876825,876837,876840-876841,876843,876849,876857-876858,876862,876873,876890,876897,876905,876908,876925,876931,876934,876948-876949,876953,876987,876993,877011,877016,877028-877029,877038,877119,877127,877146,877157,877191,877195,877203,877211,877230,877234,877237,877243,877249,877259,877261,877304,877319,877407,877437,877441-877442,877453,877459,877472,877544,877553,877565,877568,87757
3,877593,877595,877597,877601,877612,877665,877667,877681,877692,877696,877701,877720,877730,877784,877793,877797,877809,877815,877819,877821,877842,877848,877853,877867,877869,877873,877901,877909,877916,877931,877942,877953,877964,877968,877970,877981-877982,878005,878013,878015,878020,878046,878053,878062,878074,878080,878089,878091,878093,878095,878127,878129,878131,878142,878173-878176,878216,878240,878242,878255,878269,878272,878279,878296-878297,878303,878321,878335,878338,878341,878343,878353,878364,878367-878368,878385,878399,878423,878426,878462,878484,878491,878498,878532,878595,878646,878659,878673,878682-878683,878690-878691,878693,878723,878760-878761,878873,878875,878877,878879,878905,878915,878924-878925,878946,878949,878955,878960,878970,878981,879001,879033,879056,879074,879076,879081-879082,879093,879105,879126,879148,879170,879198-879199,879201,879271,879293,879357,879375-879376,879403,879631,879635-879636,879688,879709-879711,879747,879954,879961,880082,
880095,880105,880162,880226,880274-880275,880370,880450,880474,880525-880526,880552,881905,884842,886164,886197,888715,888979,889081,889840,891672,892050,892085,895514,895653,896522,896915,898963,899826,899828,900797,901752,902093,904301,904394,904594,905303,905326,906256,906305,908980-908981,917640,918211,922516,923389,923391,926151,926167,927323,927328
Propchange: subversion/branches/1.6.x/CHANGES
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Mar 31 17:44:10 2010
@@ -35,6 +35,7 @@
/subversion/branches/1.6.x-r898963/CHANGES:899874-915098
/subversion/branches/1.6.x-r905326/CHANGES:905545-923537
/subversion/branches/1.6.x-r923389/CHANGES:927445-929628
+/subversion/branches/1.6.x-r926151/CHANGES:927922-929631
/subversion/branches/1.6.x-wc-ng-check-override/CHANGES:910213-923776
/subversion/branches/bdb-reverse-deltas/CHANGES:872050-872529
/subversion/branches/diff-callbacks3/CHANGES:870059-870761
@@ -65,4 +66,4 @@
/subversion/branches/tc_url_rev/CHANGES:874351-874483
/subversion/branches/tree-conflicts/CHANGES:868291-873154
/subversion/branches/tree-conflicts-notify/CHANGES:873926-874008
-/subversion/trunk/CHANGES:837701-841355,875962-901365,901752,904301,904394,904594,905303,905326,908980-908981,917640,918211,922516,923389,923391,927323,927328
+/subversion/trunk/CHANGES:837701-841355,875962-901365,901752,904301,904394,904594,905303,905326,908980-908981,917640,918211,922516,923389,923391,926151,926167,927323,927328
Modified: subversion/branches/1.6.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/STATUS?rev=929634&r1=929633&r2=929634&view=diff
==============================================================================
--- subversion/branches/1.6.x/STATUS (original)
+++ subversion/branches/1.6.x/STATUS Wed Mar 31 17:44:10 2010
@@ -177,22 +177,6 @@ Veto-blocked changes:
Approved changes:
=================
- * r926151, r926167
- Add an additional check for bogus `changes' table item sequencing.
- Apparently the custom Google backend managed to trigger some badness
- along these lines.
- Notes:
- r926151 adds a test and fix for BDB; r926167 ports the change to FSFS.
- Justification:
- Safety first.
- Branch:
- r876470 has not been backported to 1.6.x, so the test added in r926151
- must use the old-style SVN_TEST_PASS macro rather than the
- SVN_TEST_OPTS_PASS macro.
- ^/subversion/branches/1.6.x-r926151
- Votes:
- +1: cmpilato, pburba, gstein
-
* r929382
Ignore errors reading .svn/wc.db from up the tree somewhere, in
trying to prevent creating a 1.6 wc inside a wc-ng.
@@ -205,4 +189,3 @@ Approved changes:
^/subversion/branches/1.6.x-wc-ng-error
Votes:
+1: peters, gstein, julianfoad
-
Modified: subversion/branches/1.6.x/subversion/libsvn_fs_base/bdb/changes-table.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/libsvn_fs_base/bdb/changes-table.c?rev=929634&r1=929633&r2=929634&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/libsvn_fs_base/bdb/changes-table.c (original)
+++ subversion/branches/1.6.x/subversion/libsvn_fs_base/bdb/changes-table.c Wed Mar 31 17:44:10 2010
@@ -163,6 +163,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/branches/1.6.x/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/libsvn_fs_fs/fs_fs.c?rev=929634&r1=929633&r2=929634&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/branches/1.6.x/subversion/libsvn_fs_fs/fs_fs.c Wed Mar 31 17:44:10 2010
@@ -3698,6 +3698,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/branches/1.6.x/subversion/tests/libsvn_fs_base/changes-test.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/tests/libsvn_fs_base/changes-test.c?rev=929634&r1=929633&r2=929634&view=diff
==============================================================================
--- subversion/branches/1.6.x/subversion/tests/libsvn_fs_base/changes-test.c (original)
+++ subversion/branches/1.6.x/subversion/tests/libsvn_fs_base/changes-test.c Wed Mar 31 17:44:10 2010
@@ -727,6 +727,211 @@ changes_fetch_ordering(const char **msg,
}
+static svn_error_t *
+changes_bad_sequences(const char **msg,
+ svn_boolean_t msg_only,
+ 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;
+
+ *msg = "verify that bad change sequences raise errors";
+
+ if (msg_only)
+ return SVN_NO_ERROR;
+
+ /* 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. */
@@ -738,5 +943,6 @@ struct svn_test_descriptor_t test_funcs[
SVN_TEST_PASS(changes_delete),
SVN_TEST_PASS(changes_fetch),
SVN_TEST_PASS(changes_fetch_ordering),
+ SVN_TEST_PASS(changes_bad_sequences),
SVN_TEST_NULL
};