Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.85k stars 527 forks source link

Time::Local module bug #4019

Closed p5pRT closed 20 years ago

p5pRT commented 23 years ago

Migrated from rt.perl.org#7029 (status was 'resolved')

Searchable as RT7029$

p5pRT commented 23 years ago

From uros.juvan@arnes.si

Example where Time​::Local fails miserably​: -- # timelocal fials for example at nonexistant date​: # dd.mm.yyyy hh​:mm​:ss # 30.02.2001 00​:00​:00 use Time​::Local;

print scalar(localtime(timelocal(0\,0\,0\,30\,1\,101)))\, "\n"; -- If I run this I get​: Fri Mar 2 00​:00​:00 2001 which is wrong.

Perl Info ``` Flags: category=library severity=high Site configuration information for perl v5.6.0: Configured by matija at Tue Apr 11 11:07:46 MET DST 2000. Summary of my perl5 (revision 5.0 version 6 subversion 0) configuration: Platform: osname=solaris, osvers=2.7, archname=sun4-solaris uname='sunos rzenik 5.7 generic_106541-08 sun4u sparc sunw,ultra-250 ' config_args='' hint=recommended, useposix=true, d_sigaction=define usethreads=undef use5005threads=undef useithreads=undef usemultiplicity=undef useperlio=undef d_sfio=undef uselargefiles=define use64bitint=undef use64bitall=undef uselongdouble=undef usesocks=undef Compiler: cc='gcc', optimize='-O', gccversion=2.95.1 19990816 (release) cppflags='-fno-strict-aliasing -I/usr/local/include -I/opt/gnu/include' ccflags ='-fno-strict-aliasing -I/usr/local/include -I/opt/gnu/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64' stdchar='char', d_stdstdio=define, usevfork=false intsize=4, longsize=4, ptrsize=4, doublesize=8 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, usemymalloc=y, prototype=define Linker and Libraries: ld='gcc', ldflags =' -L/usr/local/lib -L/opt/gnu/lib ' libpth=/usr/local/lib /opt/gnu/lib /lib /usr/lib /usr/ccs/lib libs=-lsocket -lnsl -lgdbm -ldb -ldl -lm -lc -lcrypt -lsec libc=/lib/libc.so, so=so, useshrplib=false, libperl=libperl.a Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags=' ' cccdlflags='-fPIC', lddlflags='-G -L/usr/local/lib -L/opt/gnu/lib' Locally applied patches: @INC for perl v5.6.0: /opt/gnu/lib/perl5/5.6.0/sun4-solaris /opt/gnu/lib/perl5/5.6.0 /opt/gnu/lib/perl5/site_perl/5.6.0/sun4-solaris /opt/gnu/lib/perl5/site_perl/5.6.0 /opt/gnu/lib/perl5/site_perl/5.005/sun4-solaris /opt/gnu/lib/perl5/site_perl/5.005 /opt/gnu/lib/perl5/site_perl . Environment for perl v5.6.0: HOME=/export/home/urosj LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH=/usr/openwin/lib:/opt/SUNWmotif/lib LOGDIR (unset) PATH=/export/home/urosj/apps/bin:/sbin:/usr/sbin:/usr/bin:/etc:/usr/ccs/bin:/usr/ucb:/usr/openwin/bin:/usr/local/bin:/opt/gnu/bin:/opt/mysql/bin PERL_BADLANG (unset) SHELL=/bin/bash ```
p5pRT commented 23 years ago

From @schwern

Feburary 30th?

The range checks in Time​::Local are a bit primitive...

  croak "Day '$_[3]' out of range 1..31" if $_[3] > 31 || $_[3] \< 1;

This could be beefed up very easily.

p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

Jarkko and I exchanged some mail about this when he was banging on Time​::Piece. I'd like to suggest there are two distinct flavors of error possible. One is something like February 30\, above\, which will never be valid. One might expect a fair amount of computation to detect logical inconsistencies such as this. The other is reaching a date that *would* be valid\, but is too large or too small to fit in the prevailing time_t. One would like to detect this\, but it should cost much less than a full-blown consistency check on each component. If one is willing to assume responsibility for keeping the components consistent\, it should be possible (via some interface) to avoid the extra overhead -- jpl

p5pRT commented 23 years ago

From @schwern

Would much else be needed than just...

@​Days_In_Month = ( [31\,28\,31\,30\,31\,30\,31\,31\,30\,31\,30\,31]\, [31\,29\,31\,30\,31\,30\,31\,31\,30\,31\,30\,31]\, );

my $is_leap = ($year % 4==0) and ($year % 400 == 0 or $year % 100 != 0); my $max_day = $Days_In_Month[$is_leap][$mon];

