Bug 287487: User with no privs can not add comments to bugs that have a deadline field set - Patch by Frédéric Buclin <LpSolit@gmail.com> r=mkanat a=myk

3 files changed, 57 insertions, 47 deletions

diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index 70b4c6845..ed415c7be 100644
--- a/Bugzilla/Util.pm
+++ b/Bugzilla/Util.pm
@@ -249,38 +249,44 @@ sub find_wrap_point ($$) {
     return $wrappoint;
 }
 
-sub format_time {
-    my ($time) = @_;
-
-    my ($year, $month, $day, $hour, $min, $sec);
-    if ($time =~ m/^\d{14}$/) {
-        # We appear to have a timestamp direct from MySQL
-        $year  = substr($time,0,4);
-        $month = substr($time,4,2);
-        $day   = substr($time,6,2);
-        $hour  = substr($time,8,2);
-        $min   = substr($time,10,2);
-    }
-    elsif ($time =~ m/^(\d{4})[-\.](\d{2})[-\.](\d{2}) (\d{2}):(\d{2})(:(\d{2}))?$/) {
-        $year  = $1;
-        $month = $2;
-        $day   = $3;
-        $hour  = $4;
-        $min   = $5;
-        $sec   = $7;
+sub format_time ($;$) {
+    my ($date, $format) = @_;
+
+    # If $format is undefined, try to guess the correct date format.    
+    my $show_timezone;
+    if (!defined($format)) {
+        if ($date =~ m/^(\d{4})[-\.](\d{2})[-\.](\d{2}) (\d{2}):(\d{2})(:(\d{2}))?$/) {
+            my $sec = $7;
+            if (defined $sec) {
+                $format = "%Y-%m-%d %T";
+            } else {
+                $format = "%Y-%m-%d %R";
+            }
+        } else {
+            # Default date format. See Date::Format for other formats available.
+            $format = "%Y-%m-%d %R";
+        }
+        # By default, we want the timezone to be displayed.
+        $show_timezone = 1;
     }
     else {
-        warn "Date/Time format ($time) unrecogonzied";
+        # Search for %Z or %z, meaning we want the timezone to be displayed.
+        # Till bug 182238 gets fixed, we assume Param('timezone') is used.
+        $show_timezone = ($format =~ s/\s?%Z$//i);
     }
 
-    if (defined $year) {
-        $time = "$year-$month-$day $hour:$min";
-        if (defined $sec) {
-            $time .= ":$sec";
-        }
-        $time .= " " . &::Param('timezone') if &::Param('timezone');
+    # str2time($date) is undefined if $date has an invalid date format.
+    my $time = str2time($date);
+
+    if (defined $time) {
+        $date = time2str($format, $time);
+        $date .= " " . &::Param('timezone') if $show_timezone;
+    }
+    else {
+        # Don't let invalid (time) strings to be passed to templates!
+        $date = '';
     }
-    return $time;
+    return trim($date);
 }
 
 sub format_time_decimal {
@@ -531,10 +537,15 @@ The search starts at $maxpos and goes back to the beginning of the string.
 
 =item C<format_time($time)>
 
-Takes a time and appends the timezone as defined in editparams.cgi.  This routine
-will be expanded in the future to adjust for user preferences regarding what
-timezone to display times in.  In the future, it may also allow for the time to be
-shown in different formats.
+Takes a time, converts it to the desired format and appends the timezone
+as defined in editparams.cgi, if desired. This routine will be expanded
+in the future to adjust for user preferences regarding what timezone to
+display times in.
+
+This routine is mainly called from templates to filter dates, see
+"FILTER time" in Templates.pm. In this case, $format is undefined and
+the routine has to "guess" the date format that was passed to $dbh->sql_date_format().
+
 
 =item C<format_time_decimal($time)>
 
diff --git a/process_bug.cgi b/process_bug.cgi
index 05f4fec0d..451613e29 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -1185,7 +1185,13 @@ foreach my $id (@idlist) {
     my %formhash;
     foreach my $col (@::log_columns) {
         # Consider NULL db entries to be equivalent to the empty string
-        $oldvalues[$i] = '' unless defined $oldvalues[$i];
+        $oldvalues[$i] = defined($oldvalues[$i]) ? $oldvalues[$i] : '';
+        # Convert the deadline taken from the DB into the YYYY-MM-DD format
+        # for consistency with the deadline provided by the user, if any.
+        # Else CheckCanChangeField() would see them as different in all cases.
+        if ($col eq 'deadline') {
+            $oldvalues[$i] = format_time($oldvalues[$i], "%Y-%m-%d");
+        }
         $oldhash{$col} = $oldvalues[$i];
         $formhash{$col} = $cgi->param($col) if defined $cgi->param($col);
         $i++;
@@ -1717,6 +1723,10 @@ foreach my $id (@idlist) {
     foreach my $col (@::log_columns) {
         # Consider NULL db entries to be equivalent to the empty string
         $newvalues[$i] ||= '';
+        # Convert the deadline to the YYYY-MM-DD format.
+        if ($col eq 'deadline') {
+            $newvalues[$i] = format_time($newvalues[$i], "%Y-%m-%d");
+        }
         $newhash{$col} = $newvalues[$i];
         $i++;
     }
@@ -1779,16 +1789,6 @@ foreach my $id (@idlist) {
                 $check_dep_bugs = 1;
             }
 
-            # Convert deadlines to the YYYY-MM-DD format. We use an
-            # intermediate $xxxtime to prevent errors in the web
-            # server log file when str2time($xxx) is undefined.
-            if ($col eq 'deadline') {
-                my $oldtime = str2time($old);
-                $old = ($oldtime) ? time2str("%Y-%m-%d", $oldtime) : '';
-                my $newtime = str2time($new);
-                $new = ($newtime) ? time2str("%Y-%m-%d", $newtime) : '';
-            }
-
             LogActivityEntry($id,$col,$old,$new,$whoid,$timestamp);
             $bug_changed = 1;
         }
diff --git a/t/007util.t b/t/007util.t
index 48c925aa3..02069dcc1 100644
--- a/t/007util.t
+++ b/t/007util.t
@@ -28,8 +28,8 @@ use lib 't';
 use Support::Files;
 
 BEGIN { 
-	use Test::More tests => 13;
-	use_ok(Bugzilla::Util);
+        use Test::More tests => 14;
+        use_ok(Bugzilla::Util);
 }
 
 # We need to override the the Param() function so we can get an expected
@@ -68,8 +68,7 @@ is(min(@list),2,'min()');
 is(trim(" fg<*\$%>+=~~ "),'fg<*$%>+=~~','trim()');
 
 #format_time();
-is(format_time("20021123140436"),'2002-11-23 14:04 TEST','format_time("20021123140436")');
 is(format_time("2002.11.24 00:05"),'2002-11-24 00:05 TEST','format_time("2002.11.24 00:05")');
 is(format_time("2002.11.24 00:05:56"),'2002-11-24 00:05:56 TEST','format_time("2002.11.24 00:05:56")');
-
-
+is(format_time("2002.11.24 00:05:56", "%Y-%m-%d %R"), '2002-11-24 00:05', 'format_time("2002.11.24 00:05:56", "%Y-%m-%d %R") (with no timezone)');
+is(format_time("2002.11.24 00:05:56", "%Y-%m-%d %R %Z"), '2002-11-24 00:05 TEST', 'format_time("2002.11.24 00:05:56", "%Y-%m-%d %R %Z") (with timezone)');