On Sun, Nov 24, 2013 at 03:44:44AM -0500, Jeff King wrote:
> In any code path where we call parse_object, we double-check that the
> result matches the sha1 we asked for. But low-level commands like
> cat-file just call read_sha1_file directly, and do not have such a
> check. We could add it, but I suspect the processing cost would be
> noticeable.
Curious, I tested this. It is noticeable. Here's the best-of-five
timings for the patch below when running a --batch cat-file on every
object in my git.git repo, using the patch below:
[before]
real 0m12.941s
user 0m12.700s
sys 0m0.244s
[after]
real 0m15.800s
user 0m15.472s
sys 0m0.344s
So it's about 20% slower. I don't know what the right tradeoff is. It's
cool to check the data each time we look at it, but it does carry a
performance penalty. I notice elsewhere in git we are inconsistent. If
you call parse_object() on an object, you get the sha1 check. But if you
just call parse_commit() on something you know to be a commit (e.g.,
because you are traversing the history and looked it up as a parent
pointer), you do not. I don't know if that is oversight, or an
intentional performance decision.
-Peff
---
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index b2ca775..2b09773 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -199,6 +199,8 @@ static void print_object_or_die(int fd, const unsigned char *sha1,
if (type == OBJ_BLOB) {
if (stream_blob_to_fd(fd, sha1, NULL, 0) < 0)
die("unable to stream %s to stdout", sha1_to_hex(sha1));
+ if (check_sha1_signature(sha1, NULL, 0, NULL) < 0)
+ die("object %s sha1 mismatch", sha1_to_hex(sha1));
}
else {
enum object_type rtype;
@@ -208,6 +210,8 @@ static void print_object_or_die(int fd, const unsigned char *sha1,
contents = read_sha1_file(sha1, &rtype, &rsize);
if (!contents)
die("object %s disappeared", sha1_to_hex(sha1));
+ if (check_sha1_signature(sha1, contents, rsize, typename(rtype)) < 0)
+ die("object %s sha1 mismatch", sha1_to_hex(sha1));
if (rtype != type)
die("object %s changed type!?", sha1_to_hex(sha1));
if (rsize != size)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html