From d21c463d558a1450c2560869193f279fc7ddba4a Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 15 Mar 2012 14:57:02 -0700 Subject: [PATCH] fetch/receive: remove over-pessimistic connectivity check MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Git 1.7.8 introduced an object and history re-validation step after "fetch" or "push" causes new history to be added to a receiving repository. This is to protect a malicious server or pushing client from corrupting the repository by taking advantage of an existing corrupt object that is unconnected to existing history. But this check is way over-pessimistic. During "fetch" or "receive-pack" (the server side of "push"), unpack-objects and index-pack already validate individual objects that are received, and the only thing we would want to catch are corrupted objects that already happen to exist in our repository but are not referenced from our refs. Such objects must have been written by an earlier run of our codepaths that write out loose objects or packfiles, and they must have done the validation of individual objects when they did so. The only thing left to worry about is the connectivity integrity, which can be checked with "rev-list --objects", which is much cheaper. We have been paying the 5x to 8x runtime overhead the --verify-objects often adds for no real gain. Revert check_everything_connected() not to use this over-pessimistic check. Credit goes to Nguyễn Thái Ngọc Duy, who originally identified the performance regression and endured multiple rounds of reviews to fix it. Signed-off-by: Junio C Hamano --- connected.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/connected.c b/connected.c index d7624230d..1e89c1cd1 100644 --- a/connected.c +++ b/connected.c @@ -6,18 +6,18 @@ /* * If we feed all the commits we want to verify to this command * - * $ git rev-list --verify-objects --stdin --not --all + * $ git rev-list --objects --stdin --not --all * * and if it does not error out, that means everything reachable from - * these commits locally exists and is connected to some of our - * existing refs. + * these commits locally exists and is connected to our existing refs. + * Note that this does _not_ validate the individual objects. * * Returns 0 if everything is connected, non-zero otherwise. */ int check_everything_connected(sha1_iterate_fn fn, int quiet, void *cb_data) { struct child_process rev_list; - const char *argv[] = {"rev-list", "--verify-objects", + const char *argv[] = {"rev-list", "--objects", "--stdin", "--not", "--all", NULL, NULL}; char commit[41]; unsigned char sha1[20]; -- 2.26.2