All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Ivankov <divanorama@gmail.com>
To: git@vger.kernel.org
Cc: Jonathan Nieder <jrnieder@gmail.com>,
	"Shawn O. Pearce" <spearce@spearce.org>,
	David Barr <davidbarr@google.com>,
	Dmitry Ivankov <divanorama@gmail.com>
Subject: [PATCH 2/3] fast-import: add a check for tree delta base sha1
Date: Fri, 12 Aug 2011 16:32:49 +0600	[thread overview]
Message-ID: <1313145170-24471-3-git-send-email-divanorama@gmail.com> (raw)
In-Reply-To: <1313145170-24471-1-git-send-email-divanorama@gmail.com>

fast-import is able to write imported tree objects in delta format.
It holds a tree structure in memory where each tree entry may have
a delta base sha1 assigned. When delta base data is needed it is
reconstructed from this in-memory structure. Though sometimes the
delta base data doesn't match the delta base sha1 so wrong or even
corrupt pack is produced.

To create a small easily reproducible test, add an excessive check
for delta base sha1. It's not likely that computing sha1 for each
tree delta base costs us much.

Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
 fast-import.c          |   20 +++++++++++++++-----
 t/t9300-fast-import.sh |   38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index d0f8580..8196d1b 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1455,12 +1455,22 @@ static void store_tree(struct tree_entry *root)
 			store_tree(t->entries[i]);
 	}
 
-	le = find_object(root->versions[0].sha1);
-	if (S_ISDIR(root->versions[0].mode) && le && le->pack_id == pack_id) {
+	if (!is_null_sha1(root->versions[0].sha1)
+					&& S_ISDIR(root->versions[0].mode)) {
+		unsigned char old_tree_sha1[20];
 		mktree(t, 0, &old_tree);
-		lo.data = old_tree;
-		lo.offset = le->idx.offset;
-		lo.depth = t->delta_depth;
+		prepare_object_hash(OBJ_TREE, &old_tree,
+						NULL, NULL, old_tree_sha1);
+
+		if (hashcmp(old_tree_sha1, root->versions[0].sha1))
+			die("internal tree delta base sha1 mismatch");
+
+		le = find_object(root->versions[0].sha1);
+		if (le && le->pack_id == pack_id) {
+			lo.data = old_tree;
+			lo.offset = le->idx.offset;
+			lo.depth = t->delta_depth;
+		}
 	}
 
 	mktree(t, 1, &new_tree);
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index f256475..c70e489 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -734,6 +734,44 @@ test_expect_success \
 	 git diff-tree --abbrev --raw L^ L >output &&
 	 test_cmp expect output'
 
+cat >input <<INPUT_END
+blob
+mark :1
+data <<EOF
+the data
+EOF
+
+commit refs/heads/L2
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+init L2
+COMMIT
+
+M 644 :1 a/b/c
+M 644 :1 a/b/d
+M 644 :1 a/e/f
+INPUT_END
+
+cat >input2 <<INPUT_END
+commit refs/heads/L2
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+update L2
+COMMIT
+from refs/heads/L2^0
+M 040000 @A g
+M 040000 @E g/b
+M 040000 @E g/b/h
+INPUT_END
+
+test_expect_failure \
+    'L: verify internal tree delta base' \
+	'git fast-import <input &&
+	A=$(git ls-tree L2 a | tr " " "\t" | cut -f 3) &&
+	E=$(git ls-tree L2 a/e | tr " " "\t" | cut -f 3) &&
+	cat input2 | sed -e "s/@A/$A/" -e "s/@E/$E/" >input &&
+	git fast-import <input'
+
 ###
 ### series M
 ###
-- 
1.7.3.4

  parent reply	other threads:[~2011-08-12 10:32 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-12 10:32 [PATCH 0/3] fix data corruption in fast-import Dmitry Ivankov
2011-08-12 10:32 ` [PATCH 1/3] fast-import: extract object preparation function Dmitry Ivankov
2011-08-12 10:32 ` Dmitry Ivankov [this message]
2011-08-13 21:02   ` [PATCH 2/3] fast-import: add a check for tree delta base sha1 Jonathan Nieder
2011-08-12 10:32 ` [PATCH 3/3] fast-import: prevent producing bad delta Dmitry Ivankov
2011-08-14 18:32 ` [PATCH v2 0/2] fix data corruption in fast-import Dmitry Ivankov
2011-08-14 18:32 ` [PATCH v2 1/2] fast-import: add a test for tree delta base corruption Dmitry Ivankov
2011-08-14 18:32 ` [PATCH v2 2/2] fast-import: prevent producing bad delta Dmitry Ivankov
2011-08-20  1:09   ` [PATCH v3] fast-import: do not write bad delta for replaced subtrees Jonathan Nieder
2011-08-20  9:08     ` Andreas Schwab
2011-08-20 15:43       ` Jonathan Nieder
2011-08-20 17:22         ` [PATCH v4] " Dmitry Ivankov
2011-08-20 17:48           ` Jonathan Nieder
2011-08-20 18:28             ` Dmitry Ivankov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1313145170-24471-3-git-send-email-divanorama@gmail.com \
    --to=divanorama@gmail.com \
    --cc=davidbarr@google.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=spearce@spearce.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.