{"thread":{"id":"20253","subject":"git-send-email generates mail with invalid Message-Id","startedAt":"2009-07-28T02:46:22Z","lastAt":"2009-07-28T15:07:45Z","messageCount":13,"participants":["Frans Pop","Erik Faye-Lund","Thomas Rast","Nicolas Sebrecht"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"118918","messageId":"200907280446.22890.elendil@planet.nl","threadId":"20253","inReplyTo":null,"subject":"git-send-email generates mail with invalid Message-Id","fromName":"Frans Pop","fromEmail":"elendil@planet.nl","sentAt":"2009-07-28T02:46:22Z","receivedAt":"2009-07-28T02:46:22Z","isPatch":false,"sender":{"key":"elendil@planet.nl","avatar":null},"body":"I follow lkml through a local news server (inn2), using a mail2news script \nto convert incoming mails to news items.\n\nOccasionally I get the following error in my system logs:\ninnd: localhost:18 bad_messageid <12487185672026-git-send-email->\n\nThe problem is that a Message-Id is supposed (RFC 822) to end in a domain \npart (...@example.com), but that's missing.\n\nI assume that this is a configuration issue in the git setup of the \nsender, but shouldn't git-send-email refuse to send out messages with an \ninvalid Message-Id?\n\nCheers,\nFJP\n"},{"id":"118936","messageId":"40aa078e0907280217g76cbfai8544edde605f8772@mail.gmail.com","threadId":"20253","inReplyTo":"200907280446.22890.elendil@planet.nl","subject":"Re: git-send-email generates mail with invalid Message-Id","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-07-28T09:17:10Z","receivedAt":"2009-07-28T09:17:10Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Jul 28, 2009 at 4:46 AM, Frans Pop wrote:\n> The problem is that a Message-Id is supposed (RFC 822) to end in a domain\n> part (...@example.com), but that's missing.\n\nCorrect.\n\n> I assume that this is a configuration issue in the git setup of the\n> sender, but shouldn't git-send-email refuse to send out messages with an\n> invalid Message-Id?\n\nNot quite. git-send-email generates these message-ids itself (those\nwho contain \"-git-send-email-\", that is), and should as such be able\nto rely on them being generated correctly. However, I'm a bit curious\nas to how these ends up incorrect in the first place. The code in\nmake_message_id tries to append the sender's e-mail to\ntimestamp+\"-git-send-email-\", if that fails it tries the comitter's\ne-mail, then the author's e-mail. As a last resort, it tries to append\n\"user@\"+hostname.\n\nI'm no perl-expert, but the code looks pretty much correct to me.\n\nThe problematic e-mails, are they coming from another user than you?\nCan you find out who that is, and check what git-version he or she\nruns?\n\n-- \nErik \"kusma\" Faye-Lund\nkusmabite@gmail.com\n(+47) 986 59 656\n"},{"id":"118937","messageId":"200907281127.44558.trast@student.ethz.ch","threadId":"20253","inReplyTo":"40aa078e0907280217g76cbfai8544edde605f8772@mail.gmail.com","subject":"Re: git-send-email generates mail with invalid Message-Id","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-07-28T09:27:43Z","receivedAt":"2009-07-28T09:27:43Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Erik Faye-Lund wrote:\n> On Tue, Jul 28, 2009 at 4:46 AM, Frans Pop wrote:\n> > I assume that this is a configuration issue in the git setup of the\n> > sender, but shouldn't git-send-email refuse to send out messages with an\n> > invalid Message-Id?\n> \n> Not quite. git-send-email generates these message-ids itself (those\n> who contain \"-git-send-email-\", that is), and should as such be able\n> to rely on them being generated correctly. [...]\n> I'm no perl-expert, but the code looks pretty much correct to me.\n\ngit-format-patch generates its own message IDs if it needs them for\nthreading, with gen_message_id() (in builtin-log.c). That one appends\nthe committer email address blindly, without verifying that it has an\n@ in it.\n\nBlame the committer's broken config, I guess. The untested patch at\nthe end might catch this, but then it's still a fair ways from correct\naddress verification _and_ email addresses aren't required to have a\nhostname part.\n\ndiff --git i/builtin-log.c w/builtin-log.c\nindex fe8e4e1..7003784 100644\n--- i/builtin-log.c\n+++ w/builtin-log.c\n@@ -604,9 +604,12 @@ static void gen_message_id(struct rev_info *info, char *base)\n \tconst char *committer = git_committer_info(IDENT_WARN_ON_NO_NAME);\n \tconst char *email_start = strrchr(committer, '<');\n \tconst char *email_end = strrchr(committer, '>');\n+\tconst char *email_at = strrchr(committer, '@');\n \tstruct strbuf buf = STRBUF_INIT;\n \tif (!email_start || !email_end || email_start > email_end - 1)\n \t\tdie(\"Could not extract email from committer identity.\");\n+\tif (!email_at || email_start > email_at - 1 || email_at > email_end - 1)\n+\t\tdie (\"Committer email address invalid, cannot form message-id\");\n \tstrbuf_addf(&buf, \"%s.%lu.git.%.*s\", base,\n \t\t (unsigned long) time(NULL),\n \t\t (int)(email_end - email_start - 1), email_start + 1);\n"},{"id":"118939","messageId":"40aa078e0907280251k159d9a93xa8c90413b3fab5a@mail.gmail.com","threadId":"20253","inReplyTo":"200907281127.44558.trast@student.ethz.ch","subject":"Re: git-send-email generates mail with invalid Message-Id","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-07-28T09:51:01Z","receivedAt":"2009-07-28T09:51:01Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Jul 28, 2009 at 11:27 AM, Thomas Rast wrote:\n> Erik Faye-Lund wrote:\n>> On Tue, Jul 28, 2009 at 4:46 AM, Frans Pop wrote:\n>> > I assume that this is a configuration issue in the git setup of the\n>> > sender, but shouldn't git-send-email refuse to send out messages with an\n>> > invalid Message-Id?\n>>\n>> Not quite. git-send-email generates these message-ids itself (those\n>> who contain \"-git-send-email-\", that is), and should as such be able\n>> to rely on them being generated correctly. [...]\n>> I'm no perl-expert, but the code looks pretty much correct to me.\n>\n> git-format-patch generates its own message IDs if it needs them for\n> threading, with gen_message_id() (in builtin-log.c).  That one appends\n> the committer email address blindly, without verifying that it has an\n> @ in it.\n\nThat must be a separate issue (but quite possibly a valid one), since\ngen_message_id's ids dont' contain \"-send-email-\" like the message-id\nin the report does, no?\n\n-- \nErik \"kusma\" Faye-Lund\nkusmabite@gmail.com\n(+47) 986 59 656\n"},{"id":"118940","messageId":"200907281203.49732.trast@student.ethz.ch","threadId":"20253","inReplyTo":"40aa078e0907280251k159d9a93xa8c90413b3fab5a@mail.gmail.com","subject":"Re: git-send-email generates mail with invalid Message-Id","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-07-28T10:03:48Z","receivedAt":"2009-07-28T10:03:48Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Erik Faye-Lund wrote:\n> On Tue, Jul 28, 2009 at 11:27 AM, Thomas Rast wrote:\n> > git-format-patch generates its own message IDs if it needs them for\n> > threading, with gen_message_id() (in builtin-log.c). That one appends\n> > the committer email address blindly, without verifying that it has an\n> > @ in it.\n> \n> That must be a separate issue (but quite possibly a valid one), since\n> gen_message_id's ids dont' contain \"-send-email-\" like the message-id\n> in the report does, no?\n\nAh, true of course.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"118941","messageId":"200907281214.34362.elendil@planet.nl","threadId":"20253","inReplyTo":"40aa078e0907280217g76cbfai8544edde605f8772@mail.gmail.com","subject":"Re: git-send-email generates mail with invalid Message-Id","fromName":"Frans Pop","fromEmail":"elendil@planet.nl","sentAt":"2009-07-28T10:14:33Z","receivedAt":"2009-07-28T10:14:33Z","isPatch":false,"sender":{"key":"elendil@planet.nl","avatar":null},"body":"On Tuesday 28 July 2009, Erik Faye-Lund wrote:\n> The problematic e-mails, are they coming from another user than you?\n> Can you find out who that is, and check what git-version he or she\n> runs?\n\nThe Message-Id in my mail was an actual example. Luckily a search for it \ngave the following link: http://patchwork.kernel.org/patch/37597/\n\nSee the link for the name of the user. The headers have:\n X-Mailer: git-send-email 1.5.2.5\n\nCheers,\nFJP\n"},{"id":"118942","messageId":"200907281226.22122.elendil@planet.nl","threadId":"20253","inReplyTo":"200907281214.34362.elendil@planet.nl","subject":"Re: git-send-email generates mail with invalid Message-Id","fromName":"Frans Pop","fromEmail":"elendil@planet.nl","sentAt":"2009-07-28T10:26:21Z","receivedAt":"2009-07-28T10:26:21Z","isPatch":false,"sender":{"key":"elendil@planet.nl","avatar":null},"body":"On Tuesday 28 July 2009, Frans Pop wrote:\n> On Tuesday 28 July 2009, Erik Faye-Lund wrote:\n> > The problematic e-mails, are they coming from another user than you?\n> > Can you find out who that is, and check what git-version he or she\n> > runs?\n>\n> The Message-Id in my mail was an actual example. Luckily a search for\n> it gave the following link: http://patchwork.kernel.org/patch/37597/\n>\n> See the link for the name of the user. The headers have:\n> X-Mailer: git-send-email 1.5.2.5\n\nAnd here are two variants of the full thread containing that patch:\nhttp://lkml.org/lkml/2009/7/27/274\nhttp://groups.google.com/group/linux.kernel/browse_thread/thread/49acfc484d261758\n\nIn the second the original messages (incl. headers) can be seen, but you \nhave to look for X-Original-... because of Google mail2news conversion.\n"},{"id":"118943","messageId":"20090728104423.GA12947@vidovic","threadId":"20253","inReplyTo":"40aa078e0907280217g76cbfai8544edde605f8772@mail.gmail.com","subject":"Re: git-send-email generates mail with invalid Message-Id","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-07-28T10:44:23Z","receivedAt":"2009-07-28T10:44:23Z","isPatch":false,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"Erik Faye-Lund wrote:\n> On Tue, Jul 28, 2009 at 4:46 AM, Frans Pop wrote:\n>\n> > I assume that this is a configuration issue in the git setup of the\n> > sender, but shouldn't git-send-email refuse to send out messages with an\n> > invalid Message-Id?\n\nStricly speacking, it is not an invalid Message-Id. RFC 2822 says that\nthe Message-Id has to be unique. The right hand side may not contain a\ndomain identifier. It is a RECOMMENDED practice (a good one, though).\n\nIMHO, inn2 does a wrong assumption.\n\n> Not quite. git-send-email generates these message-ids itself (those\n> who contain \"-git-send-email-\", that is), and should as such be able\n> to rely on them being generated correctly. [...]\n> I'm no perl-expert, but the code looks pretty much correct to me.\n\nLooks good here too. That said, if $du_part is still empty after all the\nstuff over, we could add a fake domain name. This prevent the Message-Id\nfrom ending with \"-git-send-email->\".\n\n---\nThis is untested.\n\n git-send-email.perl | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex d508f83..82fb3b9 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -748,6 +748,9 @@ sub make_message_id\n use Sys::Hostname qw();\n $du_part = 'user@' . Sys::Hostname::hostname();\n }\n+ if (not defined $du_part or $du_part eq '') {\n+ $du_part = 'git@fake.dom';\n+ }\n my $message_id_template = \"<%s-git-send-email-%s>\";\n $message_id = sprintf($message_id_template, $uniq, $du_part);\n #print \"new message id = $message_id\\n\"; # Was useful for debugging\n-- \nNicolas Sebrecht\n"},{"id":"118947","messageId":"200907281313.51304.elendil@planet.nl","threadId":"20253","inReplyTo":"20090728104423.GA12947@vidovic","subject":"Re: git-send-email generates mail with invalid Message-Id","fromName":"Frans Pop","fromEmail":"elendil@planet.nl","sentAt":"2009-07-28T11:13:50Z","receivedAt":"2009-07-28T11:13:50Z","isPatch":false,"sender":{"key":"elendil@planet.nl","avatar":null},"body":"On Tuesday 28 July 2009, you wrote:\n> Erik Faye-Lund wrote:\n> > On Tue, Jul 28, 2009 at 4:46 AM, Frans Pop wrote:\n> > > I assume that this is a configuration issue in the git setup of the\n> > > sender, but shouldn't git-send-email refuse to send out messages\n> > > with an invalid Message-Id?\n>\n> Stricly speacking, it is not an invalid Message-Id. RFC 2822 says that\n> the Message-Id has to be unique. The right hand side may not contain a\n> domain identifier. It is a RECOMMENDED practice (a good one, though).\n\nIt also says that (3.6.4):\n The message identifier (msg-id) is similar in syntax to an angle-addr\n construct without the internal CFWS.\n\nAnd defines:\n message-id = \"Message-ID:\" msg-id CRLF\n msg-id = [CFWS] \"<\" id-left \"@\" id-right \">\" [CFWS]\n\nSo, the domain part *is* required (or at least: there has to be a \"@\"; it \nmaybe does not require id-right to be an actual domain, but that's not \nreally relevant here).\n\nSo, IMO inn2's check is correct.\n"},{"id":"118949","messageId":"20090728113814.GB12947@vidovic","threadId":"20253","inReplyTo":"200907281313.51304.elendil@planet.nl","subject":"Re: git-send-email generates mail with invalid Message-Id","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-07-28T11:38:14Z","receivedAt":"2009-07-28T11:38:14Z","isPatch":false,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"The 28/07/09, Frans Pop wrote:\n> On Tuesday 28 July 2009, you wrote:\n> > Erik Faye-Lund wrote:\n> >\n> > Stricly speacking, it is not an invalid Message-Id. RFC 2822 says that\n> > the Message-Id has to be unique. The right hand side may not contain a\n> > domain identifier. It is a RECOMMENDED practice (a good one, though).\n> \n> It also says that (3.6.4):\n> The message identifier (msg-id) is similar in syntax to an angle-addr\n> construct without the internal CFWS.\n> \n> And defines:\n> message-id = \"Message-ID:\" msg-id CRLF\n> msg-id = [CFWS] \"<\" id-left \"@\" id-right \">\" [CFWS]\n> \n> So, the domain part *is* required (or at least: there has to be a \"@\"; it \n> maybe does not require id-right to be an actual domain, but that's not \n> really relevant here).\n> \n> So, IMO inn2's check is correct.\n\nHum, you're right. The '@' symbol is required, whatever \"id-right\" is.\nMy previous patch should fix it.\n\n-- \nNicolas Sebrecht\n"},{"id":"118950","messageId":"40aa078e0907280447p4ed92133jb5e586fb0ca40ef2@mail.gmail.com","threadId":"20253","inReplyTo":"20090728113814.GB12947@vidovic","subject":"Re: git-send-email generates mail with invalid Message-Id","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-07-28T11:47:11Z","receivedAt":"2009-07-28T11:47:11Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Jul 28, 2009 at 1:38 PM, Nicolas Sebrecht wrote:\n> Hum, you're right. The '@' symbol is required, whatever \"id-right\" is.\n> My previous patch should fix it.\n\nWith all due respect, I don't see how that patch fixes anything. The\nprevious last-resort solution should already be just as valid, it\nassigns 'user@'+hostname to $du_part. Even if hostname is \"\" it should\ninsert an '@', which didn't happen here.\n\nI'm suspecting that git-send-email in v1.5.2.5 didn't do enough\nchecks, and that this is an already-solved issue. Looking at the\nsource code from v1.5.2.5 seems to confirm this.\nhttp://repo.or.cz/w/git.git?a=blob;f=git-send-email.perl;h=7c0c90bd21bbb009de81aa315bed1c947a32c423;hb=b13ef4916ac5a25cc5897f85ba0b4c5953cff609\n\nmy $message_id_from = extract_valid_address($from);\nmy $message_id_template = \"<%s-git-send-email-$message_id_from>\";\n\nsub make_message_id\n{\n\tmy $date = time;\n\tmy $pseudo_rand = int (rand(4200));\n\t$message_id = sprintf $message_id_template, \"$date$pseudo_rand\";\n\t#print \"new message id = $message_id\\n\"; # Was useful for debugging\n}\n\nSo I think it's pretty safe to disregard this as an already solved issue.\n\n-- \nErik \"kusma\" Faye-Lund\nkusmabite@gmail.com\n(+47) 986 59 656\n"},{"id":"118952","messageId":"40aa078e0907280510s1afee3ddw3a9333620a3c7d7a@mail.gmail.com","threadId":"20253","inReplyTo":"40aa078e0907280447p4ed92133jb5e586fb0ca40ef2@mail.gmail.com","subject":"Re: git-send-email generates mail with invalid Message-Id","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-07-28T12:10:03Z","receivedAt":"2009-07-28T12:10:03Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Jul 28, 2009 at 1:47 PM, Erik Faye-Lund wrote:\n> On Tue, Jul 28, 2009 at 1:38 PM, Nicolas Sebrecht wrote:\n>> Hum, you're right. The '@' symbol is required, whatever \"id-right\" is.\n>> My previous patch should fix it.\n>\n> With all due respect, I don't see how that patch fixes anything. The\n> previous last-resort solution should already be just as valid, it\n> assigns 'user@'+hostname to $du_part. Even if hostname is \"\" it should\n> insert an '@', which didn't happen here.\n\nHere's an attempt to fix the case when Sys::Hostname::hostname returns\n\"\" (domains aren't allowed to be empty if I read RFC2822 correctly).\nThe problem with the previous attempt was that the earlier if assigned\n\"user@\" to $du_part, so the last if was never entered ($du_part was\nalways defined).\n\nI generally don't write Perl, so people will most likely barf all over\nthis one, but at least it should show the concept. It might not even\nwork.\n\nI also suspect that it is not needed.\nhttp://search.cpan.org/~tty/kurila-1.19_0/ext/Sys-Hostname/Hostname.pm\nseems to indicate that it either returns something sensible or dies.\n\n---\nUntested.\n\n git-send-email.perl | 6 +++++-\n 1 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 303e03a..baadbdb 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -742,7 +742,11 @@ sub make_message_id\n }\n if (not defined $du_part or $du_part eq '') {\n use Sys::Hostname qw();\n- $du_part = 'user@' . Sys::Hostname::hostname();\n+ my $domain = Sys::Hostname::hostname();\n+ if (not defined $domain or $domain eq '') {\n+ $domain = 'fake.dom';\n+ }\n+ $du_part = \"user@$domain\";\n }\n my $message_id_template = \"<%s-git-send-email-%s>\";\n $message_id = sprintf($message_id_template, $uniq, $du_part);\n-- \nErik \"kusma\" Faye-Lund\nkusmabite@gmail.com\n(+47) 986 59 656\n"},{"id":"118966","messageId":"20090728150745.GB16168@vidovic","threadId":"20253","inReplyTo":"40aa078e0907280510s1afee3ddw3a9333620a3c7d7a@mail.gmail.com","subject":"Re: git-send-email generates mail with invalid Message-Id","fromName":"Nicolas Sebrecht","fromEmail":"nicolas.s.dev@gmx.fr","sentAt":"2009-07-28T15:07:45Z","receivedAt":"2009-07-28T15:07:45Z","isPatch":false,"sender":{"key":"nicolas.s.dev@gmx.fr","avatar":null},"body":"The 28/07/09, Erik Faye-Lund wrote:\n> \n> Here's an attempt to fix the case when Sys::Hostname::hostname returns\n> \"\" (domains aren't allowed to be empty if I read RFC2822 correctly).\n>\n> The problem with the previous attempt was that the earlier if assigned\n> \"user@\" to $du_part, so the last if was never entered ($du_part was\n> always defined).\n\nYes, thank you.\n\n> I generally don't write Perl, so people will most likely barf all over\n> this one, but at least it should show the concept. It might not even\n> work.\n\nLooks ok here.\n\n> I also suspect that it is not needed.\n\nI'm not sure because http://linux.die.net/man/2/gethostname does not\ntell either (and POSIX neither).\n\nThat said, I tend to think it worth to merge this fix before having a\nbug report.\n\n-- \nNicolas Sebrecht\n"}]}