To generate a patch id, we format the diff header into a
fixed-size buffer, and then feed the result to our sha1
computation. The fixed buffer has size '4*PATH_MAX + 20',
which in theory accomodates the four filenames plus some
extra data. Except:
1. The filenames may not be constrained to PATH_MAX. The
static value may not be a real limit on the current
filesystem. Moreover, we may compute patch-ids for
names stored only in git, without touching the current
filesystem at all.
2. The 20 bytes is not nearly enough to cover the
extra content we put in the buffer.
As a result, the data we feed to the sha1 computation may be
truncated, and it's possible that a commit with a very long
filename could erroneously collide in the patch-id space
with another commit. For instance, if one commit modified
"really-long-filename/foo" and another modified "bar" in the
same directory.
In practice this is unlikely. Because the filenames are
repeated, and because there's a single cutoff at the end of
the buffer, the offending filename would have to be on the
order of four times larger than PATH_MAX.
But it's easy to fix by moving to a strbuf.
Technically this may change the output of patch-id for very
long filenames, but it's not worth making an exception for
this in the --stable output. It was a bug, and one that only
affected an unlikely set of paths. And anyway, the exact
value would have varied from platform to platform depending
on the value of PATH_MAX, so there is no "stable" value.
Signed-off-by: Jeff King <p...@peff.net>
---
diff.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/diff.c b/diff.c
index 58cb72d7e..89b5dc890 100644
--- a/diff.c
+++ b/diff.c
@@ -4577,7 +4577,7 @@ static int diff_get_patch_id(struct diff_options
*options, unsigned char *sha1,
int i;
git_SHA_CTX ctx;
struct patch_id_t data;
- char buffer[PATH_MAX * 4 + 20];
+ struct strbuf buffer = STRBUF_INIT;
git_SHA1_Init(&ctx);
memset(&data, 0, sizeof(struct patch_id_t));
@@ -4607,10 +4607,11 @@ static int diff_get_patch_id(struct diff_options
*options, unsigned char *sha1,
diff_fill_sha1_info(p->one);
diff_fill_sha1_info(p->two);
+ strbuf_reset(&buffer);
len1 = remove_space(p->one->path, strlen(p->one->path));
len2 = remove_space(p->two->path, strlen(p->two->path));
if (p->one->mode == 0)
- len1 = snprintf(buffer, sizeof(buffer),
+ strbuf_addf(&buffer,
"diff--gita/%.*sb/%.*s"
"newfilemode%06o"
"---/dev/null"
@@ -4620,7 +4621,7 @@ static int diff_get_patch_id(struct diff_options
*options, unsigned char *sha1,
p->two->mode,
len2, p->two->path);
else if (p->two->mode == 0)
- len1 = snprintf(buffer, sizeof(buffer),
+ strbuf_addf(&buffer,
"diff--gita/%.*sb/%.*s"
"deletedfilemode%06o"
"---a/%.*s"
@@ -4630,7 +4631,7 @@ static int diff_get_patch_id(struct diff_options
*options, unsigned char *sha1,
p->one->mode,
len1, p->one->path);
else
- len1 = snprintf(buffer, sizeof(buffer),
+ strbuf_addf(&buffer,
"diff--gita/%.*sb/%.*s"
"---a/%.*s"
"+++b/%.*s",
@@ -4638,14 +4639,16 @@ static int diff_get_patch_id(struct diff_options
*options, unsigned char *sha1,
len2, p->two->path,
len1, p->one->path,
len2, p->two->path);
- git_SHA1_Update(&ctx, buffer, len1);
+ git_SHA1_Update(&ctx, buffer.buf, buffer.len);
if (diff_header_only)
continue;
if (fill_mmfile(&mf1, p->one) < 0 ||
- fill_mmfile(&mf2, p->two) < 0)
+ fill_mmfile(&mf2, p->two) < 0) {
+ strbuf_release(&buffer);
return error("unable to read files to diff");
+ }
if (diff_filespec_is_binary(p->one) ||
diff_filespec_is_binary(p->two)) {
@@ -4660,11 +4663,14 @@ static int diff_get_patch_id(struct diff_options
*options, unsigned char *sha1,
xecfg.ctxlen = 3;
xecfg.flags = 0;
if (xdi_diff_outf(&mf1, &mf2, patch_id_consume, &data,
- &xpp, &xecfg))
+ &xpp, &xecfg)) {
+ strbuf_release(&buffer);
return error("unable to generate patch-id diff for %s",
p->one->path);
+ }
}
+ strbuf_release(&buffer);
git_SHA1_Final(sha1, &ctx);
return 0;
}
--
2.12.2.845.g55fcf8b10
