The read_mailmap_buf function reads each line of the mailmap
using strchrnul, like:
const char *end = strchrnul(buf, '\n');
unsigned long linelen = end - buf + 1;
But that's off-by-one when we actually hit the NUL byte; our
line does not have a terminator, and so is only "end - buf"
bytes long. As a result, when we subtract the linelen from
the total len, we end up with (unsigned long)-1 bytes left
in the buffer, and we start reading random junk from memory.
We could fix it with:
unsigned long linelen = end - buf + !!*end;
but it is questionable for a function called read_mailmap_buf
to be using strchrnul in the first place. It happens to work
because our buffers come from blobs, and read_sha1_file always
NUL-terminates the object data. But let's future-proof the
function by actually handling non-terminated strings
correctly, and fix the off-by-one at the same time.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
Intended for 'maint'. The bug was introduced in 0861090, but I built the
fix on top of 8c473ce, the tip of the jk/mailmap-from-blob topic, as it
avoids annoying textual conflicts in the test script.
v1.8.2 was the first version with the bug, so this is not an "oops, we
failed to find this new bug during v1.8.4-rc series" problem. I found it
now because I turned on mailmap.blob for all of github.com, which
exposed the code to a much larger array of random inputs.
This is the minimal fix. Another option would be to switch
read_mailmap_buf to read_mailmap_string, and I think we could even get
away with avoiding the extra allocation/copy in the loop (because
read_mailmap_line seems to cope with newline-or-EOS just fine). But it
may be better to save that for 'master'.
mailmap.c | 12 +++++++++---
t/t4203-mailmap.sh | 16 +++++++++++++++-
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/mailmap.c b/mailmap.c
index b16542f..a635873 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -192,10 +192,16 @@ static void read_mailmap_buf(struct string_list *map,
char **repo_abbrev)
{
while (len) {
- const char *end = strchrnul(buf, '\n');
- unsigned long linelen = end - buf + 1;
- char *line = xmemdupz(buf, linelen);
+ const char *end = memchr(buf, '\n', len);
+ unsigned long linelen;
+ char *line;
+ if (end)
+ linelen = end - buf + 1;
+ else
+ linelen = len;
+
+ line = xmemdupz(buf, linelen);
read_mailmap_line(map, line, repo_abbrev);
free(line);
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index aae30d9..10c7b12 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -159,7 +159,8 @@ test_expect_success 'setup mailmap blob tests' '
Blob Guy <author@xxxxxxxxxxx>
Blob Guy <bugs@xxxxxxxxxx>
EOF
- git add just-bugs both &&
+ printf "Tricky Guy <author@xxxxxxxxxxx>" >no-newline &&
+ git add just-bugs both no-newline &&
git commit -m "my mailmaps" &&
echo "Repo Guy <author@xxxxxxxxxxx>" >.mailmap &&
echo "Internal Guy <author@xxxxxxxxxxx>" >internal.map
@@ -243,6 +244,19 @@ test_expect_success 'mailmap.blob defaults to HEAD:.mailmap in bare repo' '
)
'
+test_expect_success 'mailmap.blob can handle blobs without trailing newline' '
+ cat >expect <<-\EOF &&
+ Tricky Guy (1):
+ initial
+
+ nick1 (1):
+ second
+
+ EOF
+ git -c mailmap.blob=map:no-newline shortlog HEAD >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'cleanup after mailmap.blob tests' '
rm -f .mailmap
'
--
1.8.4.2.g87d4a77
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html