croak "Day '$mday' out of range 1..$max_day" unless   1 \< $mday && $mday \<= $max_day;

to beef up the month-day check?

The other is reaching a date that *would* be valid\, but is too large or too small to fit in the prevailing time_t.

That's easy\, too.

# Can I get the max size of time_t from some better source? # POSIX? Config? my @​Time_Bounds = (   [localtime(2**31 - 1)]\,   [localtime(-2**31)]\, );

unless( $Time_Bounds[0][5] \< $year && $year \< $Time_Bounds[1][5] ) {   # exercise for the reader }

I suppose I should be patching this.

p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

A little less primitive. I'm not sure its worth the effort to take care of leap years correctly\, but it wouldn't be difficult​:

Inline Patch ```diff --- Local.pm Wed Dec 6 10:10:17 2000 +++ Local.pm.new Tue May 22 11:35:01 2001 @@ -4,7 +4,7 @@ use Carp; use strict; -our $VERSION = '1.00'; +our $VERSION = '1.01'; our @ISA = qw( Exporter ); our @EXPORT = qw( timegm timelocal ); our @EXPORT_OK = qw( timegm_nocheck timelocal_nocheck ); @@ -87,12 +87,21 @@ sub cheat { my($ym, @date) = @_; my($sec, $min, $hour, $day, $month, $year) = @date; + my($md); unless ($Options{no_range_check}) { + if ($month == 1) { + $md = 29; # February + } elsif ($month == 2 || $month == 4 || $month == 6 || + $month == 9 || $month == 11) { + $md = 30; # March, May, July, September, November + } else { + $md = 31; # The rest + } croak "Month '$month' out of range 0..11" if $month > 11 || $month < 0; - croak "Day '$day' out of range 1..31" if $day > 31 || $day < 1; - croak "Hour '$hour' out of range 0..23" if $hour > 23 || $hour < 0; - croak "Minute '$min' out of range 0..59" if $min > 59 || $min < 0; - croak "Second '$sec' out of range 0..59" if $sec > 59 || $sec < 0; + croak "Day '$day' out of range 1..$md" if $day > $md || $day < 1; + croak "Hour '$hour' out of range 0..23" if $hour > 23 || $hour < 0; + croak "Minute '$min' out of range 0..59" if $min > 59 || $min < 0; + croak "Second '$sec' out of range 0..59" if $sec > 59 || $sec < 0; } my $guess = $^T; my @g = gmtime($guess); @@ -151,8 +160,8 @@ January 1, 1970). This value can be positive or negative. It is worth drawing particular attention to the expected ranges for -the values provided. While the day of the month is expected to be in -the range 1..31, the month should be in the range 0..11. +the values provided. The value for the day of the month is the actual day +(ie 1..31), while the month is the number of month since January ( 0..11). This is consistent with the values returned from localtime() and gmtime(). The timelocal() and timegm() functions perform range checking on the ```
p5pRT commented 23 years ago

From @tamias

You appear to have forgotten the -1 for September and November.

But why not just​:

$md = (31\, 29\, 31\, 30\, 31\, 30\, 31\, 30\, 30\, 31\, 30\, 31)[$month];

?

@​@​ -151\,8 +160\,8 @​@​ January 1\, 1970). This value can be positive or negative.

It is worth drawing particular attention to the expected ranges for -the values provided. While the day of the month is expected to be in -the range 1..31\, the month should be in the range 0..11.
+the values provided. The value for the day of the month is the actual day +(ie 1..31)\, while the month is the number of month since January ( 0..11).

That should be​:

  while the month is the number of months since January (0..11).

with an s added and a space removed.

Ronald

p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

Because August has 31 days?

  $md = (31\, 29\, 31\, 30\, 31\, 30\, 31\, 31\, 30\, 31\, 30\, 31)[$month];   # J F M A M J J A S O N D

Chris

p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

You achieve the effect of covering both error conditions in a single call\, letting localtime handle the requisite hassles...

D

At 11​:56 AM -0400 5/22/01\, Horsley Tom wrote​:

As an alternate direction for all this​: You could just always run the output from timelocal back through localtime and see if you get the same values\, if not\, warn about them (that should take care of all possible errors\, at least it seems like it should :-).

p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

?

But why not just​:
$md = (31\, 29\, 31\, 30\, 31\, 30\, 31\, 30\, 30\, 31\, 30\, 31)[$month];

Because mine is more self documenting? Because mine is easier to modify if someone decides they want to worry about leap years? Because I didn't think of it? The third one is probably the most truthful. ;-)

| with an s added and a space removed.

eeeks.. Yes. Bad fingers.

-spp

p5pRT commented 23 years ago

From @tamias

9 and 11 are October and December. :)

Ronald

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

