Index: subversion/libsvn_fs/fs-loader.c =================================================================== --- subversion/libsvn_fs/fs-loader.c (revision 1663355) +++ subversion/libsvn_fs/fs-loader.c (working copy) @@ -929,13 +929,22 @@ svn_fs_txn_proplist(apr_hash_t **table_p, svn_fs_t return svn_error_trace(txn->vtable->get_proplist(table_p, txn, pool)); } +static svn_boolean_t +is_internal_txn_prop(const char *name) +{ + return strcmp(name, SVN_FS__PROP_TXN_CHECK_LOCKS) == 0 || + strcmp(name, SVN_FS__PROP_TXN_CHECK_OOD) == 0 || + strcmp(name, SVN_FS__PROP_TXN_CLIENT_DATE) == 0; +} + svn_error_t * svn_fs_change_txn_prop(svn_fs_txn_t *txn, const char *name, const svn_string_t *value, apr_pool_t *pool) { - /* Silently drop attempts to modify the internal property. */ - if (!strcmp(name, SVN_FS__PROP_TXN_CLIENT_DATE)) - return SVN_NO_ERROR; + if (is_internal_txn_prop(name)) + return svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL, + _("Attempt to modify internal transaction " + "property '%s'"), name); return svn_error_trace(txn->vtable->change_prop(txn, name, value, pool)); } @@ -946,25 +955,14 @@ svn_fs_change_txn_props(svn_fs_txn_t *txn, const a { int i; - /* Silently drop attempts to modify the internal property. */ for (i = 0; i < props->nelts; ++i) { svn_prop_t *prop = &APR_ARRAY_IDX(props, i, svn_prop_t); - if (!strcmp(prop->name, SVN_FS__PROP_TXN_CLIENT_DATE)) - { - apr_array_header_t *reduced_props - = apr_array_make(pool, props->nelts - 1, sizeof(svn_prop_t)); - - for (i = 0; i < props->nelts; ++i) - { - prop = &APR_ARRAY_IDX(props, i, svn_prop_t); - if (strcmp(prop->name, SVN_FS__PROP_TXN_CLIENT_DATE)) - APR_ARRAY_PUSH(reduced_props, svn_prop_t) = *prop; - } - props = reduced_props; - break; - } + if (is_internal_txn_prop(prop->name)) + return svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL, + _("Attempt to modify internal transaction " + "property '%s'"), prop->name); } return svn_error_trace(txn->vtable->change_props(txn, props, pool)); Index: subversion/tests/libsvn_fs/fs-test.c =================================================================== --- subversion/tests/libsvn_fs/fs-test.c (revision 1663355) +++ subversion/tests/libsvn_fs/fs-test.c (working copy) @@ -5196,29 +5196,6 @@ commit_timestamp(const svn_test_opts_t *opts, /* Commit that overwrites the specified svn:date. */ SVN_ERR(svn_fs_begin_txn(&txn, fs, rev, pool)); - { - /* Setting the internal property doesn't enable svn:date behaviour. */ - apr_array_header_t *props = apr_array_make(pool, 3, sizeof(svn_prop_t)); - svn_prop_t prop, other_prop1, other_prop2; - svn_string_t *val; - - prop.name = SVN_FS__PROP_TXN_CLIENT_DATE; - prop.value = svn_string_create("1", pool); - other_prop1.name = "foo"; - other_prop1.value = svn_string_create("fooval", pool); - other_prop2.name = "bar"; - other_prop2.value = svn_string_create("barval", pool); - APR_ARRAY_PUSH(props, svn_prop_t) = other_prop1; - APR_ARRAY_PUSH(props, svn_prop_t) = prop; - APR_ARRAY_PUSH(props, svn_prop_t) = other_prop2; - SVN_ERR(svn_fs_change_txn_props(txn, props, pool)); - SVN_ERR(svn_fs_txn_prop(&val, txn, other_prop1.name, pool)); - SVN_TEST_ASSERT(val && !strcmp(val->data, other_prop1.value->data)); - SVN_ERR(svn_fs_txn_prop(&val, txn, other_prop2.name, pool)); - SVN_TEST_ASSERT(val && !strcmp(val->data, other_prop2.value->data)); - - SVN_ERR(svn_fs_change_txn_prop(txn, prop.name, prop.value, pool)); - } SVN_ERR(svn_fs_txn_root(&txn_root, txn, pool)); SVN_ERR(svn_fs_make_dir(txn_root, "/bar", pool)); SVN_ERR(svn_fs_change_txn_prop(txn, SVN_PROP_REVISION_DATE, date, pool)); @@ -6698,6 +6675,119 @@ test_prop_and_text_rep_sharing_collision(const svn return SVN_NO_ERROR; } +static svn_error_t * +test_internal_txn_props(const svn_test_opts_t *opts, + apr_pool_t *pool) +{ + svn_fs_t *fs; + svn_fs_txn_t *txn; + svn_string_t *val; + svn_prop_t prop; + svn_prop_t internal_prop; + apr_array_header_t *props; + apr_hash_t *actual_props; + svn_error_t *err; + + SVN_ERR(svn_test__create_fs(&fs, "test-repo-internal-txn-props", + opts, pool)); + SVN_ERR(svn_fs_begin_txn2(&txn, fs, 0, + SVN_FS_TXN_CHECK_OOD | + SVN_FS_TXN_CHECK_LOCKS | + SVN_FS_TXN_CLIENT_DATE, pool)); + + val = svn_string_create("Ooops!", pool); + + /* All attempts to modify internal txn properties should error out. */ + err = svn_fs_change_txn_prop(txn, SVN_FS__PROP_TXN_CHECK_OOD, val, pool); + SVN_TEST_ASSERT_ERROR(err, SVN_ERR_INCORRECT_PARAMS); + err = svn_fs_change_txn_prop(txn, SVN_FS__PROP_TXN_CHECK_OOD, NULL, pool); + SVN_TEST_ASSERT_ERROR(err, SVN_ERR_INCORRECT_PARAMS); + err = svn_fs_change_txn_prop(txn, SVN_FS__PROP_TXN_CHECK_LOCKS, val, pool); + SVN_TEST_ASSERT_ERROR(err, SVN_ERR_INCORRECT_PARAMS); + err = svn_fs_change_txn_prop(txn, SVN_FS__PROP_TXN_CHECK_LOCKS, NULL, pool); + SVN_TEST_ASSERT_ERROR(err, SVN_ERR_INCORRECT_PARAMS); + err = svn_fs_change_txn_prop(txn, SVN_FS__PROP_TXN_CLIENT_DATE, val, pool); + SVN_TEST_ASSERT_ERROR(err, SVN_ERR_INCORRECT_PARAMS); + err = svn_fs_change_txn_prop(txn, SVN_FS__PROP_TXN_CLIENT_DATE, NULL, pool); + SVN_TEST_ASSERT_ERROR(err, SVN_ERR_INCORRECT_PARAMS); + + /* Try the same with svn_fs_change_txn_props(). */ + prop.name = "foo"; + prop.value = svn_string_create("bar", pool); + internal_prop.name = SVN_FS__PROP_TXN_CHECK_LOCKS; + internal_prop.value = svn_string_create("Ooops!", pool); + + props = apr_array_make(pool, 2, sizeof(svn_prop_t)); + APR_ARRAY_PUSH(props, svn_prop_t) = prop; + APR_ARRAY_PUSH(props, svn_prop_t) = internal_prop; + + err = svn_fs_change_txn_props(txn, props, pool); + SVN_TEST_ASSERT_ERROR(err, SVN_ERR_INCORRECT_PARAMS); + + /* Ensure that the txn properties are left unchanged: + + K 13 + svn:check-ood + V 4 + true + K 15 + svn:check-locks + V 4 + true + K 15 + svn:client-date + V 1 + 0 + K 8 + svn:date + V 27 + (timestamp like "2015-03-02T12:16:53.148931Z") + END + */ + SVN_ERR(svn_fs_txn_proplist(&actual_props, txn, pool)); + + SVN_TEST_ASSERT(apr_hash_count(actual_props) == 4); + + val = svn_hash_gets(actual_props, SVN_FS__PROP_TXN_CHECK_OOD); + SVN_TEST_ASSERT(val); + SVN_TEST_STRING_ASSERT(val->data, "true"); + + val = svn_hash_gets(actual_props, SVN_FS__PROP_TXN_CHECK_LOCKS); + SVN_TEST_ASSERT(val); + SVN_TEST_STRING_ASSERT(val->data, "true"); + + val = svn_hash_gets(actual_props, SVN_FS__PROP_TXN_CLIENT_DATE); + SVN_TEST_ASSERT(val); + + if (strcmp(opts->fs_type, SVN_FS_TYPE_BDB) == 0) + { + /* ### Skip the svn:client-date check with BDB. I think that we have a + bug in the BDB backend, because when SVN_FS_TXN_CLIENT_DATE is + set, a transaction created with svn_fs_begin_txn2() has the + svn:client-date property value = "1", even though no explicit + svn:date changes happened (it's "0" in FSFS). + + This happens because svn_fs_base__begin_txn() sets svn:date in a + separate svn_fs_base__change_txn_prop() call when the initial + internal properties, including SVN_FS__PROP_TXN_CLIENT_DATE, are + already set. As a consequence, we should have different bevavior + for transactions created with SVN_FS_TXN_CLIENT_DATE, and with no + explicit svn:date changes happening depending on the FS backend. + When such transaction is committed, FSFS is going to use the time + of the commit for the 'svn:date' value. BDB, however, is going + to erroneously use the timestamp of the svn_fs_base__begin_txn() + call. + */ + } + else + SVN_TEST_STRING_ASSERT(val->data, "0"); + + val = svn_hash_gets(actual_props, SVN_PROP_REVISION_DATE); + SVN_TEST_ASSERT(val); + + return SVN_NO_ERROR; +} + /* ------------------------------------------------------------------------ */ /* The test table. */ @@ -6829,6 +6919,8 @@ static struct svn_test_descriptor_t test_funcs[] = "test modify txn being written in FSFS"), SVN_TEST_OPTS_PASS(test_prop_and_text_rep_sharing_collision, "test property and text rep-sharing collision"), + SVN_TEST_OPTS_PASS(test_internal_txn_props, + "test attempts to change internal txn properties"), SVN_TEST_NULL };