{"thread":{"id":"14995","subject":"[PATCH] Allow emails with boundaries to work again","startedAt":"2008-08-13T22:45:59Z","lastAt":"2008-08-14T01:56:04Z","messageCount":5,"participants":["Don Zickus","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"87121","messageId":"1218667559-26618-1-git-send-email-dzickus@redhat.com","threadId":"14995","inReplyTo":null,"subject":"[PATCH] Allow emails with boundaries to work again","fromName":"Don Zickus","fromEmail":"dzickus@redhat.com","sentAt":"2008-08-13T22:45:59Z","receivedAt":"2008-08-13T22:45:59Z","isPatch":true,"sender":{"key":"dzickus@redhat.com","avatar":null},"body":"Recent changes to is_multipart_boundary() caused git-mailinfo to segfault.\nThe reason was after handling the end of the boundary the code tried to look\nfor another boundary. Because the boundary list was empty, dereferencing\nthe pointer to the top of the boundary caused the program to go boom.\n\nThe fix is to check to see if the list is empty and if so go on its merry\nway instead of looking for another boundary.\n\nI also fixed a couple of increments and decrements that didn't look correct\nrelating to content_top.\n\nSigned-Off-by: Don Zickus \n---\n builtin-mailinfo.c | 12 +++++++-----\n 1 files changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\nindex 6ae2bf3..0209e82 100644\n--- a/builtin-mailinfo.c\n+++ b/builtin-mailinfo.c\n@@ -175,7 +175,7 @@ static void handle_content_type(struct strbuf *line)\n \t\t message_type = TYPE_OTHER;\n \tif (slurp_attr(line->buf, \"boundary=\", boundary)) {\n \t\tstrbuf_insert(boundary, 0, \"--\", 2);\n-\t\tif (content_top++ >= &content[MAX_BOUNDARIES]) {\n+\t\tif (++content_top > &content[MAX_BOUNDARIES]) {\n \t\t\tfprintf(stderr, \"Too many boundaries to handle\\n\");\n \t\t\texit(1);\n \t\t}\n@@ -626,7 +626,7 @@ again:\n \t\t/* technically won't happen as is_multipart_boundary()\n \t\t will fail first. But just in case..\n \t\t */\n-\t\tif (content_top-- < content) {\n+\t\tif (--content_top < content) {\n \t\t\tfprintf(stderr, \"Detected mismatched boundaries, \"\n \t\t\t\t\t\"can't recover\\n\");\n \t\t\texit(1);\n@@ -635,9 +635,11 @@ again:\n \t\tstrbuf_release(&newline);\n \n \t\t/* skip to the next boundary */\n-\t\tif (!find_boundary())\n-\t\t\treturn 0;\n-\t\tgoto again;\n+\t\tif (*content_top) {\n+\t\t\tif (!find_boundary())\n+\t\t\t\treturn 0;\n+\t\t\tgoto again;\n+\t\t}\n \t}\n \n \t/* set some defaults */\n-- \n1.5.5.1\n"},{"id":"87125","messageId":"7vbpzwjx8d.fsf@gitster.siamese.dyndns.org","threadId":"14995","inReplyTo":"1218667559-26618-1-git-send-email-dzickus@redhat.com","subject":"Re: [PATCH] Allow emails with boundaries to work again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-13T23:45:06Z","receivedAt":"2008-08-13T23:45:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Don Zickus writes:\n\n> Recent changes to is_multipart_boundary() caused git-mailinfo to segfault.\n> The reason was after handling the end of the boundary the code tried to look\n> for another boundary. Because the boundary list was empty, dereferencing\n> the pointer to the top of the boundary caused the program to go boom.\n\nWe keep fixing and breaking this thing, don't we? Can we have a testcase\nto protect this codepath?\n"},{"id":"87133","messageId":"20080814005631.GC24172@redhat.com","threadId":"14995","inReplyTo":"7vbpzwjx8d.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow emails with boundaries to work again","fromName":"Don Zickus","fromEmail":"dzickus@redhat.com","sentAt":"2008-08-14T00:56:31Z","receivedAt":"2008-08-14T00:56:31Z","isPatch":true,"sender":{"key":"dzickus@redhat.com","avatar":null},"body":"On Wed, Aug 13, 2008 at 04:45:06PM -0700, Junio C Hamano wrote:\n> Don Zickus writes:\n> \n> > Recent changes to is_multipart_boundary() caused git-mailinfo to segfault.\n> > The reason was after handling the end of the boundary the code tried to look\n> > for another boundary. Because the boundary list was empty, dereferencing\n> > the pointer to the top of the boundary caused the program to go boom.\n> \n> We keep fixing and breaking this thing, don't we? Can we have a testcase\n> to protect this codepath?\n\nHeh. Ok I will try to work on one for tomorrow.\n\nCheers,\nDon\n"},{"id":"87137","messageId":"7vtzdogyx6.fsf@gitster.siamese.dyndns.org","threadId":"14995","inReplyTo":"1218667559-26618-1-git-send-email-dzickus@redhat.com","subject":"Re: [PATCH] Allow emails with boundaries to work again","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-14T01:36:53Z","receivedAt":"2008-08-14T01:36:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Don Zickus writes:\n\n> Recent changes to is_multipart_boundary() caused git-mailinfo to segfault.\n> The reason was after handling the end of the boundary the code tried to look\n> for another boundary. Because the boundary list was empty, dereferencing\n> the pointer to the top of the boundary caused the program to go boom.\n>\n> The fix is to check to see if the list is empty and if so go on its merry\n> way instead of looking for another boundary.\n\nHmm, at this point !*content_top means that we are at the outermost level\nand we have just seen --boundary-- which is the terminating one, haven't\nwe? Shouldn't we be simply returning?\n"},{"id":"87138","messageId":"20080814015604.GD24172@redhat.com","threadId":"14995","inReplyTo":"7vtzdogyx6.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Allow emails with boundaries to work again","fromName":"Don Zickus","fromEmail":"dzickus@redhat.com","sentAt":"2008-08-14T01:56:04Z","receivedAt":"2008-08-14T01:56:04Z","isPatch":true,"sender":{"key":"dzickus@redhat.com","avatar":null},"body":"On Wed, Aug 13, 2008 at 06:36:53PM -0700, Junio C Hamano wrote:\n> Don Zickus writes:\n> \n> > Recent changes to is_multipart_boundary() caused git-mailinfo to segfault.\n> > The reason was after handling the end of the boundary the code tried to look\n> > for another boundary. Because the boundary list was empty, dereferencing\n> > the pointer to the top of the boundary caused the program to go boom.\n> >\n> > The fix is to check to see if the list is empty and if so go on its merry\n> > way instead of looking for another boundary.\n> \n> Hmm, at this point !*content_top means that we are at the outermost level\n> and we have just seen --boundary-- which is the terminating one, haven't\n> we? Shouldn't we be simply returning?\n\nThat's what I originally did, but then I realized the rest of the\nhandle_boundary() reads the next line of text, which is needed to continue\nprocessing in handle_body(). :-)\n\nCheers,\nDon\n"}]}