Bug #56775 for Net-ISC-DHCPd: 10-leases-parse.t fails in non-UTC timezones

Wed Apr 21 04:58:17 2010 littlesavage [...] orionet.ru - Ticket created

Subject:

10-leases-parse.t fails in non-UTC timezones

10-leases-parse.t fails on non-UTC timezones: t/10-leases-parse.......NOK 5/11 # Failed test 'lease->0 starts' # at t/10-leases-parse.t line 27. # got: '1215963752' # expected: '1215970952' # Failed test 'lease->0 ends' # at t/10-leases-parse.t line 28. # got: '1216050152' # expected: '1216057352' t/10-leases-parse.......NOK 6/11# 104: 0 wallclock secs ( 0.03 usr + 0.00 sys = 0.03 CPU) @ 32.00/s (n=1) # Looks like you failed 2 tests of 11. t/10-leases-parse.......dubious Test returned status 2 (wstat 512, 0x200) DIED. FAILED tests 5-6 Failed 2/11 tests, 81.82% okay patch: --- 10-leases-parse.t.orig 2010-04-21 12:51:11.131276476 +0400 +++ 10-leases-parse.t 2010-04-21 12:52:26.996286135 +0400 @@ -25,8 +25,8 @@ $lease = $leases->leases->[0]; - is($lease->starts, timelocal(32,42,19,14,7-1,2008), "lease->0 starts"); - is($lease->ends, 1216057352, "lease->0 ends"); + is($lease->starts, timelocal(32,42,19,13,7-1,2008), "lease->0 starts"); + is($lease->ends, timelocal(32,42,19,14,7-1,2008), "lease->0 ends"); is($lease->state, "free", "lease->0 binding"); is($lease->hardware_address, "00:15:58:2f:83:bc", "lease->0 hw_ethernet"); is($lease->client_hostname, undef, "lease->0 hostname");

Wed Apr 21 05:01:07 2010 JHTHORSEN [...] cpan.org - Taken

Wed Apr 21 05:05:00 2010 JHTHORSEN [...] cpan.org - Correspondence added

Need to think a bit more about that one... I suggest you install my module with App::cpanminus if this stands in your way from using it: http://search.cpan.org/dist/App-cpanminus/ wget -q http://xrl.us/cpanm -O - | perl - -n Net::ISC::DHCPd

Wed Apr 21 05:05:02 2010 The RT System itself - Status changed from 'new' to 'open'

Sat Apr 24 10:53:37 2010 JHTHORSEN [...] cpan.org - Correspondence added

0.05_01 Sat Apr 24 16:52:43 CEST 2010 - Fix RT56775: 10-leases-parse.t failed in any other timezone than CEST. Contributor: Alexey Illarionov

Sat Apr 24 10:53:38 2010 JHTHORSEN [...] cpan.org - Status changed from 'open' to 'resolved'

Sat Jul 09 00:29:11 2011 gno [...] nwonknu.org - Correspondence added

Hi, I'm reopening this bug because I think the patch is backwards. According to dhcpd.leases(5), dates are stored in UTC. Therefore the portable way to produce a time_t from a dhcpd.leases is by calling timegm. So, to fix, I think first POE::Filter::DHCPd::Lease needs a patch like 110c110 < $lease->{$k} = timelocal(reverse @values); --- Show quoted text

> $lease->{$k} = timegm(reverse @values);

Then this test suite needs a patch like 28,29c28,29 < is($lease->starts, timegm(32, 42, 19, 13, 6, 2008), "lease->0 starts"); < is($lease->ends, timegm(32, 42, 19, 14, 6, 2008), "lease->0 ends"); --- Show quoted text

> is($lease->starts, timelocal(32, 42, 19, 13, 6, 2008), "lease->0

starts"); Show quoted text

> is($lease->ends, timelocal(32, 42, 19, 14, 6, 2008), "lease->0 ends");

This aligns with how Net::Dhcp::Info does things.

Sat Jul 09 00:29:13 2011 The RT System itself - Status changed from 'resolved' to 'open'

Mon Oct 03 15:38:08 2011 JHTHORSEN [...] cpan.org - Correspondence added

I'm not sure about changing this... What is worse? Having the wrong, but consistent value - or changing the value stored? Can I make this backward compat? ...or should it simply get fixed? (Btw: I don't care about Net::DHCP::Info)

Sat Nov 19 02:30:18 2011 gno [...] nwonknu.org - Correspondence added 15 min

RT-Send-CC:

noc [...] trap.mtview.ca.us

On Mon Oct 03 15:38:08 2011, JHTHORSEN wrote: Show quoted text

> I'm not sure about changing this... What is worse? Having the wrong, but > consistent value - or changing the value stored? >

Right is always better than wrong. Anyone using this module outside of UTC, who does testing of their own code and needs this method, isn't using this code, I'd suppose. I'm not, but would prefer to be. Show quoted text

> Can I make this backward compat? ...or should it simply get fixed? >

This could be fixed backward compat; Introduce new methods ->start_NO_TZ_DAMAGE and ->end_NO_TZ_DAMAGE, update documentation to indicate they are preferred over ->start and ->end. Perhaps add warn()'s to callers of ->{start,end}, and set a course to deprecate the broken methods. I can send patches to this effect if need be. Another, less cpan-y approach, I suppose, would be to simply fix the bug, and bump the major version number of your module. Show quoted text

> (Btw: I don't care about Net::DHCP::Info)

I simply mentioned it because I think you authored it, and it does not have this bug, so was useful for comparison purposes. I suppose cpan doesn't have a mechanism for marking modules as orphaned...

Fri Apr 25 15:53:28 2014 RDRAKE [...] cpan.org - Correspondence added

I think this was fixed in POE::Filter::DHCPd::Lease At least I haven't run into it when testing against the dhcpd.leases file in non-UTC. I also tried setting my TZ=UTC to see if it made a difference and it didn't. I just put in a pull request for a fix for another DHCP lease issue that happened intermittently and might be related to your problem. Please let me know if it's still an ongoing issue and I'll reopen and investigate.

Fri Apr 25 15:53:29 2014 RDRAKE [...] cpan.org - Status changed from 'open' to 'resolved'

Fri Apr 25 15:53:29 2014 RDRAKE [...] cpan.org - Fixed in 0.14 added

Bug #56775 for Net-ISC-DHCPd: 10-leases-parse.t fails in non-UTC timezones

Preferred bug tracker