You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2015/12/12 20:36:09 UTC

svn commit: r1719726 - in /subversion/trunk/subversion/libsvn_fs_x: fs_x.c revprops.c revprops.h transaction.c

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;
 }