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

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

[Dmitry V. Levin]

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

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

[Nguyen Thai Ngoc Duy]

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 <pclouds@gmail.com>
---
 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< --

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

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 1275 bytes --]

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:
> > 
> > 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

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

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

[Nguyen Thai Ngoc Duy]

2011/11/1 Dmitry V. Levin <ldv@altlinux.org>:
> 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

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

[Nguyen Thai Ngoc Duy]

2011/11/1 Nguyen Thai Ngoc Duy <pclouds@gmail.com>:
> 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?

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  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< --
>



-- 
Duy

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

[Junio C Hamano]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> 2011/11/1 Nguyen Thai Ngoc Duy <pclouds@gmail.com>:
>> 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?

At this late point in the release cycle, the topic itself is a Meh for me,
especially when other discussions for next cycle occupies my attention.