David Glasser wrote:
> When dealing with some nasty cases in the almost-retired old Google
> Code subversion backend, I found a kind of data corruption that the FS
> code wasn't catching even though it caught relatively similar issues.
> Specifically, find the fold_change function in both of the FS
> implementations. There's a check:
>
> /* Sanity check: an add, replacement, or reset must be the first
> thing to follow a deletion. */
> if ((old_change->change_kind == svn_fs_path_change_delete)
> && (! ((change->kind == svn_fs_path_change_replace)
> || (change->kind == svn_fs_path_change_reset)
> || (change->kind == svn_fs_path_change_add))))
> return svn_error_create
> (SVN_ERR_FS_CORRUPT, NULL,
> _("Invalid change ordering: non-add change on deleted path"));
>
> It might also be nice to check the opposite condition: if change->kind
> is add or replace, and old_change is not delete (or reset, I guess),
> throw an error. (We had accidentally recorded the sequence "add,
> prop-mod, text-mod" out of order as "prop-mod, add, text-mod", which
> was interpreted as an "R" below instead of an "A"; this suggested
> check would have caught it earlier.)Shouldn't this be just: "if change->kind is add, and old_change is not delete, throw an error"? It would be legit for someone to record the sequence "add, prop-mod, replace, text-mod", for example. Is the attached patch what you had in mind? (Plus similar logic for FSFS, of course.) -- C. Michael Pilato <cmpil...@collab.net> CollabNet <> www.collab.net <> Distributed Development On Demand
Index: subversion/libsvn_fs_base/bdb/changes-table.c
===================================================================
--- subversion/libsvn_fs_base/bdb/changes-table.c (revision 924936)
+++ subversion/libsvn_fs_base/bdb/changes-table.c (working copy)
@@ -168,6 +168,15 @@
(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)
{
signature.asc
Description: OpenPGP digital signature
