git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Junio C Hamano <junkio@cox.net>, Git Mailing List <git@vger.kernel.org>
Subject: Don't ever return corrupt objects from "parse_object()"
Date: Tue, 20 Mar 2007 10:05:20 -0700 (PDT)	[thread overview]
Message-ID: <Pine.LNX.4.64.0703200953250.6730@woody.linux-foundation.org> (raw)


Looking at the SHA1 validation code due to the corruption that Alexander 
Litvinov is seeing under Cygwin, I notice that one of the most central 
places where we read objects, we actually do end up verifying the SHA1 of 
the result, but then we happily parse it anyway.

And using "printf" to write the error message means that it not only can 
get lost, but will actually mess up stdout, and cause other strange and 
hard-to-debug failures downstream.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

Of course, messing up stdout is actually a good way to make sure that 
anything that pipes stdout to another process will cause a failure in the 
reading process, which is likely why we did it that way in the first 
place. But any pipeline really *should* check the source process exit 
status anyway, and it looks rather dangerous to return a valid object as 
if nothing bad had happened!

NOTE! A lot of things don't do a "parse_object" at all, but use the raw 
"read_sha1_file()" interface. So this won't catch all corrupted objects 
when they are used, but since we already did the check there, we should at 
least do the right thing.

		Linus

---
 object.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/object.c b/object.c
index 5b46889..78a44a6 100644
--- a/object.c
+++ b/object.c
@@ -184,8 +184,10 @@ struct object *parse_object(const unsigned char *sha1)
 
 	if (buffer) {
 		struct object *obj;
-		if (check_sha1_signature(sha1, buffer, size, typename(type)) < 0)
-			printf("sha1 mismatch %s\n", sha1_to_hex(sha1));
+		if (check_sha1_signature(sha1, buffer, size, typename(type)) < 0) {
+			error("sha1 mismatch %s\n", sha1_to_hex(sha1));
+			return NULL;
+		}
 
 		obj = parse_object_buffer(sha1, type, size, buffer, &eaten);
 		if (!eaten)

                 reply	other threads:[~2007-03-20 17:05 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=Pine.LNX.4.64.0703200953250.6730@woody.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).