{"thread":{"id":"7336","subject":"Don't ever return corrupt objects from \"parse_object()\"","startedAt":"2007-03-20T17:05:20Z","lastAt":"2007-03-20T17:05:20Z","messageCount":1,"participants":["Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"37633","messageId":"Pine.LNX.4.64.0703200953250.6730@woody.linux-foundation.org","threadId":"7336","inReplyTo":null,"subject":"Don't ever return corrupt objects from \"parse_object()\"","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-03-20T17:05:20Z","receivedAt":"2007-03-20T17:05:20Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nLooking at the SHA1 validation code due to the corruption that Alexander \nLitvinov is seeing under Cygwin, I notice that one of the most central \nplaces where we read objects, we actually do end up verifying the SHA1 of \nthe result, but then we happily parse it anyway.\n\nAnd using \"printf\" to write the error message means that it not only can \nget lost, but will actually mess up stdout, and cause other strange and \nhard-to-debug failures downstream.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n\nOf course, messing up stdout is actually a good way to make sure that \nanything that pipes stdout to another process will cause a failure in the \nreading process, which is likely why we did it that way in the first \nplace. But any pipeline really *should* check the source process exit \nstatus anyway, and it looks rather dangerous to return a valid object as \nif nothing bad had happened!\n\nNOTE! A lot of things don't do a \"parse_object\" at all, but use the raw \n\"read_sha1_file()\" interface. So this won't catch all corrupted objects \nwhen they are used, but since we already did the check there, we should at \nleast do the right thing.\n\n\t\tLinus\n\n---\n object.c |    6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/object.c b/object.c\nindex 5b46889..78a44a6 100644\n--- a/object.c\n+++ b/object.c\n@@ -184,8 +184,10 @@ struct object *parse_object(const unsigned char *sha1)\n \n \tif (buffer) {\n \t\tstruct object *obj;\n-\t\tif (check_sha1_signature(sha1, buffer, size, typename(type)) < 0)\n-\t\t\tprintf(\"sha1 mismatch %s\\n\", sha1_to_hex(sha1));\n+\t\tif (check_sha1_signature(sha1, buffer, size, typename(type)) < 0) {\n+\t\t\terror(\"sha1 mismatch %s\\n\", sha1_to_hex(sha1));\n+\t\t\treturn NULL;\n+\t\t}\n \n \t\tobj = parse_object_buffer(sha1, type, size, buffer, &eaten);\n \t\tif (!eaten)\n"}]}