Item11869: Foswiki::Time::parseTime doesn't like 1901-1959

pencil
Priority: Urgent
Current State: Closed
Released In: 1.2.0
Target Release: minor
Applies To: Engine
Component: FoswikiTime
Branches: trunk
Reported By: JulianLevens
Waiting For:
Last Change By: CrawfordCurrie
I was reading the code and thought it looked odd. So I wrote a quick test.

   my $laste = Foswiki::Time::parseTime("10",1);
   for my $d (11..2012) {
        my $ed = Foswiki::Time::parseTime($d,1);
        $text .= "Undef when year = $d\n" if !defined $ed;
        next if !defined $ed;
        $text .= "Failed when year = $d\n" if $ed < $laste;
        $laste = $ed;
   }
   
   $laste = Foswiki::Time::parseTime("10-01-01",1);
   for my $d (11..2012) {
        my $ed = Foswiki::Time::parseTime("$d-01-01",1);
        $text .= "Undef when year = $d\n" if !defined $ed;
        next if !defined $ed;
        my $diff = $ed-$laste;
        $text .= "Failed when year = $d.  $ed lt $laste ($diff)\n" if $diff > 31622400 || $diff < 1;
        $laste = $ed;
   }

With the following astonishing results:

Simple integer treated as a year:
Undef when year = 11
...
Undef when year = 59
Failed when year = 63
Failed when year = 1000
Undef when year = 1901
...
Undef when year = 1959
Failed when year = 1963

Repeated with integer as n-01-01
Undef when year = 11
...
Undef when year = 12
...
Undef when year = 59
Failed when year = 63
Failed when year = 1000
Undef when year = 1901
...
Undef when year = 1959
Failed when year = 1963

The causes are I believe as follows:
  • Not wanting to treat 01-59 as a year in case it's the number of seconds or minutes
  • Incorrectly subtracting 1900 from 4 digit years before passing to Time::Local::timelocal or Time::Local::timegm, which according to http://perldoc.perl.org/Time/Local.html will treat 4 digit years just as they are.
  • Also from http://perldoc.perl.org/Time/Local.html we cannot handle 3 digit years (unless we accept interpretation as year+1900)

Considering 62 > 63 makes sense from the digit windowing point of view as it's really 2062 > 1963. However, this get extended to 1962 > 1963, because after subtracting 1900 unnecessarily we are back to 62 > 63 which is two digit windowed.

I also note the following useful bit of info from http://perldoc.perl.org/Time/Local.html

  • As of version 5.12.0, perl has stopped using the underlying time library of the operating system it's running on and has its own implementation of those routines with a safe range of at least +/ 2**52 (about 142 million years).

As an aside, do we really want to consider any plain integer of 2 digits or more as a year and construct the epoch time from $int-01-01 00:00?

Although I'm testing this from trunk I suspect that this goes way back.

-- JulianLevens - 17 May 2012

Confirmed. This is broken on both trunk and release11 branch.

-- GeorgeClark - 17 May 2012

