git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [bug] blame duplicates trailing ">" in mailmapped emails

From
Felipe Contreras <felipe.contreras@gmail.com>
Date
Feb 4, 2012, 19:30 UTC
Message-ID
<CAMP44s2r-fcnfpdT4u5U7TwF1z6Abp+J1U7oqfsSrYMuD6weOQ@mail.gmail.com>
In-Reply-To
<20120204182611.GA31091@sigill.intra.peff.net>
On Sat, Feb 4, 2012 at 8:26 PM, Jeff King <peff@peff.net> wrote:
Show 9 quoted lines
> On Sat, Feb 04, 2012 at 05:46:05PM +0200, Felipe Contreras wrote:
>
>> In any case, the one to blame for the header corruption is git:
>> [...]
>> f2bb9f88 (<spearce@spearce.org>> 2006-11-27 03:41:01 -0500 952)
>>
>> Notice the mail is wrong.
>
> Ugh. The fault lies in this code:
Yes, I found that as well.
Show 24 quoted lines
> But that comment is wrong. If there's no email mapping needed, map_user
> will leave the "mail" buffer intact, in which case it will have the
> trailing ">" (because we feed the address with enclosing angle
> brackets).  So while map_user tries to accept either "foo@example.com\0"
> and "foo@example.com>", it is up to the contents of the mailmap whether
> you get back something with the closing angle bracket or not. Which is a
> pretty error-prone interface.
>
> You can fix it with this:
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 5a67c20..9b886fa 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1406,7 +1406,8 @@ static void get_ac_line(const char *inbuf, const char *what,
>                /* Add a trailing '>' to email, since map_user returns plain emails
>                   Note: It already has '<', since we replace from mail+1 */
>                mailpos = memchr(mail, '\0', mail_len);
> -               if (mailpos && mailpos-mail < mail_len - 1) {
> +               if (mailpos && mailpos-mail < mail_len - 1 &&
> +                   mailpos > mail && *(mailpos-1) != '>') {
>                        *mailpos = '>';
>                        *(mailpos+1) = '\0';
>                }

Yes, I already did this. I'm writing tests for this right now, but I think I found yet another bug...

Show 5 quoted lines
> but it feels like the fix should go into map_user.  I tried a few things,
> like "git log -1 --format=%aE", and couldn't find other code paths with
> this problem. So presumably they are all feeding email addresses without
> the closing ">" (so one option is to just say "map_user needs to get
> NUL-terminated strings).

Perhaps, but I though the idea was to make it efficient. I think the above fix should be ok.

We should have tests for this though, to make sure it doesn't get broken again. I'm on that.

Cheers.
-- 
Felipe Contreras
Previous: Jeff KingNext: Jeff King
Message 15 of 38 in “completion: couple of cleanups”
  1. 0/4 completion: couple of cleanupsFelipe Contreras, Feb 2, 2012
  2. Jonathan NiederFeb 2, 2012
  3. Felipe ContrerasFeb 2, 2012
  4. Thomas RastFeb 2, 2012
  5. Jonathan NiederFeb 2, 2012
  6. Jonathan NiederFeb 2, 2012
  7. Felipe ContrerasFeb 2, 2012
  8. Junio C HamanoFeb 2, 2012
  9. Felipe ContrerasFeb 2, 2012
  10. Junio C HamanoFeb 3, 2012
  11. Felipe ContrerasFeb 3, 2012
  12. Junio C HamanoFeb 3, 2012
  13. Felipe ContrerasFeb 4, 2012
  14. [bug] blame duplicates trailing ">" in mailmapped emailsJeff King, Feb 4, 2012
  15. Felipe ContrerasFeb 4, 2012
  16. Jeff KingFeb 4, 2012
  17. Felipe ContrerasFeb 5, 2012
  18. Jeff KingFeb 5, 2012
  19. Junio C HamanoFeb 5, 2012
  20. Junio C HamanoFeb 5, 2012
  21. Jeff KingFeb 5, 2012
  22. Junio C HamanoFeb 6, 2012
  23. Jeff KingFeb 6, 2012
  24. Junio C HamanoFeb 6, 2012
  25. Junio C HamanoFeb 6, 2012
  26. Jeff KingFeb 6, 2012
  27. Felipe ContrerasFeb 6, 2012
  28. Junio C HamanoFeb 6, 2012
  29. Felipe ContrerasFeb 6, 2012
  30. Junio C HamanoFeb 5, 2012
  31. Jonathan NiederFeb 2, 2012
  32. Junio C HamanoFeb 2, 2012
  33. Jonathan NiederFeb 2, 2012
  34. Felipe ContrerasFeb 2, 2012
  35. Jonathan NiederFeb 2, 2012
  36. Felipe ContrerasFeb 2, 2012
  37. Jonathan NiederFeb 2, 2012
  38. Felipe ContrerasFeb 2, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.