{"thread":{"id":"63621","subject":"[PATCH] cat-file: fix mailmap application for different author and committer","startedAt":"2025-06-11T06:27:11Z","lastAt":"2025-06-13T15:57:02Z","messageCount":10,"participants":["siddharthasthana31@gmail.com","Christian Couder","Junio C Hamano","Eric Sunshine","Siddharth Asthana"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"520075","messageId":"20250611062643.8639-1-siddharthasthana31@gmail.com","threadId":"63621","inReplyTo":null,"subject":"[PATCH] cat-file: fix mailmap application for different author and committer","fromName":"","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2025-06-11T06:26:43Z","receivedAt":"2025-06-11T06:27:11Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"From: Siddharth Asthana <siddharthasthana31@gmail.com>\n\nThe git cat-file command with --mailmap option fails to apply mailmap\ntransformations to the committer field when the author and committer\nidentities are different. This occurs due to a missing newline handling\nin apply_mailmap_to_header() after processing each identity line.\n\nWhen rewrite_ident_line() processes an identity, it stops at the end\nof the identity data (e.g., \"Author Name <email> timestamp\"), but\ndoesn't account for the trailing newline. The current code adds the\nidentity length to buf_offset but fails to advance past the newline\ncharacter. This causes the next iteration to start parsing from the\nnewline instead of the beginning of the next header line, making it\nimpossible to match subsequent headers like \"committer\".\n\nAdditionally, rewrite_ident_line() may reallocate the buffer during\nits operation. Any code using pointers into the old buffer would be\nusing invalid memory after such a reallocation.\n\nLet's fix this by addressing both issues:\n1. After processing an identity line, we now check if we're at a\n   newline and advance past it, ensuring the next header line is\n   parsed correctly.\n2. We recompute the buffer position after rewrite_ident_line() to\n   handle potential buffer reallocation.\n\nThis ensures that all identity headers in commit and tag objects are\nconsistently processed regardless of whether the author and committer\nare the same person.\n\nReported-by: Vasilii Iakliushin <viakliushin@gitlab.com>\nReviewed-by: Christian Couder <christian.couder@gmail.com>\nSigned-off-by: Siddharth Asthana <siddharthasthana31@gmail.com>\n---\n ident.c            |  4 ++++\n t/t4203-mailmap.sh | 33 +++++++++++++++++++++++++++++++++\n 2 files changed, 37 insertions(+)\n\ndiff --git a/ident.c b/ident.c\nindex 967895d885..281e830573 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -412,6 +412,10 @@ void apply_mailmap_to_header(struct strbuf *buf, const char **header,\n \t\t\t\tfound_header = 1;\n \t\t\t\tbuf_offset += endp - line;\n \t\t\t\tbuf_offset += rewrite_ident_line(person, endp - person, buf, mailmap);\n+\t\t\t\t/* Recompute endp after potential buffer reallocation */\n+\t\t\t\tendp = buf->buf + buf_offset;\n+\t\t\t\tif (*endp == '\\n')\n+\t\t\t\t\tbuf_offset++;\n \t\t\t\tbreak;\n \t\t\t}\n \ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex 4a6242ff99..98dd0ae12f 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -1133,4 +1133,37 @@ test_expect_success 'git cat-file --batch-command returns correct size with --us\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git cat-file --mailmap works with different author and committer' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tMailmapped User <mailmapped-user@gitlab.com> C O Mitter <committer@example.com>\n+\tEOF\n+\tgit commit --allow-empty -m \"different author/committer\" \\\n+\t\t--author=\"Different Author <different@example.com>\" &&\n+\tcat >expect <<-\\EOF &&\n+\tauthor Different Author <different@example.com>\n+\tcommitter Mailmapped User <mailmapped-user@gitlab.com>\n+\tEOF\n+\tgit cat-file --mailmap commit HEAD >log &&\n+\tsed -n \"/^author /s/\\([^>]*>\\).*/\\1/p; /^committer /s/\\([^>]*>\\).*/\\1/p\" log >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --mailmap maps both author and committer when both need mapping' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tMapped Author <mapped-author@example.com> <different@example.com>\n+\tMapped Committer <mapped-committer@example.com> C O Mitter <committer@example.com>\n+\tEOF\n+\tgit commit --allow-empty -m \"both author and committer mapped\" \\\n+\t\t--author=\"Different Author <different@example.com>\" &&\n+\tcat >expect <<-\\EOF &&\n+\tauthor Mapped Author <mapped-author@example.com>\n+\tcommitter Mapped Committer <mapped-committer@example.com>\n+\tEOF\n+\tgit cat-file --mailmap commit HEAD >log &&\n+\tsed -n \"/^author /s/\\([^>]*>\\).*/\\1/p; /^committer /s/\\([^>]*>\\).*/\\1/p\" log >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.49.0\n\n"},{"id":"520082","messageId":"CAP8UFD1hBo_ZH1nyGBOoQOwx=44CfnkhEOHYu3_XMwSzXQAHdg@mail.gmail.com","threadId":"63621","inReplyTo":"20250611062643.8639-1-siddharthasthana31@gmail.com","subject":"Re: [PATCH] cat-file: fix mailmap application for different author and committer","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-06-11T09:38:02Z","receivedAt":"2025-06-11T09:38:15Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Jun 11, 2025 at 8:27 AM <siddharthasthana31@gmail.com> wrote:\n\n> Reported-by: Vasilii Iakliushin <viakliushin@gitlab.com>\n> Reviewed-by: Christian Couder <christian.couder@gmail.com>\n\nNit: I reviewed it when you suggested it on a GitLab MR (Merge\nRequest), but I am not sure it counts unless I also review it here. I\nthink the \"Reviewed-by: ...\" trailer is for patches reviewed on the\nregular Git mailing list (or maybe on the private Git security list).\nSo maybe \"Helped-by: ...\" would have been better in this case.\n\nAnyway I have now reviewed it again and I found it great.\n\nThanks for working on this!\n\n> Signed-off-by: Siddharth Asthana <siddharthasthana31@gmail.com>\n> ---\n>  ident.c            |  4 ++++\n>  t/t4203-mailmap.sh | 33 +++++++++++++++++++++++++++++++++\n>  2 files changed, 37 insertions(+)\n>\n> diff --git a/ident.c b/ident.c\n> index 967895d885..281e830573 100644\n> --- a/ident.c\n> +++ b/ident.c\n> @@ -412,6 +412,10 @@ void apply_mailmap_to_header(struct strbuf *buf, const char **header,\n>                                 found_header = 1;\n>                                 buf_offset += endp - line;\n>                                 buf_offset += rewrite_ident_line(person, endp - person, buf, mailmap);\n> +                               /* Recompute endp after potential buffer reallocation */\n> +                               endp = buf->buf + buf_offset;\n> +                               if (*endp == '\\n')\n> +                                       buf_offset++;\n\nYeah, without this, in the next iteration of the `for (;;) { ... }`\nloop after the \"author\" header has been found, we have:\n\n        line = buf->buf + buf_offset;\n\nwhich sets `line` to something like \"\\ncommitter C O Mitter\n<committer@example.com> ...\", and then:\n\n        if (!*line || *line == '\\n')\n            return; /* End of headers */\n\nwhich just returns as `*line` is indeed '\\n'.\n\n>                                 break;\n>                         }\n"},{"id":"520109","messageId":"xmqqy0tyi8aj.fsf@gitster.g","threadId":"63621","inReplyTo":"20250611062643.8639-1-siddharthasthana31@gmail.com","subject":"Re: [PATCH] cat-file: fix mailmap application for different author and committer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-11T15:53:40Z","receivedAt":"2025-06-11T15:53:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"siddharthasthana31@gmail.com writes:\n\n> From: Siddharth Asthana <siddharthasthana31@gmail.com>\n>\n> The git cat-file command with --mailmap option fails to apply mailmap\n> transformations to the committer field when the author and committer\n> identities are different. This occurs due to a missing newline handling\n> in apply_mailmap_to_header() after processing each identity line.\n> ...\n> This ensures that all identity headers in commit and tag objects are\n> consistently processed regardless of whether the author and committer\n> are the same person.\n\nNicely described.\n\nWhile the above explains what is wrong in the current code, it does\nnot tell if that was buggy from the beginning or we unintentionally\nbroke it.  It seems that this logic came from e9c1b0e3 (revision:\nimprove commit_rewrite_person(), 2022-07-19) when a much simpler\nversion of commit_rewrite_person() that worked on one \"person\nheader\" at a time (as there are only two, author and committer,\nanyway) was rewritten to use this function, and it was broken during\nthe rewrite?\n\n> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\n> index 4a6242ff99..98dd0ae12f 100755\n> --- a/t/t4203-mailmap.sh\n> +++ b/t/t4203-mailmap.sh\n> @@ -1133,4 +1133,37 @@ test_expect_success 'git cat-file --batch-command returns correct size with --us\n>  \ttest_cmp expect actual\n>  '\n>  \n> +test_expect_success 'git cat-file --mailmap works with different author and committer' '\n> +\ttest_when_finished \"rm .mailmap\" &&\n> +\tcat >.mailmap <<-\\EOF &&\n> +\tMailmapped User <mailmapped-user@gitlab.com> C O Mitter <committer@example.com>\n> +\tEOF\n> +\tgit commit --allow-empty -m \"different author/committer\" \\\n> +\t\t--author=\"Different Author <different@example.com>\" &&\n> +\tcat >expect <<-\\EOF &&\n> +\tauthor Different Author <different@example.com>\n> +\tcommitter Mailmapped User <mailmapped-user@gitlab.com>\n> +\tEOF\n> +\tgit cat-file --mailmap commit HEAD >log &&\n> +\tsed -n \"/^author /s/\\([^>]*>\\).*/\\1/p; /^committer /s/\\([^>]*>\\).*/\\1/p\" log >actual &&\n\nPerhaps just a  matter of taste, but\n\n\tsed -n -e \"/^author /s/>.*/>/p\" -e \"/^committer /s/>.*/>/p\"\n\nmay be easier to read and more portable (as some implementation of\nsed is picky about semicolon concatenated multiple commands).\n\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'git cat-file --mailmap maps both author and committer when both need mapping' '\n> +\ttest_when_finished \"rm .mailmap\" &&\n> +\tcat >.mailmap <<-\\EOF &&\n> +\tMapped Author <mapped-author@example.com> <different@example.com>\n> +\tMapped Committer <mapped-committer@example.com> C O Mitter <committer@example.com>\n> +\tEOF\n> +\tgit commit --allow-empty -m \"both author and committer mapped\" \\\n> +\t\t--author=\"Different Author <different@example.com>\" &&\n> +\tcat >expect <<-\\EOF &&\n> +\tauthor Mapped Author <mapped-author@example.com>\n> +\tcommitter Mapped Committer <mapped-committer@example.com>\n> +\tEOF\n> +\tgit cat-file --mailmap commit HEAD >log &&\n> +\tsed -n \"/^author /s/\\([^>]*>\\).*/\\1/p; /^committer /s/\\([^>]*>\\).*/\\1/p\" log >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n>  test_done\n"},{"id":"520120","messageId":"CAPig+cTezW8XYjNo3L3Qy+f+aiCXppTj-Z=N0bBMG8mp9jJ8ZA@mail.gmail.com","threadId":"63621","inReplyTo":"xmqqy0tyi8aj.fsf@gitster.g","subject":"Re: [PATCH] cat-file: fix mailmap application for different author and committer","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-06-11T19:05:14Z","receivedAt":"2025-06-11T19:05:26Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jun 11, 2025 at 11:55 AM Junio C Hamano <gitster@pobox.com> wrote:\n> From: Siddharth Asthana <siddharthasthana31@gmail.com>\n> > +     sed -n \"/^author /s/\\([^>]*>\\).*/\\1/p; /^committer /s/\\([^>]*>\\).*/\\1/p\" log >actual &&\n>\n> Perhaps just a  matter of taste, but\n>\n>         sed -n -e \"/^author /s/>.*/>/p\" -e \"/^committer /s/>.*/>/p\"\n>\n> may be easier to read and more portable (as some implementation of\n> sed is picky about semicolon concatenated multiple commands).\n\nFor what it's worth, Git test scripts already contain a fair number of\nuses of semicolon-separated `sed` commands, and we haven't heard of\nany problems with them; not even from the very old and quite picky\nSolaris `sed` (or was it the ancient SunOS `sed`?).\n\nThe only case I can think of in which there was a semicolon-related\nproblem (and perhaps what you're thinking of) was when a recent\npatch[*] neglected to insert a semicolon where it was expected. That\nparticular case involved a missing semicolon before a closing brace:\n\n    sed -n '/ version /{p;q}'\n\nwhich should have been:\n\n    sed -n '/ version /{p;q;}'\n\nTo summarize: Using semicolon-separated commands is safe and portable;\nI don't think there is any evidence that doing so would be\nproblematic. Whether to use semicolon-separated commands or multiple\n`-e` arguments is subjective, and I don't believe the project has\nexpressed a preference for one for or the other.\n\n[*]: https://lore.kernel.org/git/CAPig+cR+ESNg4tV1G6jbKKeRKABD053qZcG0BoFuQ7aC+1tGYw@mail.gmail.com/\n"},{"id":"520135","messageId":"xmqq7c1hhmx7.fsf@gitster.g","threadId":"63621","inReplyTo":"CAPig+cTezW8XYjNo3L3Qy+f+aiCXppTj-Z=N0bBMG8mp9jJ8ZA@mail.gmail.com","subject":"Re: [PATCH] cat-file: fix mailmap application for different author and committer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-11T23:35:16Z","receivedAt":"2025-06-11T23:35:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>> may be easier to read and more portable (as some implementation of\n>> sed is picky about semicolon concatenated multiple commands).\n>\n> For what it's worth, Git test scripts already contain a fair number of\n> uses of semicolon-separated `sed` commands, and we haven't heard of\n> any problems with them; not even from the very old and quite picky\n> Solaris `sed` (or was it the ancient SunOS `sed`?).\n\nI think it was of BSD lineage, but I phrased it poorly.\n\n>     sed -n '/ version /{p;q}'\n\nThis pattern did cause issues in the past.  I was hoping that we can\navoid it by training our developers to avoid concatenation with\nsemicolons in general, but {grouped} commands cannot be fed without\nproperly using semicolons anyway, so it would not help to just\ngenerally avoid use of semicolons.\n\nOn the other hand, the suggestion that was given in the message ...\n\n>> > +     sed -n \"/^author /s/\\([^>]*>\\).*/\\1/p; /^committer /s/\\([^>]*>\\).*/\\1/p\" log >actual &&\n>>\n>> Perhaps just a  matter of taste, but\n>>\n>>         sed -n -e \"/^author /s/>.*/>/p\" -e \"/^committer /s/>.*/>/p\"\n\n... is much shorter, simpler and easier to understand.  With the\nadded benefit that you can even line-wrap sensibly\n\n        sed -n -e \"/^author /s/>.*/>/p\" \\\n\t       -e \"/^committer /s/>.*/>/p\"\n\nthere really isn't a good reason not to adopt the style, compared to\nthe way the patch was originally written.\n"},{"id":"520202","messageId":"xmqqecvobjwc.fsf@gitster.g","threadId":"63621","inReplyTo":"CAP8UFD1hBo_ZH1nyGBOoQOwx=44CfnkhEOHYu3_XMwSzXQAHdg@mail.gmail.com","subject":"Re: [PATCH] cat-file: fix mailmap application for different author and committer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-12T23:49:23Z","receivedAt":"2025-06-12T23:49:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> On Wed, Jun 11, 2025 at 8:27 AM <siddharthasthana31@gmail.com> wrote:\n>\n>> Reported-by: Vasilii Iakliushin <viakliushin@gitlab.com>\n>> Reviewed-by: Christian Couder <christian.couder@gmail.com>\n>\n> Nit: I reviewed it when you suggested it on a GitLab MR (Merge\n> Request), but I am not sure it counts unless I also review it here. I\n> think the \"Reviewed-by: ...\" trailer is for patches reviewed on the\n> regular Git mailing list (or maybe on the private Git security list).\n> So maybe \"Helped-by: ...\" would have been better in this case.\n\nIf somebody (including me) sees your reviewed-by on a patch and do\nnot remember they saw your review here, they might ask, but as long\nas you are OK to have your name on the reviewed-by trailer, meaning\nyou have carefully inspected exactly the same version of the patch\nand are willing to stand behind the change, it is perfectly fine.\n\nOn the other hand, if you see somebody attach your reviewed-by to a\npatch that you didn't review, or is substantially different from the\none you reviewed, please raise a stink about it.  I do not think\nthis case is such a case.\n\n> Anyway I have now reviewed it again and I found it great.\n>\n> Thanks for working on this!\n>\n>> Signed-off-by: Siddharth Asthana <siddharthasthana31@gmail.com>\n>> ---\n\nThanks.\n"},{"id":"520214","messageId":"c9c39e01-1244-427b-a496-ec35e43f7636@gmail.com","threadId":"63621","inReplyTo":"xmqqy0tyi8aj.fsf@gitster.g","subject":"Re: [PATCH] cat-file: fix mailmap application for different author and committer","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2025-06-13T09:20:45Z","receivedAt":"2025-06-13T09:20:51Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"\nOn 11/06/25 21:23, Junio C Hamano wrote:\n> siddharthasthana31@gmail.com writes:\n>\n>> From: Siddharth Asthana <siddharthasthana31@gmail.com>\n>>\n>> The git cat-file command with --mailmap option fails to apply mailmap\n>> transformations to the committer field when the author and committer\n>> identities are different. This occurs due to a missing newline handling\n>> in apply_mailmap_to_header() after processing each identity line.\n>> ...\n>> This ensures that all identity headers in commit and tag objects are\n>> consistently processed regardless of whether the author and committer\n>> are the same person.\n\nThanks for the detailed review and suggestions!\n\n> Nicely described.\n>\n> While the above explains what is wrong in the current code, it does\n> not tell if that was buggy from the beginning or we unintentionally\n> broke it.  It seems that this logic came from e9c1b0e3 (revision:\n> improve commit_rewrite_person(), 2022-07-19) when a much simpler\n> version of commit_rewrite_person() that worked on one \"person\n> header\" at a time (as there are only two, author and committer,\n> anyway) was rewritten to use this function, and it was broken during\n> the rewrite?\n\nYou are absolutely right! I should have included this historical context\nin the commit message. The bug was indeed introduced during that rewrite\nin e9c1b0e3. The original implementation processed author and committer\nseparately, but the rewrite introduced the loop-based approach that failed\nto properly handle the transition between identity lines.\n\n>\n>> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\n>> index 4a6242ff99..98dd0ae12f 100755\n>> --- a/t/t4203-mailmap.sh\n>> +++ b/t/t4203-mailmap.sh\n>> @@ -1133,4 +1133,37 @@ test_expect_success 'git cat-file --batch-command returns correct size with --us\n>>   \ttest_cmp expect actual\n>>   '\n>>   \n>> +test_expect_success 'git cat-file --mailmap works with different author and committer' '\n>> +\ttest_when_finished \"rm .mailmap\" &&\n>> +\tcat >.mailmap <<-\\EOF &&\n>> +\tMailmapped User <mailmapped-user@gitlab.com> C O Mitter <committer@example.com>\n>> +\tEOF\n>> +\tgit commit --allow-empty -m \"different author/committer\" \\\n>> +\t\t--author=\"Different Author <different@example.com>\" &&\n>> +\tcat >expect <<-\\EOF &&\n>> +\tauthor Different Author <different@example.com>\n>> +\tcommitter Mailmapped User <mailmapped-user@gitlab.com>\n>> +\tEOF\n>> +\tgit cat-file --mailmap commit HEAD >log &&\n>> +\tsed -n \"/^author /s/\\([^>]*>\\).*/\\1/p; /^committer /s/\\([^>]*>\\).*/\\1/p\" log >actual &&\n> Perhaps just a  matter of taste, but\n>\n> \tsed -n -e \"/^author /s/>.*/>/p\" -e \"/^committer /s/>.*/>/p\"\n>\n> may be easier to read and more portable (as some implementation of\n> sed is picky about semicolon concatenated multiple commands).\n\nGood point about portability and readability. The `-e` flag approach\nis indeed cleaner and more maintainable. I'll update both test cases\nto use this format.\n\nI will send a v2 with:\n1. Updated commit message explaining the historical context from e9c1b0e3\n2. Improved sed commands using the `-e` flag format\n\nThanks\n\n>\n>> +\ttest_cmp expect actual\n>> +'\n>> +\n>> +test_expect_success 'git cat-file --mailmap maps both author and committer when both need mapping' '\n>> +\ttest_when_finished \"rm .mailmap\" &&\n>> +\tcat >.mailmap <<-\\EOF &&\n>> +\tMapped Author <mapped-author@example.com> <different@example.com>\n>> +\tMapped Committer <mapped-committer@example.com> C O Mitter <committer@example.com>\n>> +\tEOF\n>> +\tgit commit --allow-empty -m \"both author and committer mapped\" \\\n>> +\t\t--author=\"Different Author <different@example.com>\" &&\n>> +\tcat >expect <<-\\EOF &&\n>> +\tauthor Mapped Author <mapped-author@example.com>\n>> +\tcommitter Mapped Committer <mapped-committer@example.com>\n>> +\tEOF\n>> +\tgit cat-file --mailmap commit HEAD >log &&\n>> +\tsed -n \"/^author /s/\\([^>]*>\\).*/\\1/p; /^committer /s/\\([^>]*>\\).*/\\1/p\" log >actual &&\n>> +\ttest_cmp expect actual\n>> +'\n>> +\n>>   test_done\n"},{"id":"520219","messageId":"20250613115750.41205-1-siddharthasthana31@gmail.com","threadId":"63621","inReplyTo":"20250611062643.8639-1-siddharthasthana31@gmail.com","subject":"[PATCH v2] cat-file: fix mailmap application for different author and committer","fromName":"","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2025-06-13T11:57:17Z","receivedAt":"2025-06-13T11:58:10Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"From: Siddharth Asthana <siddharthasthana31@gmail.com>\n\nThe git cat-file command with --mailmap option fails to apply mailmap\ntransformations to the committer field when the author and committer\nidentities are different. This occurs due to a missing newline handling\nin apply_mailmap_to_header() after processing each identity line.\n\nWhen rewrite_ident_line() processes an identity, it stops at the end\nof the identity data (e.g., \"Author Name <email> timestamp\"), but\ndoesn't account for the trailing newline. The current code adds the\nidentity length to buf_offset but fails to advance past the newline\ncharacter. This causes the next iteration to start parsing from the\nnewline instead of the beginning of the next header line, making it\nimpossible to match subsequent headers like \"committer\".\n\nAdditionally, rewrite_ident_line() may reallocate the buffer during\nits operation. Any code using pointers into the old buffer would be\nusing invalid memory after such a reallocation.\n\nThis bug was introduced in e9c1b0e3 (revision: improve\ncommit_rewrite_person(), 2022-07-19) when the much simpler version of\ncommit_rewrite_person() that worked on one \"person header\" at a time\nwas rewritten to use the current apply_mailmap_to_header() function.\nThe original implementation processed author and committer separately,\nbut the rewrite introduced this loop-based approach that failed to\nproperly handle the transition between identity lines.\n\nLet's fix this by addressing both issues:\n1. After processing an identity line, we now check if we're at a\n   newline and advance past it, ensuring the next header line is\n   parsed correctly.\n2. We recompute the buffer position after rewrite_ident_line() to\n   handle potential buffer reallocation.\n\nThis ensures that all identity headers in commit and tag objects are\nconsistently processed regardless of whether the author and committer\nare the same person.\n\nReported-by: Vasilii Iakliushin <viakliushin@gitlab.com>\nReviewed-by: Christian Couder <christian.couder@gmail.com>\nSigned-off-by: Siddharth Asthana <siddharthasthana31@gmail.com>\n---\n ident.c            |  4 ++++\n t/t4203-mailmap.sh | 33 +++++++++++++++++++++++++++++++++\n 2 files changed, 37 insertions(+)\n\ndiff --git a/ident.c b/ident.c\nindex 967895d885..281e830573 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -412,6 +412,10 @@ void apply_mailmap_to_header(struct strbuf *buf, const char **header,\n \t\t\t\tfound_header = 1;\n \t\t\t\tbuf_offset += endp - line;\n \t\t\t\tbuf_offset += rewrite_ident_line(person, endp - person, buf, mailmap);\n+\t\t\t\t/* Recompute endp after potential buffer reallocation */\n+\t\t\t\tendp = buf->buf + buf_offset;\n+\t\t\t\tif (*endp == '\\n')\n+\t\t\t\t\tbuf_offset++;\n \t\t\t\tbreak;\n \t\t\t}\n \ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex 4a6242ff99..74b7ddccb2 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -1133,4 +1133,37 @@ test_expect_success 'git cat-file --batch-command returns correct size with --us\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git cat-file --mailmap works with different author and committer' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tMailmapped User <mailmapped-user@gitlab.com> C O Mitter <committer@example.com>\n+\tEOF\n+\tgit commit --allow-empty -m \"different author/committer\" \\\n+\t\t--author=\"Different Author <different@example.com>\" &&\n+\tcat >expect <<-\\EOF &&\n+\tauthor Different Author <different@example.com>\n+\tcommitter Mailmapped User <mailmapped-user@gitlab.com>\n+\tEOF\n+\tgit cat-file --mailmap commit HEAD >log &&\n+\tsed -n -e \"/^author /s/>.*/>/p\" -e \"/^committer /s/>.*/>/p\" log >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --mailmap maps both author and committer when both need mapping' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tMapped Author <mapped-author@example.com> <different@example.com>\n+\tMapped Committer <mapped-committer@example.com> C O Mitter <committer@example.com>\n+\tEOF\n+\tgit commit --allow-empty -m \"both author and committer mapped\" \\\n+\t\t--author=\"Different Author <different@example.com>\" &&\n+\tcat >expect <<-\\EOF &&\n+\tauthor Mapped Author <mapped-author@example.com>\n+\tcommitter Mapped Committer <mapped-committer@example.com>\n+\tEOF\n+\tgit cat-file --mailmap commit HEAD >log &&\n+\tsed -n -e \"/^author /s/>.*/>/p\" -e \"/^committer /s/>.*/>/p\" log >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.49.0\n\n"},{"id":"520223","messageId":"CAP8UFD37rFvhO_XLhSjZNoOQ_ePwrrALYBcqKHfhMKcpqhkP8Q@mail.gmail.com","threadId":"63621","inReplyTo":"20250613115750.41205-1-siddharthasthana31@gmail.com","subject":"Re: [PATCH v2] cat-file: fix mailmap application for different author and committer","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-06-13T12:59:52Z","receivedAt":"2025-06-13T13:00:06Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Jun 13, 2025 at 1:58 PM <siddharthasthana31@gmail.com> wrote:\n\n[...]\n\n> This bug was introduced in e9c1b0e3 (revision: improve\n> commit_rewrite_person(), 2022-07-19) when the much simpler version of\n> commit_rewrite_person() that worked on one \"person header\" at a time\n> was rewritten to use the current apply_mailmap_to_header() function.\n> The original implementation processed author and committer separately,\n> but the rewrite introduced this loop-based approach that failed to\n> properly handle the transition between identity lines.\n\nThanks for adding this context and improving the `sed` invocation in\nthe tests! Happy to stand behind this change too :-)\n"},{"id":"520225","messageId":"xmqqh60jab3o.fsf@gitster.g","threadId":"63621","inReplyTo":"CAP8UFD37rFvhO_XLhSjZNoOQ_ePwrrALYBcqKHfhMKcpqhkP8Q@mail.gmail.com","subject":"Re: [PATCH v2] cat-file: fix mailmap application for different author and committer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-13T15:56:59Z","receivedAt":"2025-06-13T15:57:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> On Fri, Jun 13, 2025 at 1:58 PM <siddharthasthana31@gmail.com> wrote:\n>\n> [...]\n>\n>> This bug was introduced in e9c1b0e3 (revision: improve\n>> commit_rewrite_person(), 2022-07-19) when the much simpler version of\n>> commit_rewrite_person() that worked on one \"person header\" at a time\n>> was rewritten to use the current apply_mailmap_to_header() function.\n>> The original implementation processed author and committer separately,\n>> but the rewrite introduced this loop-based approach that failed to\n>> properly handle the transition between identity lines.\n>\n> Thanks for adding this context and improving the `sed` invocation in\n> the tests! Happy to stand behind this change too :-)\n\nThanks, all.  Queued.  Let's mark it for 'next', hoping that the fix\nwill be in the first batch in the post 2.50 cycle.\n\n"}]}