git rev-parse --since=1970-01-01 does not work reliably

Hi,

git rev-parse --since=1970-01-01 (and other git commands that take
date string arguments like --since) may fail when --since=1970-01-01 is
given.  Whether it fails or not depends on current time and timezone data.
For example, "TZ=Europe/Paris git rev-parse --since=1970-01-01" fails two
hours a day (between 00:00 and 02:00 CET), and those who use more eastern
timezones are even less lucky.  In artificial timezones like UTC-24 it
always fails:

$ TZ=UTC-24 git rev-parse --since=1970-01-01
--max-age=18446744073709523490

The problem is that several internal git functions implicitly convert
time_t to unsigned long, so when time_t gets negative, all date string
processing breaks.


-- 
ldv

On Mon, Oct 31, 2011 at 08:17:09PM +0400, Dmitry V. Levin wrote:

Hi,

git rev-parse --since=1970-01-01 (and other git commands that take
date string arguments like --since) may fail when --since=1970-01-01 is
given.  Whether it fails or not depends on current time and timezone data.
For example, "TZ=Europe/Paris git rev-parse --since=1970-01-01" fails two
hours a day (between 00:00 and 02:00 CET), and those who use more eastern
timezones are even less lucky.  In artificial timezones like UTC-24 it
always fails:

$ TZ=UTC-24 git rev-parse --since=1970-01-01
--max-age=18446744073709523490

Out of curiosity, why do you need to work with a time so close to that
date?

The problem is that several internal git functions implicitly convert
time_t to unsigned long, so when time_t gets negative, all date string
processing breaks.

I don't think it's worth supporting negative time_t, but we should at
least avoid misconversion.

-- 8< --
Subject: [PATCH] Do not accept negative time_t

We use unsigned long internally to present time, negative value just
breaks thing.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 date.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/date.c b/date.c
index 353e0a5..9cbd521 100644
--- a/date.c
+++ b/date.c

@@ -653,8 +653,12 @@ int parse_date_basic(const char *date, unsigned long *timestamp, int *offset)
 	if (*timestamp == -1)
 		return -1;
 
-	if (!tm_gmt)
+	if (!tm_gmt) {
+		if ((time_t)*timestamp < (time_t)*offset * 60)
+			die("unsupported time before Epoch");
 		*timestamp -= *offset * 60;
+	}
+
 	return 0; /* success */
 }
 

@@ -722,6 +726,8 @@ static unsigned long update_tm(struct tm *tm, struct tm *now, unsigned long sec)
 
 	n = mktime(tm) - sec;
 	localtime_r(&n, tm);
+	if (n < 0)
+		die("unsupported time before Epoch");
 	return n;
 }
 

-- 
1.7.4.74.g639db
-- 8< --

On Tue, Nov 01, 2011 at 06:13:20AM +0700, Nguyen Thai Ngoc Duy wrote:

On Mon, Oct 31, 2011 at 08:17:09PM +0400, Dmitry V. Levin wrote:

quoted

git rev-parse --since=1970-01-01 (and other git commands that take
date string arguments like --since) may fail when --since=1970-01-01 is
given.  Whether it fails or not depends on current time and timezone data.
For example, "TZ=Europe/Paris git rev-parse --since=1970-01-01" fails two
hours a day (between 00:00 and 02:00 CET), and those who use more eastern
timezones are even less lucky.  In artificial timezones like UTC-24 it
always fails:

$ TZ=UTC-24 git rev-parse --since=1970-01-01
--max-age=18446744073709523490

Out of curiosity, why do you need to work with a time so close to that
date?

There is an utility that used to invoke "git log --since='1970-01-01 UTC'"
by default, and I was unlucky enough to use it during these hours of
failure.  The utility is now fixed (it no longer calls git log with
--since option unless explicitly instructed to do so), and I hope git
is going to be fixed as well.

BTW, the timezone specifier (UTC) in "git rev-parse --since='1970-01-01 UTC'"
seems to be completely ignored by date string parser.


-- 
ldv

2011/11/1 Dmitry V. Levin [off-list ref]:

BTW, the timezone specifier (UTC) in "git rev-parse --since='1970-01-01 UTC'"
seems to be completely ignored by date string parser.

It takes this "00:00 1970-01-01 UTC"
-- 
Duy

2011/11/1 Nguyen Thai Ngoc Duy [off-list ref]:

Subject: [PATCH] Do not accept negative time_t

We use unsigned long internally to present time, negative value just
breaks thing.

Junio, what do you think about this patch?

-- 
Duy