threads / patch / 47913

patchCall timegm and timelocal with 4-digit year

Subject: [PATCH] Call timegm and timelocal with 4-digit year

## tl;dr

4 messages between Feb 23, 2018 and Feb 26, 2018. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Bernhard M. Wiedemann· Feb 23, 2018, 17:20 UTC · lore

amazingly timegm(gmtime(0)) is only 0 before 2020 because perl's timegm deviates from GNU timegm(3) in how it handles years.

man Time::Local says
 Whenever possible, use an absolute four digit year instead.
with a detailed explanation about ambiguity of 2-digit years above that.

Even though this ambiguity is error-prone with >50% of users getting it wrong, it has been like this for 20+ years, so we just use 4-digit years everywhere to be on the safe side.

We add some extra logic to cvsimport because it allows 2-digit year input and interpreting an 18 as 1918 can be avoided easily and safely.

Signed-off-by: Bernhard M. Wiedemann <bwiedemann@suse.de>
---
 contrib/examples/git-svnimport.perl | 2 +-
 git-cvsimport.perl                  | 4 +++-
 perl/Git.pm                         | 4 +++-
 perl/Git/SVN.pm                     | 2 +-
 4 files changed, 8 insertions(+), 4 deletions(-)
Show changes to 4 files +8 −4

contrib/examples/git-svnimport.perl, git-cvsimport.perl, perl/Git.pm, perl/Git/SVN.pm

diff --git a/contrib/examples/git-svnimport.perl b/contrib/examples/git-svnimport.perl
index c414f0d9c..75a43e23b 100755
--- a/contrib/examples/git-svnimport.perl
+++ b/contrib/examples/git-svnimport.perl
@@ -238,7 +238,7 @@ sub pdate($) {
 	my($d) = @_;
 	$d =~ m#(\d\d\d\d)-(\d\d)-(\d\d)T(\d\d):(\d\d):(\d\d)#
 		or die "Unparseable date: $d\n";
-	my $y=$1; $y-=1900 if $y>1900;
+	my $y=$1; $y+=1900 if $y<1000;
 	return timegm($6||0,$5,$4,$3,$2-1,$y);
 }
 
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 2d8df8317..b31613cb8 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -601,7 +601,9 @@ sub pdate($) {
 	my ($d) = @_;
 	m#(\d{2,4})/(\d\d)/(\d\d)\s(\d\d):(\d\d)(?::(\d\d))?#
 		or die "Unparseable date: $d\n";
-	my $y=$1; $y-=1900 if $y>1900;
+	my $y=$1;
+	$y+=100 if $y<70;
+	$y+=1900 if $y<1000;
 	return timegm($6||0,$5,$4,$3,$2-1,$y);
 }
 
diff --git a/perl/Git.pm b/perl/Git.pm
index ffa09ace9..df62518c7 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -534,7 +534,9 @@ If TIME is not supplied, the current local time is used.
 sub get_tz_offset {
 	# some systems don't handle or mishandle %z, so be creative.
 	my $t = shift || time;
-	my $gm = timegm(localtime($t));
+	my @t = localtime($t);
+	$t[5] += 1900;
+	my $gm = timegm(@t);
 	my $sign = qw( + + - )[ $gm <=> $t ];
 	return sprintf("%s%02d%02d", $sign, (gmtime(abs($t - $gm)))[2,1]);
 }
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index bc4eed3d7..991a5885e 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1405,7 +1405,7 @@ sub parse_svn_date {
 		$ENV{TZ} = 'UTC';
 
 		my $epoch_in_UTC =
-		    Time::Local::timelocal($S, $M, $H, $d, $m - 1, $Y - 1900);
+		    Time::Local::timelocal($S, $M, $H, $d, $m - 1, $Y);
 
 		# Determine our local timezone (including DST) at the
 		# time of $epoch_in_UTC.  $Git::SVN::Log::TZ stored the
-- 
2.13.6
Junio C Hamano· Feb 23, 2018, 22:28 UTC · re: Bernhard M. Wiedemann · lore

Re: [PATCH] Call timegm and timelocal with 4-digit year

"Bernhard M. Wiedemann" <bwiedemann@suse.de> writes:
Show 8 quoted lines
> amazingly timegm(gmtime(0)) is only 0 before 2020
> because perl's timegm deviates from GNU timegm(3) in how it handles years.
>
> man Time::Local says
>
>  Whenever possible, use an absolute four digit year instead.
>
> with a detailed explanation about ambiguity of 2-digit years above that.
That's amusing.  Will queue; thanks.
Ævar Arnfjörð Bjarmason· Feb 24, 2018, 22:28 UTC · re: Bernhard M. Wiedemann · lore

Re: [PATCH] Call timegm and timelocal with 4-digit year

On Fri, Feb 23 2018, Bernhard M. Wiedemann jotted:
Show 35 quoted lines
> amazingly timegm(gmtime(0)) is only 0 before 2020
> because perl's timegm deviates from GNU timegm(3) in how it handles years.
>
> man Time::Local says
>
>  Whenever possible, use an absolute four digit year instead.
>
> with a detailed explanation about ambiguity of 2-digit years above that.
>
> Even though this ambiguity is error-prone with >50% of users getting it
> wrong, it has been like this for 20+ years, so we just use 4-digit years
> everywhere to be on the safe side.
>
> We add some extra logic to cvsimport because it allows 2-digit year
> input and interpreting an 18 as 1918 can be avoided easily and safely.
>
> Signed-off-by: Bernhard M. Wiedemann <bwiedemann@suse.de>
> ---
>  contrib/examples/git-svnimport.perl | 2 +-
>  git-cvsimport.perl                  | 4 +++-
>  perl/Git.pm                         | 4 +++-
>  perl/Git/SVN.pm                     | 2 +-
>  4 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/contrib/examples/git-svnimport.perl b/contrib/examples/git-svnimport.perl
> index c414f0d9c..75a43e23b 100755
> --- a/contrib/examples/git-svnimport.perl
> +++ b/contrib/examples/git-svnimport.perl
> @@ -238,7 +238,7 @@ sub pdate($) {
>  	my($d) = @_;
>  	$d =~ m#(\d\d\d\d)-(\d\d)-(\d\d)T(\d\d):(\d\d):(\d\d)#
>  		or die "Unparseable date: $d\n";
> -	my $y=$1; $y-=1900 if $y>1900;
> +	my $y=$1; $y+=1900 if $y<1000;
>  	return timegm($6||0,$5,$4,$3,$2-1,$y);

I wonder if this whole thing was just cargo-culted to begin with. We need to match (\d\d\d\d) here, so did SVN's format ever have years like "0098" (just "98" wouldn't match), so I suspect the whole munging could be dropped, but this change seems harmless. Just something that jumped out at me reviewing this.

Show 14 quoted lines
> diff --git a/git-cvsimport.perl b/git-cvsimport.perl
> index 2d8df8317..b31613cb8 100755
> --- a/git-cvsimport.perl
> +++ b/git-cvsimport.perl
> @@ -601,7 +601,9 @@ sub pdate($) {
>  	my ($d) = @_;
>  	m#(\d{2,4})/(\d\d)/(\d\d)\s(\d\d):(\d\d)(?::(\d\d))?#
>  		or die "Unparseable date: $d\n";
> -	my $y=$1; $y-=1900 if $y>1900;
> +	my $y=$1;
> +	$y+=100 if $y<70;
> +	$y+=1900 if $y<1000;
>  	return timegm($6||0,$5,$4,$3,$2-1,$y);
>  }

My Time::Local 1.2300 on perl 5.024001 currently interprets "69" here as 1969, but after this it'll be 2069.

Now I doubt anyone's going to be importing CVS history of projects a little over 20 years before CVS was created in 1990 (although I suppose old imports...), but just wanted to note it since it seems odd for code that's auto-interpreting double digit years for the purposes of importing existing data to end up in an edge case where it returns dates more than 50 years in the future.

Show 13 quoted lines
> diff --git a/perl/Git.pm b/perl/Git.pm
> index ffa09ace9..df62518c7 100644
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -534,7 +534,9 @@ If TIME is not supplied, the current local time is used.
>  sub get_tz_offset {
>  	# some systems don't handle or mishandle %z, so be creative.
>  	my $t = shift || time;
> -	my $gm = timegm(localtime($t));
> +	my @t = localtime($t);
> +	$t[5] += 1900;
> +	my $gm = timegm(@t);
>

Nice. Just using the 4-digit date is always more correct and won't ever be buggy.

Show 13 quoted lines
>  	my $sign = qw( + + - )[ $gm <=> $t ];
>  	return sprintf("%s%02d%02d", $sign, (gmtime(abs($t - $gm)))[2,1]);
>  }
> diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
> index bc4eed3d7..991a5885e 100644
> --- a/perl/Git/SVN.pm
> +++ b/perl/Git/SVN.pm
> @@ -1405,7 +1405,7 @@ sub parse_svn_date {
>  		$ENV{TZ} = 'UTC';
>
>  		my $epoch_in_UTC =
> -		    Time::Local::timelocal($S, $M, $H, $d, $m - 1, $Y - 1900);
> +		    Time::Local::timelocal($S, $M, $H, $d, $m - 1, $Y);
Ditto. Nicely caught.
>
>  		# Determine our local timezone (including DST) at the
>  		# time of $epoch_in_UTC.  $Git::SVN::Log::TZ stored the

Anyway, this all looks good to me as-is. That CVS edge case is obscure and not worth focusing on, and the SVN one could be fixed up in another commit if anyone cared.

I just spent a bit more time than I should have wondering what this timegm() edge case was about and whether it might impact other (unrelated to git) code I had.

Bernhard M. Wiedemann· Feb 26, 2018, 10:59 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] Call timegm and timelocal with 4-digit year

On 2018-02-24 23:28, Ævar Arnfjörð Bjarmason wrote:
> My Time::Local 1.2300 on perl 5.024001 currently interprets "69" here as
> 1969, but after this it'll be 2069.
on one hand, there is already
perl -e 'use Time::Local;
  print scalar gmtime(Time::Local::timegm(0,0,0,1,0,68))'
Sun Jan  1 00:00:00 2068

The problem here is the 'currently', because in 11 months from now, 69 will be interpreted as 2069 by Time::Local, too.

faketime 2019-01-01 perl -e 'use Time::Local;
   print scalar gmtime(Time::Local::timegm(0,0,0,1,0,69))'
Tue Jan  1 00:00:00 2069

We could compromize for $y+=100 if $y<69;

to freeze the behaviour to what Time::Local does in 2018, but then there might not be actual repos with such 2-digit year timestamps.

There are definitely CVS repos from before 1999 and without this change, those might be misinterpreted in 2049 (or before, depending on age)

← back to recent threads