[PATCH v2] mailmap: fix check-mailmap with full mailmap line

[PATCH v2] mailmap: fix check-mailmap with full mailmap line

[Jacob Keller]

From: Jacob Keller <jacob.keller@gmail.com>

I recently had reported to me a crash from a coworker using the recently
added sendemail mailmap support:

  3724814 Segmentation fault      (core dumped) git check-mailmap "bugs@company.xx"

This appears to happen because of the NULL pointer name passed into
map_user(). Fix this by passing "" instead of NULL so that we have a
valid pointer.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
Changes in v2:
- Just fix the NULL pointer dereference, leave mailmap resolution as-is
- Link to v1: https://lore.kernel.org/r/20250213-jk-fix-sendemail-mailinfo-v1-1-c0b06c215f21@gmail.com
---
 builtin/check-mailmap.c |  2 +-
 t/t4203-mailmap.sh      | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/builtin/check-mailmap.c b/builtin/check-mailmap.c
index df00b5ee13adb87881b8c1e92cac256e6ad319d1..be2cebe12152e38d3bb8cf12948823c8d710bdda 100644
--- a/builtin/check-mailmap.c
+++ b/builtin/check-mailmap.c
@@ -35,7 +35,7 @@ static void check_mailmap(struct string_list *mailmap, const char *contact)
 		mail = ident.mail_begin;
 		maillen = ident.mail_end - ident.mail_begin;
 	} else {
-		name = NULL;
+		name = "";
 		namelen = 0;
 		mail = contact;
 		maillen = strlen(contact);
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 24214919312777b76e4d3b2b784bcb953583750a..4a6242ff99b59ea1a46eb14ca812c94e5e620162 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -113,6 +113,18 @@ test_expect_success 'check-mailmap --stdin simple address: no mapping' '
 	test_cmp expect actual
 '
 
+test_expect_success 'check-mailmap name and address: mapping' '
+	test_when_finished "rm .mailmap" &&
+	cat >.mailmap <<-EOF &&
+	Bug Reports <bugs-new@company.xx> Bugs <bugs@company.xx>
+	EOF
+	cat >expect <<-EOF &&
+	<bugs@company.xx>
+	EOF
+	git check-mailmap "bugs@company.xx" >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'No mailmap' '
 	cat >expect <<-EOF &&
 	$GIT_AUTHOR_NAME (1):

---
base-commit: b838bf1938926a7a900166136d995d86f8a00e24
change-id: 20250213-jk-fix-sendemail-mailinfo-32f027b1b9e7

Best regards,
-- 
Jacob Keller <jacob.keller@gmail.com>

Re: [PATCH v2] mailmap: fix check-mailmap with full mailmap line

[Junio C Hamano]

Jacob Keller <jacob.e.keller@intel.com> writes:

> From: Jacob Keller <jacob.keller@gmail.com>
>
> I recently had reported to me a crash from a coworker using the recently
> added sendemail mailmap support:
>
>   3724814 Segmentation fault      (core dumped) git check-mailmap "bugs@company.xx"
>
> This appears to happen because of the NULL pointer name passed into
> map_user(). Fix this by passing "" instead of NULL so that we have a
> valid pointer.
>
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> ---
> Changes in v2:
> - Just fix the NULL pointer dereference, leave mailmap resolution as-is
> - Link to v1: https://lore.kernel.org/r/20250213-jk-fix-sendemail-mailinfo-v1-1-c0b06c215f21@gmail.com
> ---
>  builtin/check-mailmap.c |  2 +-
>  t/t4203-mailmap.sh      | 12 ++++++++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/check-mailmap.c b/builtin/check-mailmap.c
> index df00b5ee13adb87881b8c1e92cac256e6ad319d1..be2cebe12152e38d3bb8cf12948823c8d710bdda 100644
> --- a/builtin/check-mailmap.c
> +++ b/builtin/check-mailmap.c
> @@ -35,7 +35,7 @@ static void check_mailmap(struct string_list *mailmap, const char *contact)
>  		mail = ident.mail_begin;
>  		maillen = ident.mail_end - ident.mail_begin;
>  	} else {
> -		name = NULL;
> +		name = "";
>  		namelen = 0;
>  		mail = contact;
>  		maillen = strlen(contact);

OK.  I audited all users of map_user(), and everybody else
(including the other side of the if/else we see here) follows the
pattern to point name and mail into an "ident" instance, which would
never be NULL.  The way an "ident" instance signals that there is no
name is to have the name_begin and name_end members point at the same
byte.

So this change obviously is a good thing to do.

The callee, map_user() does assume both email and name pointers
point at valid strings, passes them to lookup_prefix(), and have
them receive what a mailmap_info instance records.  We may want
to document the calling convention a bit better, but that is
obviously outside the scope of this immediate fix.

Thanks.  Will queue.


> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
> index 24214919312777b76e4d3b2b784bcb953583750a..4a6242ff99b59ea1a46eb14ca812c94e5e620162 100755
> --- a/t/t4203-mailmap.sh
> +++ b/t/t4203-mailmap.sh
> @@ -113,6 +113,18 @@ test_expect_success 'check-mailmap --stdin simple address: no mapping' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'check-mailmap name and address: mapping' '
> +	test_when_finished "rm .mailmap" &&
> +	cat >.mailmap <<-EOF &&
> +	Bug Reports <bugs-new@company.xx> Bugs <bugs@company.xx>
> +	EOF
> +	cat >expect <<-EOF &&
> +	<bugs@company.xx>
> +	EOF
> +	git check-mailmap "bugs@company.xx" >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_expect_success 'No mailmap' '
>  	cat >expect <<-EOF &&
>  	$GIT_AUTHOR_NAME (1):
>
> ---
> base-commit: b838bf1938926a7a900166136d995d86f8a00e24
> change-id: 20250213-jk-fix-sendemail-mailinfo-32f027b1b9e7
>
> Best regards,