What has 2**31 to do with it? Plenty of platforms support wider ranges. And negative time_t values are distinctly nonportable.

And in any case\, there's no reason why Perl should be limited by the size limits of the underlying platform.

Mike Guy

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

More generous limits are always welcome. At the root of my concern is a bunch of date manipulation routines (a couple included at the end). Given that the midnight sub establishes its date components using localtime\, I was surprised to have timelocal croak​:

Can't handle date (0\, 0\, 0\, 11\, 6\, 142)

There's nothing syntactically wrong with the components\, so there's no (easy) way to tell it's going to cause timelocal to croak. I'd prefer not to pay the overhead of having the components checked\, and I *really* dislike the need to do an eval to catch the croak. -- jpl

# Just adding 24 * 60 * 60 seconds doesn't always get you # to the next day (as we discovered when daylight saving time ended). # The safe thing to do is start from a known time\, overcompensate\, # then restore to midnight\, for consistency.

sub midnight {   my $time = $_[0];   my ($sec\, $min\, $hour\, $mday\, $mon\, $year\, $wday) = localtime($time);   timelocal(0\, 0\, 0\, $mday\, $mon\, $year); }

sub day_before {   my $time = midnight($_[0]);   midnight($time - 12 * 60 * 60); }

sub day_after {   my $time = midnight($_[0]);   midnight($time + 36 * 60 * 60); }

p5pRT commented 22 years ago

From @schwern

*cough* That's what the "Can I get the max size of time_t from some better source?" comment was about.

p5pRT commented 22 years ago

From @schwern

Given how much work Time​::Local has to do normally\, adding in a some O(1) validations at the beginning won't alter performance. I'd almost guarantee it.

Can't handle date (0\, 0\, 0\, 11\, 6\, 142)

There's nothing syntactically wrong with the components\, so there's no (easy) way to tell it's going to cause timelocal to croak.

and I *really* dislike the need to do an eval to catch the croak.

Aside from changing the world over to 64bit time_t\, or having Time​::Local roll all its own date handling routines (which would *really* kill performance) how do we handle this other than dying/throwing an exception?

For many machines\, the universe ends in the wee hours of the morning on Tuesday\, January 19th\, 2038. We would have to write a whole new time handling system for perl completely independent of the standard C library to change this. This may be useful\, but its a bit of an overkill when we'll get the same effect by just waiting for people to upgrade time_t.

p5pRT commented 22 years ago

From @gbarr

$Config{'intsize'} would probably be a good guess.

Graham.

p5pRT commented 22 years ago

From @schwern

Hmmm\, anyone actually running on a platform where "print scalar localtime(2**48)" gives something meaningful? What's $Config{intsize} say? It still doesn't tell us if time_t is signed or unsigned. :(

Alternatively\, Time​::Local can make some intellegent guesses by creeping up the size of time (0\, 2**31-1\, then 2**31\, then 2**32-1\, 2**32\, 2**47-1\, etc...) as well as the lower bound (0\, (-2**31)-1\, etc...) making sure it always gets a higher (or lower) result.

Does all this strike anyone as overkill?

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

If you really need to know then presumably same techniques Configure used to find intsize can be applied to time_t.

Alternatively\, Time​::Local can make some intellegent guesses by creeping up the size of time (0\, 2**31-1\, then 2**31\, then 2**32-1\, 2**32\, 2**47-1\, etc...) as well as the lower bound (0\, (-2**31)-1\, etc...) making sure it always gets a higher (or lower) result.

Does all this strike anyone as overkill?

Yup.

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

My personal hangup with Time​::Local has more to do with craoking than with the level of checking. If there were an interface\, presumable shared by timelocal()\, that would return undef instead of croaking\, then it wouldn't be necessary to jump through hoops to survive dirty data or stretched limits. -- jpl

p5pRT commented 22 years ago

From @schwern

So instead of this complicated code​:

  my $time = eval { timelocal(@​time); };   if( $@​ ) {   print "timelocal failed because $@​";   ...do some sort of recovery...   }

you want the much simpler​:

  my $time = timelocal(...);   unless( defined $time ) {   print "timelocal failed for some reason";   ...do some sort of recovery...   }

;)

p5pRT commented 22 years ago

From [Unknown Contact. See original ticket]

I thought the eval overhead might matter. Michael is right\, the overhead of timelocal itself pretty much dominates. I'll stop worrying. -- jpl

Benchmark​: running evaled\, inline\, each for at least 10 CPU seconds...   evaled​: 11 wallclock secs (10.45 usr + 0.03 sys = 10.48 CPU) @​ 9093.99/s (n=95305)   inline​: 11 wallclock secs (10.36 usr + 0.04 sys = 10.40 CPU) @​ 9357.21/s (n=97315)