From d8d2eb7d6b5e48c2bcb0e71a770f8a05375ac03e Mon Sep 17 00:00:00 2001
From: Jim Meyering <jim@meyering.net>
Date: Mon, 11 Oct 2010 17:41:16 +0200
Subject: [PATCH 1/3] mailmap: fix use of freed memory
On an x86_64 system (F13-based), I ran these commands in an empty directory:
git init
printf '%s\n' \
'<jdoe@example.com> <jdoe@example.COM>' \
'John <jdoe@example.com>' > .mailmap
git shortlog < /dev/null
Here's the result:
(reading log message from standard input)
*** glibc detected *** git: free(): invalid pointer: 0x0000000000f53730 ***
======= Backtrace: =========
/lib64/libc.so.6[0x31ba875676]
git[0x48c2a5]
git[0x4b9858]
...
zsh: abort (core dumped) git shortlog
What happened?
Some .mailmap entry is of the <email1> <email2> form,
while a subsequent one looks like "User Name <Email2>,
and the two email addresses on the right are not identical
but are "equal" when using a case-insensitive comparator.
Then, when add_mapping is processing the latter line, new_email is NULL
and we free me->email, yet do not replace it with a new strdup'd string.
Thus, when later we attempt to use the buffer behind that ->email pointer,
we reference freed memory.
The solution is to free ->email and ->name only if we're about to replace them.
[jc: squashed in the tests from Jonathan]
Signed-off-by: Jim Meyering <meyering@redhat.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
mailmap.c | 10 ++++++----
t/t4203-mailmap.sh | 41 ++++++++++++++++++++++++++++++++++++++---
2 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/mailmap.c b/mailmap.c
index f80b701292f..02fcfde0b0b 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -79,12 +79,14 @@ static void add_mapping(struct string_list *map,
if (old_name == NULL) {
debug_mm("mailmap: adding (simple) entry for %s at index %d\n", old_email, index);
/* Replace current name and new email for simple entry */
- free(me->name);
- free(me->email);
- if (new_name)
+ if (new_name) {
+ free(me->name);
me->name = xstrdup(new_name);
- if (new_email)
+ }
+ if (new_email) {
+ free(me->email);
me->email = xstrdup(new_email);
+ }
} else {
struct mailmap_info *mi = xmalloc(sizeof(struct mailmap_info));
debug_mm("mailmap: adding (complex) entry for %s at index %d\n", old_email, index);
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 9a7d1b44663..f4f82c02379 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -11,6 +11,7 @@ test_expect_success setup '
git commit -m initial &&
echo two >>one &&
git add one &&
+ test_tick &&
git commit --author "nick1 <bugs@company.xx>" -m second
'
@@ -54,7 +55,7 @@ Repo Guy (1):
EOF
test_expect_success 'mailmap.file set' '
- mkdir internal_mailmap &&
+ mkdir -p internal_mailmap &&
echo "Internal Guy <bugs@company.xx>" > internal_mailmap/.mailmap &&
git config mailmap.file internal_mailmap/.mailmap &&
git shortlog HEAD >actual &&
@@ -92,6 +93,40 @@ test_expect_success 'mailmap.file non-existant' '
test_cmp expect actual
'
+cat >expect <<\EOF
+Internal Guy (1):
+ second
+
+Repo Guy (1):
+ initial
+
+EOF
+
+test_expect_success 'name entry after email entry' '
+ mkdir -p internal_mailmap &&
+ echo "<bugs@company.xy> <bugs@company.xx>" >internal_mailmap/.mailmap &&
+ echo "Internal Guy <bugs@company.xx>" >>internal_mailmap/.mailmap &&
+ git shortlog >actual &&
+ test_cmp expect actual
+'
+
+cat >expect <<\EOF
+Internal Guy (1):
+ second
+
+Repo Guy (1):
+ initial
+
+EOF
+
+test_expect_success 'name entry after email entry, case-insensitive' '
+ mkdir -p internal_mailmap &&
+ echo "<bugs@company.xy> <bugs@company.xx>" >internal_mailmap/.mailmap &&
+ echo "Internal Guy <BUGS@Company.xx>" >>internal_mailmap/.mailmap &&
+ git shortlog >actual &&
+ test_cmp expect actual
+'
+
cat >expect <<\EOF
A U Thor (1):
initial
@@ -101,7 +136,7 @@ nick1 (1):
EOF
test_expect_success 'No mailmap files, but configured' '
- rm .mailmap &&
+ rm -f .mailmap internal_mailmap/.mailmap &&
git shortlog HEAD >actual &&
test_cmp expect actual
'
@@ -153,7 +188,7 @@ test_expect_success 'Shortlog output (complex mapping)' '
test_tick &&
git commit --author "CTO <cto@coompany.xx>" -m seventh &&
- mkdir internal_mailmap &&
+ mkdir -p internal_mailmap &&
echo "Committed <committer@example.com>" > internal_mailmap/.mailmap &&
echo "<cto@company.xx> <cto@coompany.xx>" >> internal_mailmap/.mailmap &&
echo "Some Dude <some@dude.xx> nick1 <bugs@company.xx>" >> internal_mailmap/.mailmap &&
From bfdfa3d414539e78f049fceab46767f3a0281721 Mon Sep 17 00:00:00 2001
From: Jonathan Nieder <jrnieder@gmail.com>
Date: Fri, 15 Oct 2010 12:12:19 -0500
Subject: [PATCH 2/3] t4203 (mailmap): stop hardcoding commit ids and dates
A seemingly innocuous change like adding test_tick somewhere can
completely upset the final mailmap test, since it checks commit
hashes and dates. Make the test less fragile by fuzzing away the
unpredictable parts and leaving in the authors (which is what the
test is about, anyway).
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t4203-mailmap.sh | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index f4f82c02379..a267d0734d7 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -4,6 +4,14 @@ test_description='.mailmap configurations'
. ./test-lib.sh
+fuzz_blame () {
+ sed "
+ s/$_x05[0-9a-f][0-9a-f][0-9a-f]/OBJID/g
+ s/$_x05[0-9a-f][0-9a-f]/OBJI/g
+ s/[-0-9]\{10\} [:0-9]\{8\} [-+][0-9]\{4\}/DATE/g
+ " "$@"
+}
+
test_expect_success setup '
echo one >one &&
git add one &&
@@ -233,18 +241,18 @@ test_expect_success 'Log output (complex mapping)' '
# git blame
cat >expect <<\EOF
-^3a2fdcb (A U Thor 2005-04-07 15:13:13 -0700 1) one
-7de6f99b (Some Dude 2005-04-07 15:13:13 -0700 2) two
-5815879d (Other Author 2005-04-07 15:14:13 -0700 3) three
-ff859d96 (Other Author 2005-04-07 15:15:13 -0700 4) four
-5ab6d4fa (Santa Claus 2005-04-07 15:16:13 -0700 5) five
-38a42d8b (Santa Claus 2005-04-07 15:17:13 -0700 6) six
-8ddc0386 (CTO 2005-04-07 15:18:13 -0700 7) seven
+^OBJI (A U Thor DATE 1) one
+OBJID (Some Dude DATE 2) two
+OBJID (Other Author DATE 3) three
+OBJID (Other Author DATE 4) four
+OBJID (Santa Claus DATE 5) five
+OBJID (Santa Claus DATE 6) six
+OBJID (CTO DATE 7) seven
EOF
-
test_expect_success 'Blame output (complex mapping)' '
git blame one >actual &&
- test_cmp expect actual
+ fuzz_blame actual >actual.fuzz &&
+ test_cmp expect actual.fuzz
'
test_done
From 3e3e1ef53285ceafd24bd88483c03a4cb604fe22 Mon Sep 17 00:00:00 2001
From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 19 Oct 2010 23:32:06 -0700
Subject: [PATCH 3/3] t4203: do not let "git shortlog" DWIM based on tty
The "shortlog" command defaults to HEAD only when its standard input is
connected to a terminal; otherwise it acts in the traditional "filter"
mode to read and summarize the "git log" output.
Two new tests added to t4203 assumed that the command always default to
HEAD, but when the standard input is closed (or connected to /dev/null),
it output empty, which is a summary of its empty input, causing the test
to break.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t4203-mailmap.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index a267d0734d7..e818de6ddd9 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -114,7 +114,7 @@ test_expect_success 'name entry after email entry' '
mkdir -p internal_mailmap &&
echo "<bugs@company.xy> <bugs@company.xx>" >internal_mailmap/.mailmap &&
echo "Internal Guy <bugs@company.xx>" >>internal_mailmap/.mailmap &&
- git shortlog >actual &&
+ git shortlog HEAD >actual &&
test_cmp expect actual
'
@@ -131,7 +131,7 @@ test_expect_success 'name entry after email entry, case-insensitive' '
mkdir -p internal_mailmap &&
echo "<bugs@company.xy> <bugs@company.xx>" >internal_mailmap/.mailmap &&
echo "Internal Guy <BUGS@Company.xx>" >>internal_mailmap/.mailmap &&
- git shortlog >actual &&
+ git shortlog HEAD >actual &&
test_cmp expect actual
'