The bulk of the issue is a check that returns undef if year < 60.
diff --git a/core/lib/Foswiki/Time.pm b/core/lib/Foswiki/Time.pm
index 1c23558..2ab17b0 100644
--- a/core/lib/Foswiki/Time.pm
+++ b/core/lib/Foswiki/Time.pm
@@ -213,7 +213,7 @@ sub parseTime {
         return undef if ( defined($h) && ( $h < 0 || $h > 24 ) );
         return undef if ( defined($m) && ( $m < 0 || $m > 60 ) );
         return undef if ( defined($s) && ( $s < 0 || $s > 60 ) );
-        return undef if ( defined($year) && $year < 60 );
+        #return undef if ( defined($year) && $year < 60 );
 
         my $day  = $D || 1;
         my $hour = $h || 0;

Running your test program after this change, the failure is mostly eliminated, although it depends upon the version of perl in use. (Tweaked to show the epochs when the test fails, or if the difference is > 1 year, or negative.

  • Perl 5.14.2
  • Perl 5.12.4
Undef when year = 11.2012
Failed when year = 63.  -220906800 lt 2903317200 (-3124224000)
Failed when year = 1000.  -30610206000 lt 29316488400 (-59926694400)
Failed when year = 1901.  978325200 lt -2208970800 (3187296000)
Failed when year = 1963.  -220906800 lt 2903317200 (-3124224000)
  • Perl 5.10.1
Day too big - 25202 > 24853
Cannot handle date (0, 0, 0, 01, 0, 2039) at /var/www/foswiki/trunk/core/lib/Foswiki/Time.pm line 223
  • Perl 5.8.8
Day too big - 25202 > 24854
Sec too small - 25202 < 74752
Cannot handle date (0, 0, 0, 01, 0, 2039) at /var/www/foswiki/trunk/core/lib/Foswiki/Time.pm line 223
  • Perl 5.8.4
Cannot handle date (0, 0, 0, 01, 0, 2039) at /var/www/foswiki/trunk/core/lib/Foswiki/Time.pm line 223

-- GeorgeClark - 17 May 2012

George, thanks for looking at this. I was reading the code to properly understand supported date-time formats for my VDBI stuff, and I didn't want the distraction of a bug fix!

I just re-read the code from svn and noticed a few lines now commented out. Not just the diff above but also lines like $year -= 1900 if $year > 1900. However, I also searched for 1900 and found references in the formatting that adds 1900 back (which it now probably shouldn't). However that raises the rather difficult awareness that that there are likely to be various epoch seconds stored in various FW sites out-there that are wrong! Therefore, if you fix the formatting then those dates will be reported incorrectly in the future.

Could you check that out, if it's true that we'll need to ask the community what to do about it! It could be quite a big deal actually as this affects (prior stored) dates > 1960 starting to be reported as 0160 (I think).

Actually, maybe not.

Nobody has reported problems with dates < 1960 or < 60. This implies nobody has been working with dates that 'old'. For any year > 1960, the windowing (a moving target) provide by timelocal/timegm means that years in the range 1963..2062 will be passed as 63..62 by FW, but then treated as 1963..2062 again by timelocal/timegm — phew. The year 2063 would get passed as 0163 by FW which because its in the range 100..999, then timelocal/time adds 1900 back so we get the right result.

This probably explains why we have not been hit by this as a bug before.

Overall, I think it is right to remove the 1900 subtract and add — (it's a bug after all), otherwise someone entering 19nn will be subject to windowing and could be interpreted as 20nn, but it will need careful thinking (and of course tests added for parsing & formatting) to tighten this up.

-- JulianLevens - 21 May 2012

Sorry George, but I'd like your feedback.

-- JulianLevens - 25 May 2012

Julian, I've not done a lot with the time modules. I did make one trunk fix distro:3227f487fdb8 which commented out the 1900 statements you mentioned.

I hate to say it but I think this also ties in with the decision to remain supporting Perl 5.8.4 on 1.1.6, and 5.8.8 on 1.2. I suspect we may have to revert and put back in the y-1900 calculations so that old versions of perl without good date support will still work.

-- GeorgeClark - 25 May 2012

I have to admit I'm not sure why this is assigned to me. I agree with Julian's analysis, that this has side-effects, and it might not be wise fixing. Also, as far as I remember, this code shouldn't be used much. It's used when:
  • A user enters a date in a form field, but it is then stored as epoch (which Sven hates btw)
  • A user tries to sort a table which contains things which look like dates

It is also used in many other places internally, but my point here is to point out that it shouldn't impact existing data, apart from the search. I guess we would need the point of view from other developers, but that would minimize the issue.

I do not like the fix, because, if I remember correctly, we use to have a threshold to know what was a date, and what wasn't. But I guess we cannot do any better anyway, so... why not.

Removing myself from the bug, as I cannot make the decision. We need more input.

-- OlivierRaginel - 01 Sep 2012

With the fixes already applied to trunk (eliminating the year 1900 offset) and the additional patch of returning undef for years < 60, it appears as though this is about as good as we will be able to get without writing our own time routines ... not a good solution. I think the resolution to this should be the patches, plus documentation. Probably it's best to send this to the discussion list for review:

Foswiki Date Handling

Foswiki follows the conventions implemented by recent releases of perl:

  • Correct date handling requires perl 5.12.4 or newer
    • Versions earlier than 5.12.4 use the underlying OS date routines and may vary by platform
    • Dates before 1900 are generally not supported on older implementations.
  • When displaying a character date from the unix epoch, negative epochs are fully supported.
  • When entering dates, 4-digit dates (years > 999) are treated as the year as written.
  • Years 100-999 are treated as an offset from 1900.
    • 110 = 2010. 112 = 2012.
  • 2-digit years are treated as the "rolling century" for the current year +- 50 years.
    • For the current year, 2012:
      • 63-12 look backwards - 1963-2012
      • 13-62 look forwards - 2013-2062

-- GeorgeClark - 27 Sep 2012

This is a can of worms... SpreadSheetPlugin has a very different algorithm. It uses Time::Local similar to Foswiki::Time, but does it's own date rolling, breaking explicitly on "80" instead of a +/- 50 year view, and also subtracts 1900.
        $year += 100 if ( $year < 80 );    # "05"   --> "105" (leave "99" as is)
        $year -= 1900 if ( $year >= 1900 );    # "2005" --> "105"

I'm beginning to suspect that the solution is to use the CPAN:DateTime and CPAN:DateTime::Format::Natural (or one of the other CPAN:DateTime::Format::Builder based routines), but they are quite large.

-- GeorgeClark - 27 Sep 2012

Based upon some of the concerns expressed, that this might be too risky to fix. I'm deferring this to 1.2.

-- GeorgeClark - 22 Oct 2012

This is very much why I used Time::ParseDate with the ActionTrackerPlugin and other plugins. Unfortunately this - and every other time module I have looked at - has some failing somewhere.

My recommendation is to leave well alone; document the limitations of the Foswiki::Time module in handling ambiguous date format, and live with it. George has already written an appropriate section above; just paste it into the doc.

-- CrawfordCurrie - 16 Apr 2013

I agree with the need to really highlight the current failings here very clearly in the docs. I remember thinking that someting useful could be done (even should be done), albeit it will need to leave some legacy behind.

Of course this is one of those apparently trivial things that will take a lot of work and serious attention to detail to get right. So, someone needs to have the spare time and motivation to tackle this, until then documentation is all that's possible.

I certainly hope something better is possible, but ...

-- JulianLevens - 09 Sep 2013

http://foswiki.org/Support/Faq76

Closed

-- CrawfordCurrie - 10 Mar 2014

 

ItemTemplate edit

Summary Foswiki::Time::parseTime doesn't like 1901-1959
ReportedBy JulianLevens
Codebase trunk
SVN Range
AppliesTo Engine
Component FoswikiTime
Priority Urgent
CurrentState Closed
WaitingFor
Checkins distro:daa03ad460aa
TargetRelease minor
ReleasedIn 1.2.0
CheckinsOnBranches trunk
trunkCheckins distro:daa03ad460aa
Release01x01Checkins
Topic revision: r16 - 10 Mar 2014, CrawfordCurrie
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy