{"thread":{"id":"47913","subject":"[PATCH] Call timegm and timelocal with 4-digit year","startedAt":"2018-02-23T17:22:03Z","lastAt":"2018-02-26T11:00:13Z","messageCount":4,"participants":["Bernhard M. Wiedemann","Junio C Hamano","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"340043","messageId":"20180223172045.32090-1-bwiedemann@suse.de","threadId":"47913","inReplyTo":null,"subject":"[PATCH] Call timegm and timelocal with 4-digit year","fromName":"Bernhard M. Wiedemann","fromEmail":"bwiedemann@suse.de","sentAt":"2018-02-23T17:20:45Z","receivedAt":"2018-02-23T17:22:03Z","isPatch":true,"sender":{"key":"bwiedemann@suse.de","avatar":null},"body":"amazingly timegm(gmtime(0)) is only 0 before 2020\nbecause perl's timegm deviates from GNU timegm(3) in how it handles years.\n\nman Time::Local says\n\n Whenever possible, use an absolute four digit year instead.\n\nwith a detailed explanation about ambiguity of 2-digit years above that.\n\nEven though this ambiguity is error-prone with >50% of users getting it\nwrong, it has been like this for 20+ years, so we just use 4-digit years\neverywhere to be on the safe side.\n\nWe add some extra logic to cvsimport because it allows 2-digit year\ninput and interpreting an 18 as 1918 can be avoided easily and safely.\n\nSigned-off-by: Bernhard M. Wiedemann <bwiedemann@suse.de>\n---\n contrib/examples/git-svnimport.perl | 2 +-\n git-cvsimport.perl                  | 4 +++-\n perl/Git.pm                         | 4 +++-\n perl/Git/SVN.pm                     | 2 +-\n 4 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/examples/git-svnimport.perl b/contrib/examples/git-svnimport.perl\nindex c414f0d9c..75a43e23b 100755\n--- a/contrib/examples/git-svnimport.perl\n+++ b/contrib/examples/git-svnimport.perl\n@@ -238,7 +238,7 @@ sub pdate($) {\n \tmy($d) = @_;\n \t$d =~ m#(\\d\\d\\d\\d)-(\\d\\d)-(\\d\\d)T(\\d\\d):(\\d\\d):(\\d\\d)#\n \t\tor die \"Unparseable date: $d\\n\";\n-\tmy $y=$1; $y-=1900 if $y>1900;\n+\tmy $y=$1; $y+=1900 if $y<1000;\n \treturn timegm($6||0,$5,$4,$3,$2-1,$y);\n }\n \ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex 2d8df8317..b31613cb8 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -601,7 +601,9 @@ sub pdate($) {\n \tmy ($d) = @_;\n \tm#(\\d{2,4})/(\\d\\d)/(\\d\\d)\\s(\\d\\d):(\\d\\d)(?::(\\d\\d))?#\n \t\tor die \"Unparseable date: $d\\n\";\n-\tmy $y=$1; $y-=1900 if $y>1900;\n+\tmy $y=$1;\n+\t$y+=100 if $y<70;\n+\t$y+=1900 if $y<1000;\n \treturn timegm($6||0,$5,$4,$3,$2-1,$y);\n }\n \ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex ffa09ace9..df62518c7 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -534,7 +534,9 @@ If TIME is not supplied, the current local time is used.\n sub get_tz_offset {\n \t# some systems don't handle or mishandle %z, so be creative.\n \tmy $t = shift || time;\n-\tmy $gm = timegm(localtime($t));\n+\tmy @t = localtime($t);\n+\t$t[5] += 1900;\n+\tmy $gm = timegm(@t);\n \tmy $sign = qw( + + - )[ $gm <=> $t ];\n \treturn sprintf(\"%s%02d%02d\", $sign, (gmtime(abs($t - $gm)))[2,1]);\n }\ndiff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\nindex bc4eed3d7..991a5885e 100644\n--- a/perl/Git/SVN.pm\n+++ b/perl/Git/SVN.pm\n@@ -1405,7 +1405,7 @@ sub parse_svn_date {\n \t\t$ENV{TZ} = 'UTC';\n \n \t\tmy $epoch_in_UTC =\n-\t\t    Time::Local::timelocal($S, $M, $H, $d, $m - 1, $Y - 1900);\n+\t\t    Time::Local::timelocal($S, $M, $H, $d, $m - 1, $Y);\n \n \t\t# Determine our local timezone (including DST) at the\n \t\t# time of $epoch_in_UTC.  $Git::SVN::Log::TZ stored the\n-- \n2.13.6\n\n"},{"id":"340086","messageId":"xmqqbmgfs5m4.fsf@gitster-ct.c.googlers.com","threadId":"47913","inReplyTo":"20180223172045.32090-1-bwiedemann@suse.de","subject":"Re: [PATCH] Call timegm and timelocal with 4-digit year","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-23T22:28:35Z","receivedAt":"2018-02-23T22:28:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Bernhard M. Wiedemann\" <bwiedemann@suse.de> writes:\n\n> amazingly timegm(gmtime(0)) is only 0 before 2020\n> because perl's timegm deviates from GNU timegm(3) in how it handles years.\n>\n> man Time::Local says\n>\n>  Whenever possible, use an absolute four digit year instead.\n>\n> with a detailed explanation about ambiguity of 2-digit years above that.\n\nThat's amusing.  Will queue; thanks.\n"},{"id":"340185","messageId":"87tvu6dntx.fsf@evledraar.gmail.com","threadId":"47913","inReplyTo":"20180223172045.32090-1-bwiedemann@suse.de","subject":"Re: [PATCH] Call timegm and timelocal with 4-digit year","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-02-24T22:28:42Z","receivedAt":"2018-02-24T22:28:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Feb 23 2018, Bernhard M. Wiedemann jotted:\n\n> amazingly timegm(gmtime(0)) is only 0 before 2020\n> because perl's timegm deviates from GNU timegm(3) in how it handles years.\n>\n> man Time::Local says\n>\n>  Whenever possible, use an absolute four digit year instead.\n>\n> with a detailed explanation about ambiguity of 2-digit years above that.\n>\n> Even though this ambiguity is error-prone with >50% of users getting it\n> wrong, it has been like this for 20+ years, so we just use 4-digit years\n> everywhere to be on the safe side.\n>\n> We add some extra logic to cvsimport because it allows 2-digit year\n> input and interpreting an 18 as 1918 can be avoided easily and safely.\n>\n> Signed-off-by: Bernhard M. Wiedemann <bwiedemann@suse.de>\n> ---\n>  contrib/examples/git-svnimport.perl | 2 +-\n>  git-cvsimport.perl                  | 4 +++-\n>  perl/Git.pm                         | 4 +++-\n>  perl/Git/SVN.pm                     | 2 +-\n>  4 files changed, 8 insertions(+), 4 deletions(-)\n>\n> diff --git a/contrib/examples/git-svnimport.perl b/contrib/examples/git-svnimport.perl\n> index c414f0d9c..75a43e23b 100755\n> --- a/contrib/examples/git-svnimport.perl\n> +++ b/contrib/examples/git-svnimport.perl\n> @@ -238,7 +238,7 @@ sub pdate($) {\n>  \tmy($d) = @_;\n>  \t$d =~ m#(\\d\\d\\d\\d)-(\\d\\d)-(\\d\\d)T(\\d\\d):(\\d\\d):(\\d\\d)#\n>  \t\tor die \"Unparseable date: $d\\n\";\n> -\tmy $y=$1; $y-=1900 if $y>1900;\n> +\tmy $y=$1; $y+=1900 if $y<1000;\n>  \treturn timegm($6||0,$5,$4,$3,$2-1,$y);\n\nI wonder if this whole thing was just cargo-culted to begin with. We\nneed to match (\\d\\d\\d\\d) here, so did SVN's format ever have years like\n\"0098\" (just \"98\" wouldn't match), so I suspect the whole munging could\nbe dropped, but this change seems harmless. Just something that jumped\nout at me reviewing this.\n\n> diff --git a/git-cvsimport.perl b/git-cvsimport.perl\n> index 2d8df8317..b31613cb8 100755\n> --- a/git-cvsimport.perl\n> +++ b/git-cvsimport.perl\n> @@ -601,7 +601,9 @@ sub pdate($) {\n>  \tmy ($d) = @_;\n>  \tm#(\\d{2,4})/(\\d\\d)/(\\d\\d)\\s(\\d\\d):(\\d\\d)(?::(\\d\\d))?#\n>  \t\tor die \"Unparseable date: $d\\n\";\n> -\tmy $y=$1; $y-=1900 if $y>1900;\n> +\tmy $y=$1;\n> +\t$y+=100 if $y<70;\n> +\t$y+=1900 if $y<1000;\n>  \treturn timegm($6||0,$5,$4,$3,$2-1,$y);\n>  }\n\nMy Time::Local 1.2300 on perl 5.024001 currently interprets \"69\" here as\n1969, but after this it'll be 2069.\n\nNow I doubt anyone's going to be importing CVS history of projects a\nlittle over 20 years before CVS was created in 1990 (although I suppose\nold imports...), but just wanted to note it since it seems odd for code\nthat's auto-interpreting double digit years for the purposes of\nimporting existing data to end up in an edge case where it returns dates\nmore than 50 years in the future.\n\n> diff --git a/perl/Git.pm b/perl/Git.pm\n> index ffa09ace9..df62518c7 100644\n> --- a/perl/Git.pm\n> +++ b/perl/Git.pm\n> @@ -534,7 +534,9 @@ If TIME is not supplied, the current local time is used.\n>  sub get_tz_offset {\n>  \t# some systems don't handle or mishandle %z, so be creative.\n>  \tmy $t = shift || time;\n> -\tmy $gm = timegm(localtime($t));\n> +\tmy @t = localtime($t);\n> +\t$t[5] += 1900;\n> +\tmy $gm = timegm(@t);\n>\n\nNice. Just using the 4-digit date is always more correct and won't ever\nbe buggy.\n\n>  \tmy $sign = qw( + + - )[ $gm <=> $t ];\n>  \treturn sprintf(\"%s%02d%02d\", $sign, (gmtime(abs($t - $gm)))[2,1]);\n>  }\n> diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\n> index bc4eed3d7..991a5885e 100644\n> --- a/perl/Git/SVN.pm\n> +++ b/perl/Git/SVN.pm\n> @@ -1405,7 +1405,7 @@ sub parse_svn_date {\n>  \t\t$ENV{TZ} = 'UTC';\n>\n>  \t\tmy $epoch_in_UTC =\n> -\t\t    Time::Local::timelocal($S, $M, $H, $d, $m - 1, $Y - 1900);\n> +\t\t    Time::Local::timelocal($S, $M, $H, $d, $m - 1, $Y);\n\nDitto. Nicely caught.\n\n>\n>  \t\t# Determine our local timezone (including DST) at the\n>  \t\t# time of $epoch_in_UTC.  $Git::SVN::Log::TZ stored the\n\nAnyway, this all looks good to me as-is. That CVS edge case is obscure\nand not worth focusing on, and the SVN one could be fixed up in another\ncommit if anyone cared.\n\nI just spent a bit more time than I should have wondering what this\ntimegm() edge case was about and whether it might impact other\n(unrelated to git) code I had.\n"},{"id":"340288","messageId":"81704602-1c81-f46c-f306-968147327e74@suse.de","threadId":"47913","inReplyTo":"87tvu6dntx.fsf@evledraar.gmail.com","subject":"Re: [PATCH] Call timegm and timelocal with 4-digit year","fromName":"Bernhard M. Wiedemann","fromEmail":"bwiedemann@suse.de","sentAt":"2018-02-26T10:59:59Z","receivedAt":"2018-02-26T11:00:13Z","isPatch":true,"sender":{"key":"bwiedemann@suse.de","avatar":null},"body":"On 2018-02-24 23:28, Ævar Arnfjörð Bjarmason wrote:\n> My Time::Local 1.2300 on perl 5.024001 currently interprets \"69\" here as\n> 1969, but after this it'll be 2069.\n\non one hand, there is already\nperl -e 'use Time::Local;\n  print scalar gmtime(Time::Local::timegm(0,0,0,1,0,68))'\nSun Jan  1 00:00:00 2068\n\nThe problem here is the 'currently', because in 11 months from now, 69\nwill be interpreted as 2069 by Time::Local, too.\n\nfaketime 2019-01-01 perl -e 'use Time::Local;\n   print scalar gmtime(Time::Local::timegm(0,0,0,1,0,69))'\nTue Jan  1 00:00:00 2069\n\n\nWe could compromize for\n$y+=100 if $y<69;\n\nto freeze the behaviour to what Time::Local does in 2018, but then there\nmight not be actual repos with such 2-digit year timestamps.\n\nThere are definitely CVS repos from before 1999 and without this change,\nthose might be misinterpreted in 2049 (or before, depending on age)\n"}]}