Author: stefan2 Date: Sat Dec 12 19:36:09 2015 New Revision: 1719726 URL: http://svn.apache.org/viewvc?rev=1719726&view=rev Log: Finally, add checksums to non-packed revprop files as well. All revprop data is checksummed now.
* subversion/libsvn_fs_x/revprops.h (svn_fs_x__write_non_packed_revprops): Declare new internal API to avoid code duplication. * subversion/libsvn_fs_x/fs_x.c (write_revision_zero): Use the new API to create the revprops. * subversion/libsvn_fs_x/transaction.c (write_final_revprop): Same. * subversion/libsvn_fs_x/revprops.c (verify_checksum): Factored out from read_packed_data_checksummed. (read_non_packed_revprop): Call the new function to verify the revprop file contents. (read_packed_data_checksummed): Call factored out function now. (svn_fs_x__write_non_packed_revprops): Implement the new API. (write_non_packed_revprop): Use the new API to write revprops. (copy_revprops): Verify revprops before packing them. Modified: subversion/trunk/subversion/libsvn_fs_x/fs_x.c subversion/trunk/subversion/libsvn_fs_x/revprops.c subversion/trunk/subversion/libsvn_fs_x/revprops.h subversion/trunk/subversion/libsvn_fs_x/transaction.c Modified: subversion/trunk/subversion/libsvn_fs_x/fs_x.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/fs_x.c?rev=1719726&r1=1719725&r2=1719726&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_fs_x/fs_x.c (original) +++ subversion/trunk/subversion/libsvn_fs_x/fs_x.c Sat Dec 12 19:36:09 2015 @@ -860,8 +860,6 @@ write_revision_zero(svn_fs_t *fs, const char *path_revision_zero = svn_fs_x__path_rev(fs, 0, scratch_pool); apr_hash_t *proplist; svn_string_t date; - svn_stream_t *stream; - svn_stringbuf_t *revprops; apr_array_header_t *index_entries; svn_fs_x__p2l_entry_t *entry; @@ -934,15 +932,13 @@ write_revision_zero(svn_fs_t *fs, proplist = apr_hash_make(scratch_pool); svn_hash_sets(proplist, SVN_PROP_REVISION_DATE, &date); - revprops = svn_stringbuf_create_empty(scratch_pool); - stream = svn_stream_from_stringbuf(revprops, scratch_pool); - SVN_ERR(svn_fs_x__write_properties(stream, proplist, scratch_pool)); - SVN_ERR(svn_stream_close(stream)); - - SVN_ERR(svn_io_file_create_bytes(svn_fs_x__path_revprops(fs, 0, - scratch_pool), - revprops->data, revprops->len, - scratch_pool)); + SVN_ERR(svn_io_file_open(&apr_file, + svn_fs_x__path_revprops(fs, 0, scratch_pool), + APR_WRITE | APR_CREATE, APR_OS_DEFAULT, + scratch_pool)); + SVN_ERR(svn_fs_x__write_non_packed_revprops(apr_file, proplist, + scratch_pool)); + SVN_ERR(svn_io_file_close(apr_file, scratch_pool)); return SVN_NO_ERROR; } Modified: subversion/trunk/subversion/libsvn_fs_x/revprops.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/revprops.c?rev=1719726&r1=1719725&r2=1719726&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_fs_x/revprops.c (original) +++ subversion/trunk/subversion/libsvn_fs_x/revprops.c Sat Dec 12 19:36:09 2015 @@ -426,6 +426,35 @@ parse_revprop(apr_hash_t **properties, return SVN_NO_ERROR; } +/* Verify the checksum attached to CONTENT and remove it. + * Use SCRATCH_POOL for temporary allocations. + */ +static svn_error_t * +verify_checksum(svn_stringbuf_t *content, + apr_pool_t *scratch_pool) +{ + const apr_byte_t *digest; + svn_checksum_t *actual, *expected; + + /* Verify the checksum. */ + if (content->len < sizeof(apr_uint32_t)) + return svn_error_create(SVN_ERR_CORRUPT_PACKED_DATA, NULL, + "File too short"); + + content->len -= sizeof(apr_uint32_t); + digest = (apr_byte_t *)content->data + content->len; + + expected = svn_checksum__from_digest_fnv1a_32x4(digest, scratch_pool); + SVN_ERR(svn_checksum(&actual, svn_checksum_fnv1a_32x4, content->data, + content->len, scratch_pool)); + + if (!svn_checksum_match(actual, expected)) + SVN_ERR(svn_checksum_mismatch_err(expected, actual, scratch_pool, + "checksum mismatch")); + + return SVN_NO_ERROR; +} + /* Read the non-packed revprops for revision REV in FS, put them into the * revprop cache if activated and return them in *PROPERTIES. * @@ -460,10 +489,17 @@ read_non_packed_revprop(apr_hash_t **pro if (content) { + svn_string_t *as_string; + + /* Consistency check. */ + SVN_ERR_W(verify_checksum(content, scratch_pool), + apr_psprintf(scratch_pool, + "Revprop file for r%ld is corrupt", + rev)); + /* The contents string becomes part of the *PROPERTIES structure, i.e. * we must make sure it lives at least as long as the latter. */ - svn_string_t *as_string = svn_string_create_from_buf(content, - result_pool); + as_string = svn_string_create_from_buf(content, result_pool); SVN_ERR(parse_revprop(properties, fs, rev, as_string, result_pool, iterpool)); } @@ -539,26 +575,9 @@ read_packed_data_checksummed(svn_packed_ apr_pool_t *result_pool, apr_pool_t *scratch_pool) { - apr_size_t data_len; - const apr_byte_t *digest; - svn_checksum_t *actual, *expected; svn_stream_t *stream; - /* Verify the checksum. */ - if (content->len < sizeof(apr_uint32_t)) - return svn_error_create(SVN_ERR_CORRUPT_PACKED_DATA, NULL, - "File too short"); - - data_len = content->len - sizeof(apr_uint32_t); - digest = (apr_byte_t *)content->data + data_len; - - expected = svn_checksum__from_digest_fnv1a_32x4(digest, scratch_pool); - SVN_ERR(svn_checksum(&actual, svn_checksum_fnv1a_32x4, content->data, - data_len, scratch_pool)); - - if (!svn_checksum_match(actual, expected)) - SVN_ERR(svn_checksum_mismatch_err(expected, actual, scratch_pool, - "checksum mismatch")); + SVN_ERR(verify_checksum(content, scratch_pool)); stream = svn_stream_from_stringbuf(content, scratch_pool); SVN_ERR(svn_packed__data_read(root, stream, result_pool, scratch_pool)); @@ -993,6 +1012,29 @@ svn_fs_x__get_revision_proplist(apr_hash return SVN_NO_ERROR; } +svn_error_t * +svn_fs_x__write_non_packed_revprops(apr_file_t *file, + apr_hash_t *proplist, + apr_pool_t *scratch_pool) +{ + svn_stream_t *stream; + svn_checksum_t *checksum; + + stream = svn_stream_from_aprfile2(file, TRUE, scratch_pool); + stream = svn_checksum__wrap_write_stream(&checksum, stream, + svn_checksum_fnv1a_32x4, + scratch_pool); + SVN_ERR(svn_fs_x__write_properties(stream, proplist, scratch_pool)); + SVN_ERR(svn_stream_close(stream)); + + /* Append the checksum */ + SVN_ERR(svn_io_file_write_full(file, checksum->digest, + svn_checksum_size(checksum), NULL, + scratch_pool)); + + return SVN_NO_ERROR; +} + /* Serialize the revision property list PROPLIST of revision REV in * filesystem FS to a non-packed file. Return the name of that temporary * file in *TMP_PATH and the file path that it must be moved to in @@ -1012,16 +1054,13 @@ write_non_packed_revprop(const char **fi apr_pool_t *scratch_pool) { apr_file_t *file; - svn_stream_t *stream; *final_path = svn_fs_x__path_revprops(fs, rev, result_pool); *tmp_path = apr_pstrcat(result_pool, *final_path, ".tmp", SVN_VA_NULL); SVN_ERR(svn_fs_x__batch_fsync_open_file(&file, batch, *tmp_path, scratch_pool)); - stream = svn_stream_from_aprfile2(file, TRUE, scratch_pool); - SVN_ERR(svn_fs_x__write_properties(stream, proplist, scratch_pool)); - SVN_ERR(svn_stream_close(stream)); + SVN_ERR(svn_fs_x__write_non_packed_revprops(file, proplist, scratch_pool)); return SVN_NO_ERROR; } @@ -1499,6 +1538,10 @@ copy_revprops(svn_fs_t *fs, /* Copy all the bits from the non-packed revprop file to the end of * the pack file. */ SVN_ERR(svn_stringbuf_from_file2(&props, path, iterpool)); + SVN_ERR_W(verify_checksum(props, iterpool), + apr_psprintf(iterpool, "Failed to read revprops for r%ld.", + rev)); + svn_packed__add_bytes(stream, props->data, props->len); } Modified: subversion/trunk/subversion/libsvn_fs_x/revprops.h URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/revprops.h?rev=1719726&r1=1719725&r2=1719726&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_fs_x/revprops.h (original) +++ subversion/trunk/subversion/libsvn_fs_x/revprops.h Sat Dec 12 19:36:09 2015 @@ -46,6 +46,17 @@ svn_fs_x__reset_revprop_generation_file( void svn_fs_x__invalidate_revprop_generation(svn_fs_t *fs); +/* Utility function serializing PROPLIST into FILE and adding the checksum. + * Use SCRATCH_POOL for temporary allocations. + * + * Call this only when creating initial revprop file contents. + * For modifications use svn_fs_x__set_revision_proplist. + */ +svn_error_t * +svn_fs_x__write_non_packed_revprops(apr_file_t *file, + apr_hash_t *proplist, + apr_pool_t *scratch_pool); + /* Read the revprops for revision REV in FS and return them in *PROPLIST_P. * If BYPASS_CACHE is set, don't consult the disks but always read from disk. * If REFRESH is set, update the revprop generation info; otherwise access Modified: subversion/trunk/subversion/libsvn_fs_x/transaction.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/transaction.c?rev=1719726&r1=1719725&r2=1719726&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_fs_x/transaction.c (original) +++ subversion/trunk/subversion/libsvn_fs_x/transaction.c Sat Dec 12 19:36:09 2015 @@ -43,6 +43,7 @@ #include "rep-cache.h" #include "index.h" #include "batch_fsync.h" +#include "revprops.h" #include "private/svn_fs_util.h" #include "private/svn_fspath.h" @@ -3434,7 +3435,6 @@ write_final_revprop(const char **path, svn_string_t date; svn_string_t *client_date; apr_file_t *file; - svn_stream_t *stream; SVN_ERR(svn_fs_x__txn_proplist(&props, txn, scratch_pool)); @@ -3463,9 +3463,7 @@ write_final_revprop(const char **path, SVN_ERR(svn_fs_x__batch_fsync_open_file(&file, batch, *path, scratch_pool)); /* Write the new contents to the final revprops file. */ - stream = svn_stream_from_aprfile2(file, TRUE, scratch_pool); - SVN_ERR(svn_fs_x__write_properties(stream, props, scratch_pool)); - SVN_ERR(svn_stream_close(stream)); + SVN_ERR(svn_fs_x__write_non_packed_revprops(file, props, scratch_pool)); return SVN_NO_ERROR; }