From 5149153fe8c63cac98b43e24e33f229e1beb9f0c Mon Sep 17 00:00:00 2001 From: Kevin Falcone Date: Wed, 14 May 2014 17:05:28 -0400 Subject: [PATCH 1/4] Add a new RT::Date->IsSet method This lets you never try to remember if it was ISO or Unix that was best, and what about timezones? Just have a simple boolean check. --- lib/RT/Date.pm | 14 ++++++++++++++ t/api/date.t | 4 ++++ 2 files changed, 18 insertions(+) diff --git a/lib/RT/Date.pm b/lib/RT/Date.pm index d99c26f6072..8a2dfa0c81c 100644 --- a/lib/RT/Date.pm +++ b/lib/RT/Date.pm @@ -1135,6 +1135,20 @@ sub Timezone { return $tz; } +=head3 IsSet + +Returns true if this Date is set in the database, otherwise returns a false value. + +This avoids needing to compare to 1970-01-01 in any of your code + +=cut + +sub IsSet { + my $self = shift; + return $self->Unix ? 1 : 0; + +} + RT::Base->_ImportOverlays(); diff --git a/t/api/date.t b/t/api/date.t index 1619d004335..ec04b5a1fca 100644 --- a/t/api/date.t +++ b/t/api/date.t @@ -83,6 +83,7 @@ my $current_user; { my $date = RT::Date->new(RT->SystemUser); + is($date->IsSet,0,"new date isn't set"); is($date->Unix, 0, "new date returns 0 in Unix format"); is($date->Get, '1970-01-01 00:00:00', "default is ISO format"); warning_like { @@ -207,6 +208,7 @@ my $current_user; $current_user->UserObj->__Set( Field => 'Timezone', Value => 'Europe/Moscow'); my $date = RT::Date->new( $current_user ); $date->Set( Format => 'ISO', Timezone => 'utc', Value => '2005-01-01 15:10:00' ); + is($date->IsSet,1,"Date has been set"); is($date->ISO( Timezone => 'user' ), '2005-01-01 18:10:00', "ISO"); is($date->W3CDTF( Timezone => 'user' ), '2005-01-01T18:10:00+03:00', "W3C DTF"); is($date->RFC2822( Timezone => 'user' ), 'Sat, 01 Jan 2005 18:10:00 +0300', "RFC2822"); @@ -251,10 +253,12 @@ warning_like foreach (undef, 0, ''){ $date->Unix(1); is($date->ISO, '1970-01-01 00:00:01', "correct value"); + is($date->IsSet,1,"Date has been set to a value"); $date->Set(Format => 'unix', Value => $_); is($date->ISO, '1970-01-01 00:00:00', "Set a date to midnight 1/1/1970 GMT due to wrong call"); is($date->Unix, 0, "unix is 0 => unset"); + is($date->IsSet,0,"Date has been unset"); } } From b4c54fafda0fe31aa47df381f4ae9d5ae5f1add2 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Thu, 15 May 2014 17:49:41 -0400 Subject: [PATCH 2/4] Use the new ->IsSet method, rather than checking vs ->Unix --- lib/RT/Action/CreateTickets.pm | 2 +- lib/RT/Action/EscalatePriority.pm | 2 +- lib/RT/Action/LinearEscalate.pm | 9 ++- lib/RT/Condition/BeforeDue.pm | 2 +- lib/RT/Condition/Overdue.pm | 2 +- lib/RT/Date.pm | 2 +- lib/RT/Interface/Web.pm | 4 +- lib/RT/SearchBuilder.pm | 2 +- lib/RT/Ticket.pm | 2 +- .../html/Approvals/Elements/PendingMyApproval | 4 +- share/html/Elements/RT__Ticket/ColumnMap | 2 +- share/html/Elements/ShowReminders | 2 +- share/html/Elements/ValidateCustomFields | 2 +- share/html/NoAuth/iCal/dhandler | 4 +- share/html/Ticket/Create.html | 2 +- share/html/Ticket/Elements/Reminders | 2 +- share/html/Ticket/Elements/ShowDates | 2 +- share/html/m/ticket/create | 2 +- share/html/m/ticket/show | 2 +- t/api/date.t | 18 +++--- t/api/ticket.t | 2 +- t/lifecycles/dates.t | 56 +++++++++---------- t/security/CVE-2011-5092-graph-links.t | 4 +- 23 files changed, 65 insertions(+), 66 deletions(-) diff --git a/lib/RT/Action/CreateTickets.pm b/lib/RT/Action/CreateTickets.pm index a88a6ee71f8..5fa64f9c030 100644 --- a/lib/RT/Action/CreateTickets.pm +++ b/lib/RT/Action/CreateTickets.pm @@ -691,7 +691,7 @@ sub ParseLines { eval { $dateobj->Set( Format => 'iso', Value => $args{$date} ); }; - if ($@ or $dateobj->Unix <= 0) { + if ($@ or not $dateobj->IsSet) { $dateobj->Set( Format => 'unknown', Value => $args{$date} ); } } diff --git a/lib/RT/Action/EscalatePriority.pm b/lib/RT/Action/EscalatePriority.pm index 046d6ddbbd6..344604c8c04 100644 --- a/lib/RT/Action/EscalatePriority.pm +++ b/lib/RT/Action/EscalatePriority.pm @@ -104,7 +104,7 @@ sub Prepare { # If we don't have a due date, adjust the priority by one # until we hit the final priority - if ($due->Unix() < 1) { + if (not $due->IsSet) { if ( $self->TicketObj->Priority > $self->TicketObj->FinalPriority ){ $self->{'prio'} = ($self->TicketObj->Priority - 1); return 1; diff --git a/lib/RT/Action/LinearEscalate.pm b/lib/RT/Action/LinearEscalate.pm index e6260aa148e..fb2f6ea414b 100644 --- a/lib/RT/Action/LinearEscalate.pm +++ b/lib/RT/Action/LinearEscalate.pm @@ -155,8 +155,7 @@ sub Prepare { my $ticket = $self->TicketObj; - my $due = $ticket->DueObj->Unix; - unless ( $due > 0 ) { + unless ( $ticket->DueObj->IsSet ) { $RT::Logger->debug('Due is not set. Not escalating.'); return 1; } @@ -181,9 +180,8 @@ sub Prepare { # now we know we have a due date. for every day that passes, # increment priority according to the formula - my $starts = $ticket->StartsObj->Unix; - $starts = $ticket->CreatedObj->Unix unless $starts > 0; - my $now = time; + my $starts = $ticket->StartsObj->IsSet ? $ticket->StartsObj->Unix : $ticket->CreatedObj->Unix; + my $now = time; # do nothing if we didn't reach starts or created date if ( $starts > $now ) { @@ -191,6 +189,7 @@ sub Prepare { return 1; } + my $due = $ticket->DueObj->Unix; $due = $starts + 1 if $due <= $starts; # +1 to avoid div by zero my $percent_complete = ($now-$starts)/($due - $starts); diff --git a/lib/RT/Condition/BeforeDue.pm b/lib/RT/Condition/BeforeDue.pm index 7e5f1f75f31..fa7f364e5ee 100644 --- a/lib/RT/Condition/BeforeDue.pm +++ b/lib/RT/Condition/BeforeDue.pm @@ -86,7 +86,7 @@ sub IsApplicable { my $cur = RT::Date->new( RT->SystemUser ); $cur->SetToNow(); my $due = $self->TicketObj->DueObj; - return (undef) if $due->Unix <= 0; + return (undef) unless $due->IsSet; my $diff = $due->Diff($cur); if ( $diff >= 0 and $diff <= $elapse ) { diff --git a/lib/RT/Condition/Overdue.pm b/lib/RT/Condition/Overdue.pm index bf044f8c0db..444e373e1b4 100644 --- a/lib/RT/Condition/Overdue.pm +++ b/lib/RT/Condition/Overdue.pm @@ -70,7 +70,7 @@ If the due date is before "now" return true sub IsApplicable { my $self = shift; - if ($self->TicketObj->DueObj->Unix > 0 and + if ($self->TicketObj->DueObj->IsSet and $self->TicketObj->DueObj->Unix < time()) { return(1); } diff --git a/lib/RT/Date.pm b/lib/RT/Date.pm index 8a2dfa0c81c..ccd23aee87f 100644 --- a/lib/RT/Date.pm +++ b/lib/RT/Date.pm @@ -463,7 +463,7 @@ sub AsString { my $self = shift; my %args = (@_); - return $self->loc("Not set") unless $self->Unix > 0; + return $self->loc("Not set") unless $self->IsSet; my $format = RT->Config->Get( 'DateTimeFormat', $self->CurrentUser ) || 'DefaultFormat'; $format = { Format => $format } unless ref $format; diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm index 439255c7d87..c1eb0e19f9b 100644 --- a/lib/RT/Interface/Web.pm +++ b/lib/RT/Interface/Web.pm @@ -2968,7 +2968,7 @@ sub ProcessTicketReminders { Format => 'unknown', Value => $due, ); - if ( defined $DateObj->Unix + if ( $DateObj->IsSet && $DateObj->Unix != $reminder->DueObj->Unix ) { ( $status, $msg ) = $reminder->SetDue( $DateObj->ISO ); @@ -3429,7 +3429,7 @@ sub ProcessTicketDates { ); my $obj = $field . "Obj"; - if ( ( defined $DateObj->Unix ) + if ( ( $DateObj->IsSet ) and ( $DateObj->Unix != $Ticket->$obj()->Unix() ) ) { my $method = "Set$field"; diff --git a/lib/RT/SearchBuilder.pm b/lib/RT/SearchBuilder.pm index aaff9f14681..b8b2c2f203d 100644 --- a/lib/RT/SearchBuilder.pm +++ b/lib/RT/SearchBuilder.pm @@ -604,7 +604,7 @@ sub _LimitCustomField { } elsif ( $type =~ /^Date(?:Time)?$/ ) { my $date = RT::Date->new( $self->CurrentUser ); $date->Set( Format => 'unknown', Value => $value ); - if ( $date->Unix ) { + if ( $date->IsSet ) { if ( $type eq 'Date' # Heuristics to determine if a date, and not diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm index c80427001f5..46577e4519d 100644 --- a/lib/RT/Ticket.pm +++ b/lib/RT/Ticket.pm @@ -2376,7 +2376,7 @@ sub _SetStatus { if ( $args{SetStarted} && $args{Lifecycle}->IsInitial($old) && !$args{NewLifecycle}->IsInitial($args{Status}) - && !$raw_started->Unix) { + && !$raw_started->IsSet) { # Set the Started time to "now" $self->_Set( Field => 'Started', diff --git a/share/html/Approvals/Elements/PendingMyApproval b/share/html/Approvals/Elements/PendingMyApproval index 96d3e860a91..c77a021d561 100644 --- a/share/html/Approvals/Elements/PendingMyApproval +++ b/share/html/Approvals/Elements/PendingMyApproval @@ -70,9 +70,9 @@ />
-<&|/l_unsafe, qq{"&>Only show approvals for requests created before [_1]
+<&|/l_unsafe, qq{"&>Only show approvals for requests created before [_1]
-<&|/l_unsafe, qq{"&>Only show approvals for requests created after [_1] +<&|/l_unsafe, qq{"&>Only show approvals for requests created after [_1] <%init> diff --git a/share/html/Elements/RT__Ticket/ColumnMap b/share/html/Elements/RT__Ticket/ColumnMap index 359a79da96f..b95a9ba1100 100644 --- a/share/html/Elements/RT__Ticket/ColumnMap +++ b/share/html/Elements/RT__Ticket/ColumnMap @@ -207,7 +207,7 @@ $COLUMN_MAP = { value => sub { my $date = $_[0]->DueObj; # Highlight the date if it was due in the past, and it's still active - if ( $date && $date->Unix > 0 && $date->Diff < 0 && $_[0]->QueueObj->IsActiveStatus($_[0]->Status)) { + if ( $date && $date->IsSet && $date->Diff < 0 && $_[0]->QueueObj->IsActiveStatus($_[0]->Status)) { return (\'' , $date->AgeAsString , \''); } else { return $date->AgeAsString; diff --git a/share/html/Elements/ShowReminders b/share/html/Elements/ShowReminders index 85b2b70e4cd..aa2346cd8ee 100644 --- a/share/html/Elements/ShowReminders +++ b/share/html/Elements/ShowReminders @@ -57,7 +57,7 @@ my $i =0; while ( my $reminder = $reminders->Next ) { $i++; my $dueobj = $reminder->DueObj; -my $overdue = $dueobj->Unix > 0 && $dueobj->Diff < 0 ? 1 : 0; +my $overdue = $dueobj->IsSet && $dueobj->Diff < 0 ? 1 : 0; my $targets = RT::Tickets->new($session{'CurrentUser'}); $targets->{'allow_deleted_search'} = 1; diff --git a/share/html/Elements/ValidateCustomFields b/share/html/Elements/ValidateCustomFields index 43f8ebd0b78..fc0049bb9a8 100644 --- a/share/html/Elements/ValidateCustomFields +++ b/share/html/Elements/ValidateCustomFields @@ -88,7 +88,7 @@ while ( my $CF = $CustomFields->Next ) { Value => $_, ($CF->Type eq "Date" ? (Timezone => 'utc') : ()) ); - $DateObj->Unix > 0 + $DateObj->IsSet } @values; } push @values, '' unless @values; diff --git a/share/html/NoAuth/iCal/dhandler b/share/html/NoAuth/iCal/dhandler index f7e51d59e05..e319754e07c 100644 --- a/share/html/NoAuth/iCal/dhandler +++ b/share/html/NoAuth/iCal/dhandler @@ -86,9 +86,9 @@ $feed->add_properties('method' => ['publish']); $feed->add_properties('prodid' => ["-//" . RT->Config->Get('rtname') ."//"]); while (my $t = $tickets->Next) { - next unless $t->DueObj->Unix > 0; + next unless $t->DueObj->IsSet; - my $starttime = $t->StartsObj->Unix > 0 ? $t->StartsObj : $t->CreatedObj; + my $starttime = $t->StartsObj->IsSet ? $t->StartsObj : $t->CreatedObj; my $url; if ( RT->Config->Get('CanonicalizeURLsInFeeds') ) { diff --git a/share/html/Ticket/Create.html b/share/html/Ticket/Create.html index 57e66df7290..e1a2ea36b3b 100644 --- a/share/html/Ticket/Create.html +++ b/share/html/Ticket/Create.html @@ -312,7 +312,7 @@ Status TimeLeft/; $clone->{$_} = $CloneTicketObj->$_->AsString - for grep { $CloneTicketObj->$_->Unix } + for grep { $CloneTicketObj->$_->IsSet } map { $_ . "Obj" } qw/Starts Started Due Resolved/; my $get_link_value = sub { diff --git a/share/html/Ticket/Elements/Reminders b/share/html/Ticket/Elements/Reminders index 0ce30abf377..5e61b1d8304 100644 --- a/share/html/Ticket/Elements/Reminders +++ b/share/html/Ticket/Elements/Reminders @@ -190,7 +190,7 @@ $Ticket $Index % my $dueobj = $Reminder->DueObj; -% my $overdue = $dueobj->Unix > 0 && $dueobj->Diff < 0 ? 1 : 0; +% my $overdue = $dueobj->IsSet && $dueobj->Diff < 0 ? 1 : 0; diff --git a/share/html/Ticket/Elements/ShowDates b/share/html/Ticket/Elements/ShowDates index 986507b88ea..9c8f9855145 100644 --- a/share/html/Ticket/Elements/ShowDates +++ b/share/html/Ticket/Elements/ShowDates @@ -65,7 +65,7 @@ <&|/l&>Due:\ % my $due = $Ticket->DueObj; -% if ( $due && $due->Unix > 0 && $due->Diff < 0 ) { +% if ( $due && $due->IsSet && $due->Diff < 0 ) { <% $due->AsString %> % } else { <% $due->AsString %> diff --git a/share/html/m/ticket/create b/share/html/m/ticket/create index af25bd09873..42ebbec4342 100644 --- a/share/html/m/ticket/create +++ b/share/html/m/ticket/create @@ -87,7 +87,7 @@ if ($CloneTicket) { Status TimeLeft/; $clone->{$_} = $CloneTicketObj->$_->AsString - for grep { $CloneTicketObj->$_->Unix } + for grep { $CloneTicketObj->$_->IsSet } map { $_ . "Obj" } qw/Starts Started Due Resolved/; my $get_link_value = sub { diff --git a/share/html/m/ticket/show b/share/html/m/ticket/show index bcc7fde7360..ddc532f5f7b 100644 --- a/share/html/m/ticket/show +++ b/share/html/m/ticket/show @@ -365,7 +365,7 @@ my $print_value = sub {
<&|/l&>Due:
% my $due = $Ticket->DueObj; -% if ( $due && $due->Unix > 0 && $due->Diff < 0 ) { +% if ( $due && $due->IsSet && $due->Diff < 0 ) {
<% $due->AsString %>
% } else {
<% $due->AsString %>
diff --git a/t/api/date.t b/t/api/date.t index ec04b5a1fca..1d746f81d45 100644 --- a/t/api/date.t +++ b/t/api/date.t @@ -241,7 +241,7 @@ warning_like { # bad format my $date = RT::Date->new(RT->SystemUser); $date->Set( Format => 'bad' ); - is($date->Unix, 0, "bad format"); + ok(!$date->IsSet, "bad format"); } qr{Unknown Date format: bad}; @@ -269,7 +269,7 @@ my $year = (localtime(time))[5] + 1900; warning_like { $date->Set(Format => 'ISO', Value => 'weird date'); } qr/Couldn't parse date 'weird date' as a ISO format/; - is($date->Unix, 0, "date was wrong => unix == 0"); + ok(!$date->IsSet, "date was wrong => unix == 0"); # XXX: ISO format has more feature than we suport # http://www.cl.cam.ac.uk/~mgk25/iso-time.html @@ -301,15 +301,15 @@ my $year = (localtime(time))[5] + 1900; warning_like { $date->Set(Format => 'ISO', Value => '2005-13-28 15:10:00'); } qr/Invalid date/; - is($date->Unix, 0, "wrong month value"); + ok(!$date->IsSet, "wrong month value"); warning_like { $date->Set(Format => 'ISO', Value => '2005-00-28 15:10:00'); } qr/Invalid date/; - is($date->Unix, 0, "wrong month value"); + ok(!$date->IsSet, "wrong month value"); $date->Set(Format => 'ISO', Value => '1960-01-28 15:10:00'); - is($date->Unix, 0, "too old, we don't support"); + ok(!$date->IsSet, "too old, we don't support"); } { # set+datemanip format(Time::ParseDate) @@ -334,7 +334,7 @@ my $year = (localtime(time))[5] + 1900; warnings_like { $date->Set(Format => 'unknown', Value => 'weird date'); } qr{Couldn't parse date 'weird date' by Time::ParseDate}; - is($date->Unix, 0, "date was wrong"); + ok(!$date->IsSet, "date was wrong"); RT->Config->Set( Timezone => 'Europe/Moscow' ); $date->Set(Format => 'unknown', Value => '2005-11-28 15:10:00'); @@ -586,13 +586,13 @@ my $year = (localtime(time))[5] + 1900; # set unknown format: edge cases my $date = RT::Date->new(RT->SystemUser); $date->Set( Value => 0, Format => 'unknown' ); - is( $date->Unix(), 0, "unix is 0 with Value => 0, Format => 'unknown'" ); + ok( !$date->IsSet, "unix is 0 with Value => 0, Format => 'unknown'" ); $date->Set( Value => '', Format => 'unknown' ); - is( $date->Unix(), 0, "unix is 0 with Value => '', Format => 'unknown'" ); + ok( !$date->IsSet, "unix is 0 with Value => '', Format => 'unknown'" ); $date->Set( Value => ' ', Format => 'unknown' ); - is( $date->Unix(), 0, "unix is 0 with Value => ' ', Format => 'unknown'" ); + ok( !$date->IsSet, "unix is 0 with Value => ' ', Format => 'unknown'" ); } #TODO: AsString diff --git a/t/api/ticket.t b/t/api/ticket.t index f1743148a9e..c5f1e240fb0 100644 --- a/t/api/ticket.t +++ b/t/api/ticket.t @@ -100,7 +100,7 @@ ok( $t->Create(Queue => 'General', Due => '2002-05-21 00:00:00', ReferredToBy => ok ( my $id = $t->Id, "Got ticket id"); like ($t->RefersTo->First->Target , qr/fsck.com/, "Got refers to"); like ($t->ReferredToBy->First->Base , qr/cpan.org/, "Got referredtoby"); -is ($t->ResolvedObj->Unix, 0, "It hasn't been resolved - ". $t->ResolvedObj->Unix); +ok (!$t->ResolvedObj->IsSet, "It hasn't been resolved"); } diff --git a/t/lifecycles/dates.t b/t/lifecycles/dates.t index af81d9e2aa7..0c74a1b213b 100644 --- a/t/lifecycles/dates.t +++ b/t/lifecycles/dates.t @@ -42,8 +42,8 @@ diag "dates on create for default schema"; Status => 'new', ); ok $id, 'created a ticket'; - ok $ticket->StartedObj->Unix <= 0, 'started is not set'; - ok $ticket->ResolvedObj->Unix <= 0, 'resolved is not set'; + ok !$ticket->StartedObj->IsSet, 'started is not set'; + ok !$ticket->ResolvedObj->IsSet, 'resolved is not set'; } { my $ticket = RT::Ticket->new( RT->SystemUser ); @@ -53,8 +53,8 @@ diag "dates on create for default schema"; Status => 'open', ); ok $id, 'created a ticket'; - ok $ticket->StartedObj->Unix > 0, 'started is set'; - ok $ticket->ResolvedObj->Unix <= 0, 'resolved is not set'; + ok $ticket->StartedObj->IsSet, 'started is set'; + ok !$ticket->ResolvedObj->IsSet, 'resolved is not set'; } { my $ticket = RT::Ticket->new( RT->SystemUser ); @@ -64,8 +64,8 @@ diag "dates on create for default schema"; Status => 'resolved', ); ok $id, 'created a ticket'; - ok $ticket->StartedObj->Unix > 0, 'started is set'; - ok $ticket->ResolvedObj->Unix > 0, 'resolved is set'; + ok $ticket->StartedObj->IsSet, 'started is set'; + ok $ticket->ResolvedObj->IsSet, 'resolved is set'; } my $test_date = '2008-11-28 12:00:00'; @@ -135,7 +135,7 @@ diag "dates on create for delivery schema"; is $ticket->Status, 'ordered', "Status is ordered"; my ($statusval,$statusmsg) = $ticket->SetStatus('on way'); ok($statusval,$statusmsg); - ok $ticket->StartedObj->Unix > 0, 'started is set to ' .$ticket->StartedObj->AsString ; + ok $ticket->StartedObj->IsSet, 'started is set to ' .$ticket->StartedObj->AsString ; is $ticket->ResolvedObj->Unix, 0, 'resolved is not set'; } { @@ -152,8 +152,8 @@ diag "dates on create for delivery schema"; ($statusval,$statusmsg) = $ticket->SetStatus('delivered'); ok($statusval,$statusmsg); - ok $ticket->StartedObj->Unix > 0, 'started is set'; - ok $ticket->ResolvedObj->Unix > 0, 'resolved is set'; + ok $ticket->StartedObj->IsSet, 'started is set'; + ok $ticket->ResolvedObj->IsSet, 'resolved is set'; } my $test_date = '2008-11-28 12:00:00'; @@ -215,30 +215,30 @@ diag "dates on status change for default schema"; Status => 'new', ); ok $id, 'created a ticket'; - ok $ticket->StartedObj->Unix <= 0, 'started is not set'; - ok $ticket->ResolvedObj->Unix <= 0, 'resolved is not set'; + ok !$ticket->StartedObj->IsSet, 'started is not set'; + ok !$ticket->ResolvedObj->IsSet, 'resolved is not set'; (my $status, $msg) = $ticket->SetStatus('open'); ok $status, 'changed status' or diag "error: $msg"; - ok $ticket->StartedObj->Unix > 0, 'started is set'; - ok $ticket->ResolvedObj->Unix <= 0, 'resolved is not set'; + ok $ticket->StartedObj->IsSet, 'started is set'; + ok !$ticket->ResolvedObj->IsSet, 'resolved is not set'; my $started = $ticket->StartedObj->Unix; ($status, $msg) = $ticket->SetStatus('stalled'); ok $status, 'changed status' or diag "error: $msg"; is $ticket->StartedObj->Unix, $started, 'started is set and the same'; - ok $ticket->ResolvedObj->Unix <= 0, 'resolved is not set'; + ok !$ticket->ResolvedObj->IsSet, 'resolved is not set'; ($status, $msg) = $ticket->SetStatus('open'); ok $status, 'changed status' or diag "error: $msg"; is $ticket->StartedObj->Unix, $started, 'started is set and the same'; - ok $ticket->ResolvedObj->Unix <= 0, 'resolved is not set'; + ok !$ticket->ResolvedObj->IsSet, 'resolved is not set'; ($status, $msg) = $ticket->SetStatus('resolved'); ok $status, 'changed status' or diag "error: $msg"; is $ticket->StartedObj->Unix, $started, 'started is set and the same'; - ok $ticket->ResolvedObj->Unix > 0, 'resolved is set'; + ok $ticket->ResolvedObj->IsSet, 'resolved is set'; } diag "dates on status change for delivery schema"; @@ -250,25 +250,25 @@ diag "dates on status change for delivery schema"; Status => 'ordered', ); ok $id, 'created a ticket'; - ok $ticket->StartedObj->Unix <= 0, 'started is not set'; - ok $ticket->ResolvedObj->Unix <= 0, 'resolved is not set'; + ok !$ticket->StartedObj->IsSet, 'started is not set'; + ok !$ticket->ResolvedObj->IsSet, 'resolved is not set'; (my $status, $msg) = $ticket->SetStatus('delayed'); ok $status, 'changed status' or diag "error: $msg"; - ok $ticket->StartedObj->Unix > 0, 'started is set'; - ok $ticket->ResolvedObj->Unix <= 0, 'resolved is not set'; + ok $ticket->StartedObj->IsSet, 'started is set'; + ok !$ticket->ResolvedObj->IsSet, 'resolved is not set'; my $started = $ticket->StartedObj->Unix; ($status, $msg) = $ticket->SetStatus('on way'); ok $status, 'changed status' or diag "error: $msg"; is $ticket->StartedObj->Unix, $started, 'started is set and the same'; - ok $ticket->ResolvedObj->Unix <= 0, 'resolved is not set'; + ok !$ticket->ResolvedObj->IsSet, 'resolved is not set'; ($status, $msg) = $ticket->SetStatus('delivered'); ok $status, 'changed status' or diag "error: $msg"; is $ticket->StartedObj->Unix, $started, 'started is set and the same'; - ok $ticket->ResolvedObj->Unix > 0, 'resolved is set'; + ok $ticket->ResolvedObj->IsSet, 'resolved is set'; } diag "add partial map between general->delivery"; @@ -294,20 +294,20 @@ diag "check date changes on moving a ticket"; Status => 'new', ); ok $id, 'created a ticket'; - ok $ticket->StartedObj->Unix <= 0, 'started is not set'; - ok $ticket->ResolvedObj->Unix <= 0, 'resolved is not set'; + ok !$ticket->StartedObj->IsSet, 'started is not set'; + ok !$ticket->ResolvedObj->IsSet, 'resolved is not set'; (my $status, $msg) = $ticket->SetQueue( $delivery->id ); ok $status, "moved ticket between queues with different schemas"; is $ticket->Status, 'on way', 'status has been changed'; - ok $ticket->StartedObj->Unix > 0, 'started is set'; - ok $ticket->ResolvedObj->Unix <= 0, 'resolved is not set'; + ok $ticket->StartedObj->IsSet, 'started is set'; + ok !$ticket->ResolvedObj->IsSet, 'resolved is not set'; ($status, $msg) = $ticket->SetQueue( $general->id ); ok $status, "moved ticket between queues with different schemas"; is $ticket->Status, 'resolved', 'status has been changed'; - ok $ticket->StartedObj->Unix > 0, 'started is set'; - ok $ticket->ResolvedObj->Unix > 0, 'resolved is set'; + ok $ticket->StartedObj->IsSet, 'started is set'; + ok $ticket->ResolvedObj->IsSet, 'resolved is set'; } done_testing; diff --git a/t/security/CVE-2011-5092-graph-links.t b/t/security/CVE-2011-5092-graph-links.t index c43f51cbf60..31ef266040b 100644 --- a/t/security/CVE-2011-5092-graph-links.t +++ b/t/security/CVE-2011-5092-graph-links.t @@ -15,12 +15,12 @@ for my $arg (qw(LeadingLink ShowLinks)) { ); ok $ticket->id, 'created ticket'; - ok !$ticket->ToldObj->Unix, 'no Told'; + ok !$ticket->ToldObj->IsSet, 'no Told'; $m->get_ok("$base/Ticket/Graphs/index.html?$arg=SetTold;id=" . $ticket->id); $ticket->Load($ticket->id); # cache busting - ok !$ticket->ToldObj->Unix, 'still no Told'; + ok !$ticket->ToldObj->IsSet, 'still no Told'; $m->content_lacks('GotoFirstItem', 'no GotoFirstItem error'); $m->content_like(qr|]+?src=['"]/Ticket/Graphs/@{[$ticket->id]}|, 'found image element'); } From 4fdde1654829bb689884f40cbe429fcaf71e2c69 Mon Sep 17 00:00:00 2001 From: Kevin Falcone Date: Fri, 16 May 2014 16:39:34 -0400 Subject: [PATCH 3/4] Some bulletproofing on bad input to Unix as a setter Since you can technically say $date->Unix(-5); and $date->IsSet would return 1 but $date->ISO would return 1970-01-01 00:00:00 we should bulletproof for insane inputs. This lets IsSet just assume that unix time starts at 0 as intended, and avoids all the code shenanigans removed in the previous commit which checked for $date->Unix <= 0 or $date->Unix < 1 --- lib/RT/Date.pm | 10 +++++++++- t/api/date.t | 4 ++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/RT/Date.pm b/lib/RT/Date.pm index ccd23aee87f..28c1c24cede 100644 --- a/lib/RT/Date.pm +++ b/lib/RT/Date.pm @@ -556,7 +556,15 @@ Returns the number of seconds since the epoch sub Unix { my $self = shift; - $self->{'time'} = int(shift || 0) if @_; + my $time = shift; + + if (defined $time) { + if ($time < 0) { + RT->Logger->notice("Passed a unix time less than 0, forcing to 0: [$time]"); + $time = 0; + } + $self->{'time'} = int $time; + } return $self->{'time'}; } diff --git a/t/api/date.t b/t/api/date.t index 1d746f81d45..9fd71e46d2b 100644 --- a/t/api/date.t +++ b/t/api/date.t @@ -83,7 +83,7 @@ my $current_user; { my $date = RT::Date->new(RT->SystemUser); - is($date->IsSet,0,"new date isn't set"); + is($date->IsSet,0, "new date isn't set"); is($date->Unix, 0, "new date returns 0 in Unix format"); is($date->Get, '1970-01-01 00:00:00', "default is ISO format"); warning_like { @@ -250,7 +250,7 @@ warning_like $date->Unix(1); is($date->ISO, '1970-01-01 00:00:01', "correct value"); - foreach (undef, 0, ''){ + foreach (undef, 0, '', -5){ $date->Unix(1); is($date->ISO, '1970-01-01 00:00:01', "correct value"); is($date->IsSet,1,"Date has been set to a value"); From 78c96a25c725c951149fc9d7c3ef3f6656df02b1 Mon Sep 17 00:00:00 2001 From: Kevin Falcone Date: Mon, 19 May 2014 10:56:07 -0400 Subject: [PATCH 4/4] Correct check for explicit undef argument The int(shift||0) is backcompat with the old behavior that Unix("") Unix(0) or Unix(undef) all ended up setting to 1970-01-01. Copy and tweak existing Unix setting tests that worked using Set() to have them test Unix() also so backwards incompatibility failures are caught in the future. --- lib/RT/Date.pm | 4 ++-- t/api/date.t | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/RT/Date.pm b/lib/RT/Date.pm index 28c1c24cede..c9f9ca49af4 100644 --- a/lib/RT/Date.pm +++ b/lib/RT/Date.pm @@ -556,9 +556,9 @@ Returns the number of seconds since the epoch sub Unix { my $self = shift; - my $time = shift; - if (defined $time) { + if (@_) { + my $time = int(shift || 0); if ($time < 0) { RT->Logger->notice("Passed a unix time less than 0, forcing to 0: [$time]"); $time = 0; diff --git a/t/api/date.t b/t/api/date.t index 9fd71e46d2b..dd22943c198 100644 --- a/t/api/date.t +++ b/t/api/date.t @@ -260,6 +260,17 @@ warning_like is($date->Unix, 0, "unix is 0 => unset"); is($date->IsSet,0,"Date has been unset"); } + + foreach (undef, 0, '', -5){ + $date->Unix(1); + is($date->ISO, '1970-01-01 00:00:01', "correct value"); + is($date->IsSet,1,"Date has been set to a value"); + + $date->Unix($_); + is($date->ISO, '1970-01-01 00:00:00', "Set a date to midnight 1/1/1970 GMT due to wrong call"); + is($date->Unix, 0, "unix is 0 => unset"); + is($date->IsSet,0,"Date has been unset"); + } } my $year = (localtime(time))[5] + 1900;