name-rev: avoid cutoff timestamp underflow

When 'git name-rev' is invoked with commit-ish parameters, it tries to save some work, and doesn't visit commits older than the committer date of the oldest given commit minus a one day worth of slop. Since our 'timestamp_t' is an unsigned type, this leads to a timestamp underflow when the committer date of the oldest given commit is within a day of the UNIX epoch. As a result the cutoff timestamp ends up far-far in the future, and 'git name-rev' doesn't visit any commits, and names each given commit as 'undefined'. Check whether subtracting the slop from the oldest committer date would lead to an underflow, and use no cutoff in that case. We don't have a TIME_MIN constant, dddbad728c (timestamp_t: a new data type for timestamps, 2017-04-26) didn't add one, so do it now. Note that the type of the cutoff timestamp variable used to be signed before 5589e87fd8 (name-rev: change a "long" variable to timestamp_t, 2017-05-20). The behavior was still the same even back then, but the underflow didn't happen when substracting the slop from the oldest committer date, but when comparing the signed cutoff timestamp with unsigned committer dates in name_rev(). IOW, this underflow bug is as old as 'git name-rev' itself. Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

SZEDER Gábor committed Sep 24, 2019 at 09:32 UTC 2e09c01232b1e1f5f383e62865b6049ff76270b3
3 files changed +28 -3
builtin/name-rev.c
+12 -3
@@ -9,7 +9,11 @@
9 #include "sha1-lookup.h"
10 #include "commit-slab.h"
11
12 -#define CUTOFF_DATE_SLOP 86400 /* one day */
12 +/*
13 + * One day. See the 'name a rev shortly after epoch' test in t6120 when
14 + * changing this value
15 + */
16 +#define CUTOFF_DATE_SLOP 86400
17
18 typedef struct rev_name {
19 const char *tip_name;
@@ -481,8 +485,13 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
485 add_object_array(object, *argv, &revs);
486 }
487
484 - if (cutoff)
485 - cutoff = cutoff - CUTOFF_DATE_SLOP;
488 + if (cutoff) {
489 + /* check for undeflow */
490 + if (cutoff > TIME_MIN + CUTOFF_DATE_SLOP)
491 + cutoff = cutoff - CUTOFF_DATE_SLOP;
492 + else
493 + cutoff = TIME_MIN;
494 + }
495 for_each_ref(name_ref, &data);
496
497 if (transform_stdin) {
git-compat-util.h
+1
@@ -344,6 +344,7 @@ typedef uintmax_t timestamp_t;
344 #define PRItime PRIuMAX
345 #define parse_timestamp strtoumax
346 #define TIME_MAX UINTMAX_MAX
347 +#define TIME_MIN 0
348
349 #ifndef PATH_SEP
350 #define PATH_SEP ':'
t/t6120-describe.sh
+15
@@ -424,4 +424,19 @@ test_expect_success 'describe complains about missing object' '
424 test_must_fail git describe $ZERO_OID
425 '
426
427 +test_expect_success 'name-rev a rev shortly after epoch' '
428 + test_when_finished "git checkout master" &&
429 +
430 + git checkout --orphan no-timestamp-underflow &&
431 + # Any date closer to epoch than the CUTOFF_DATE_SLOP constant
432 + # in builtin/name-rev.c.
433 + GIT_COMMITTER_DATE="@1234 +0000" \
434 + git commit -m "committer date shortly after epoch" &&
435 + old_commit_oid=$(git rev-parse HEAD) &&
436 +
437 + echo "$old_commit_oid no-timestamp-underflow" >expect &&
438 + git name-rev $old_commit_oid >actual &&
439 + test_cmp expect actual
440 +'
441 +
442 test_done