{"thread":{"id":"11447","subject":"[PATCH] receive-pack: check object type of sha1 before using them as commits","startedAt":"2008-01-02T07:39:21Z","lastAt":"2008-01-02T22:57:35Z","messageCount":3,"participants":["Martin Koegler","Linus Torvalds","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"64320","messageId":"11992595612601-git-send-email-mkoegler@auto.tuwien.ac.at","threadId":"11447","inReplyTo":null,"subject":"[PATCH] receive-pack: check object type of sha1 before using them as commits","fromName":"Martin Koegler","fromEmail":"mkoegler@auto.tuwien.ac.at","sentAt":"2008-01-02T07:39:21Z","receivedAt":"2008-01-02T07:39:21Z","isPatch":true,"sender":{"key":"mkoegler@auto.tuwien.ac.at","avatar":null},"body":"Signed-off-by: Martin Koegler <mkoegler@auto.tuwien.ac.at>\n---\n receive-pack.c |   14 ++++++++++++--\n 1 files changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/receive-pack.c b/receive-pack.c\nindex fba4cf8..d0a563d 100644\n--- a/receive-pack.c\n+++ b/receive-pack.c\n@@ -178,11 +178,21 @@ static const char *update(struct command *cmd)\n \tif (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&\n \t    !is_null_sha1(old_sha1) &&\n \t    !prefixcmp(name, \"refs/heads/\")) {\n+\t\tstruct object *old_object, *new_object;\n \t\tstruct commit *old_commit, *new_commit;\n \t\tstruct commit_list *bases, *ent;\n \n-\t\told_commit = (struct commit *)parse_object(old_sha1);\n-\t\tnew_commit = (struct commit *)parse_object(new_sha1);\n+\t\told_object = parse_object(old_sha1);\n+\t\tnew_object = parse_object(new_sha1);\n+\n+\t\tif (!old_object || !new_object ||\n+\t\t    old_object->type != OBJ_COMMIT ||\n+\t\t    new_object->type != OBJ_COMMIT) {\n+\t\t\terror(\"bad sha1 objects for %s\", name);\n+\t\t\treturn \"bad ref\";\n+\t\t}\n+\t\told_commit = (struct commit *)old_object;\n+\t\tnew_commit = (struct commit *)new_object;\n \t\tbases = get_merge_bases(old_commit, new_commit, 1);\n \t\tfor (ent = bases; ent; ent = ent->next)\n \t\t\tif (!hashcmp(old_sha1, ent->item->object.sha1))\n-- \n1.4.4.4\n"},{"id":"64359","messageId":"alpine.LFD.1.00.0801021418510.3010@woody.linux-foundation.org","threadId":"11447","inReplyTo":"11992595612601-git-send-email-mkoegler@auto.tuwien.ac.at","subject":"Re: [PATCH] receive-pack: check object type of sha1 before using them as commits","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-01-02T22:21:51Z","receivedAt":"2008-01-02T22:21:51Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 2 Jan 2008, Martin Koegler wrote:\n>  \n> -\t\told_commit = (struct commit *)parse_object(old_sha1);\n> -\t\tnew_commit = (struct commit *)parse_object(new_sha1);\n> +\t\told_object = parse_object(old_sha1);\n> +\t\tnew_object = parse_object(new_sha1);\n\nI think it would be better to use\n\n\told_object = lookup_commit_reference(old_sha1);\n\tif (!old_object)\n\t\treturn \"bad ref\";\n\tnew_object = lookup_commit_reference(new_sha1);\n\tif (!new_object)\n\t\treturn \"bad ref\";\n\nwhich will write a slightly more useful error message if it's not a commit \n(ie it will use the \"check_commit()\" function in commit.c)\n\n\t\tLinus\n"},{"id":"64361","messageId":"7vprwjvns0.fsf@gitster.siamese.dyndns.org","threadId":"11447","inReplyTo":"alpine.LFD.1.00.0801021418510.3010@woody.linux-foundation.org","subject":"Re: [PATCH] receive-pack: check object type of sha1 before using them as commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-01-02T22:57:35Z","receivedAt":"2008-01-02T22:57:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Wed, 2 Jan 2008, Martin Koegler wrote:\n>>  \n>> -\t\told_commit = (struct commit *)parse_object(old_sha1);\n>> -\t\tnew_commit = (struct commit *)parse_object(new_sha1);\n>> +\t\told_object = parse_object(old_sha1);\n>> +\t\tnew_object = parse_object(new_sha1);\n>\n> I think it would be better to use\n>\n> \told_object = lookup_commit_reference(old_sha1);\n> \tif (!old_object)\n> \t\treturn \"bad ref\";\n> \tnew_object = lookup_commit_reference(new_sha1);\n> \tif (!new_object)\n> \t\treturn \"bad ref\";\n>\n> which will write a slightly more useful error message if it's not a commit \n> (ie it will use the \"check_commit()\" function in commit.c)\n\nHmmm...  Three points.\n\n * lookup_commit_reference() is silent if the object pointed at\n   by old/new SHA-1 is missing.\n\n * when storing in refs/heads, we do not want to have committish\n   but we do want an actual commit object.\n\n * returning like that the user cannot tell which ref had the\n   error.\n"}]}