Bug 257767: Option to have fields in CSV files separated by semicolons instead…

Bugzilla/User/Setting.pm

@@ -27,6 +27,7 @@ use base qw(Exporter);
      add_setting);
 
 use Bugzilla::Error;
+use Bugzilla::Util qw{trick_taint};
 
 ###############################
 ###  Module Initialization  ###
@@ -224,6 +225,19 @@ sub legal_values {
     return $self->{'legal_values'};
 }
 
+sub validate_value {
+    my $self = shift;
+
+    if (grep(/^$_[0]$/, @{$self->legal_values()})) {
+        trick_taint($_[0]);
+    }
+    else {
+        ThrowCodeError('setting_value_invalid',
+                       {'name'  => $self->{'_setting_name'},
+                        'value' => $_[0]});
+    }
+}
+
 sub reset_to_default {
     my ($self) = @_;
 
@@ -346,6 +360,15 @@ Description: Returns all legal values for this setting
 Params:      none
 Returns:     A reference to an array containing all legal values
 
+=item C<validate_value>
+
+Description: Determines whether a value is valid for the setting
+             by checking against the list of legal values.
+             Untaints the parameter if the value is indeed valid,
+             and throws a setting_value_invalid code error if not.
+Params:      An lvalue containing a candidate for a setting value
+Returns:     nothing
+
 =item C<reset_to_default>
 
 Description: If a user chooses to use the global default for a given 

checksetup.pl

@@ -4053,6 +4053,9 @@ add_setting ("post_bug_submit_action", {"next_bug" => 1,
                                        },
              "next_bug" );
 
+# 2005-06-29 wurblzap@gmail.com -- Bug 257767
+add_setting ('csv_colsepchar', {',' => 1, ';' => 2 }, ',' );
+
 ###########################################################################
 # Create Administrator  --ADMIN--
 ###########################################################################

editsettings.cgi

@@ -54,11 +54,9 @@ sub SaveSettings{
         my $old_value   = $vars->{'settings'}->{$name}->{'default_value'};
         my $enabled = defined $cgi->param("${name}-enabled") || 0;
         my $value = $cgi->param("${name}");
+        my $setting = new Bugzilla::User::Setting($name);
 
-        # remove taint
-        if ($value =~ /^(\w+)$/ ) {
-            $value = $1;
-        }
+        $setting->validate_value($value);
 
         if ( ($old_enabled != $enabled) ||
              ($old_value ne $value) ) {

template/en/default/filterexceptions.pl

@@ -135,7 +135,7 @@
 'reports/report-table.csv.tmpl' => [
   'num_bugs',
   'data.$tbl.$col.$row',
-  'title',
+  'colsepchar',
 ],
 
 'reports/report-table.html.tmpl' => [
@@ -190,6 +190,7 @@
 
 'reports/chart.csv.tmpl' => [
   'data.$j.$i', 
+  'colsepchar',
 ],
 
 'reports/create-chart.html.tmpl' => [
@@ -231,6 +232,7 @@
 
 'list/list.csv.tmpl' => [
   'bug.bug_id', 
+  'colsepchar',
 ],
 
 'list/list.js.tmpl' => [

template/en/default/global/code-error.html.tmpl

@@ -260,6 +260,10 @@
     option. Setting names must begin with a letter, and contain only
     letters, digits, or the symbols '_', '-', '.', or ':'.
 
+  [% ELSIF error == "setting_value_invalid" %]
+    The value "<code>[% value FILTER html %]</code>" is not in the list of
+    legal values for the <em>[% name FILTER html %]</em> setting.
+
   [% ELSIF error == "token_generation_error" %]
     Something is seriously wrong with the token generation system.
 

template/en/default/global/setting-descs.none.tmpl

@@ -21,6 +21,7 @@
 
 [% setting_descs = { 
    "comment_sort_order"               => "When viewing $terms.abug, show comments in this order",
+   "csv_colsepchar"                   => "Field separator character for CSV files",
    "display_quips"                    => "Show a quip at the top of each bug list",
    "newest_to_oldest"                 => "Newest to Oldest",
    "newest_to_oldest_desc_first"      => "Newest to Oldest, but keep Description at the top",

template/en/default/list/list.csv.tmpl

@@ -23,19 +23,22 @@
 [% PROCESS global/variables.none.tmpl %]
 [% USE date %]
 
-bug_id,
+[% colsepchar = user.settings.csv_colsepchar.value %]
+
+bug_id
 [% FOREACH column = displaycolumns %]
-  [% column FILTER csv %],
+  [% colsepchar %][% column FILTER csv %]
 [% END %]
 
 [% FOREACH bug = bugs %]
-  [% bug.bug_id %],
+  [% bug.bug_id %]
     [% FOREACH column = displaycolumns %]
+      [% colsepchar %]
       [% IF column == "opendate" OR column == "changeddate" %]
         [% rawcolumn = column.replace("date", "time") %]
         [% bug.$column = date.format(bug.$rawcolumn, "%Y-%m-%d %H:%M:%S") %]
       [% END %]
-      [% bug.$column FILTER csv %],
+      [% bug.$column FILTER csv %]
     [% END %]
 
 [% END %]

template/en/default/reports/chart.csv.tmpl

@@ -19,10 +19,12 @@
   # Contributor(s): Gervase Markham <gerv@gerv.net>
   #%]
 
+[% colsepchar = user.settings.csv_colsepchar.value %]
+
 [% data = chart.data %]
-Date\Series,
+Date\Series
 [% FOREACH label = chart.labels %]
-  [% label FILTER csv %][% "," UNLESS loop.last %]
+  [% colsepchar %][% label FILTER csv %]
 [% END %]
 [%# The data, which is in the correct format for GD, is conceptually the wrong
   # way round for CSV output. So, we need to invert it here, which is why 
@@ -32,7 +34,10 @@ Date\Series,
 [% WHILE i < data.0.size %]
   [% j = 0 %]
   [% WHILE j < data.size %]
-    [% data.$j.$i %][% "," UNLESS (j == data.size - 1) %]
+    [% IF j > 0 %]
+      [% colsepchar %]
+    [% END %]
+    [% data.$j.$i %]
     [% j = j + 1 %]
   [% END %]
   [% i = i + 1 %]

template/en/default/reports/report-table.csv.tmpl

@@ -23,51 +23,50 @@
   #%]
 [% PROCESS global/variables.none.tmpl %]
 
+[% colsepchar = user.settings.csv_colsepchar.value %]
+
 [% num_bugs = BLOCK %]Number of [% terms.bugs %][% END %]
 [% tbl_field_disp = field_descs.$tbl_field || tbl_field %]
 [% col_field_disp = field_descs.$col_field || col_field %]
 [% row_field_disp = field_descs.$row_field || row_field %]
 
-[% title = BLOCK %]
-  [% IF tbl_field %]
+[% IF tbl_field %]
   [% tbl_field_disp FILTER csv %]: [% tbl FILTER csv %]
-
-  [% END %]
-  [% IF row_field %]
+[% END %]
+[% IF row_field %]
   [% row_field_disp FILTER csv %]
-  [% END %]
-  [% " / " IF col_field AND row_field %]
-  [% col_field_disp FILTER csv %]
 [% END %]
-
-[% title %],
+[% " / " IF col_field AND row_field %]
+[% col_field_disp FILTER csv %]
 [% IF col_field -%]
-[% FOREACH col = col_names -%]
+  [% FOREACH col = col_names -%]
+    [% colsepchar %]
     [% IF col_field == 'bug_status' %]
       [% status_descs.$col FILTER csv -%]
     [% ELSIF col_field == 'resolution' %]
       [% resolution_descs.$col FILTER csv -%]
     [% ELSE %]
-    [% col FILTER csv -%],
+      [% col FILTER csv -%]
     [% END %]
-[% END -%]
+  [% END -%]
 [% ELSE -%]
-  [% num_bugs %],
+  [% colsepchar %][% num_bugs %]
 [% END %]
 
 [% FOREACH row = row_names %]
-[% IF row_field == 'bug_status' %]
+  [% IF row_field == 'bug_status' %]
     [% status_descs.$row FILTER csv -%]
   [% ELSIF row_field == 'resolution' %]
     [% resolution_descs.$row FILTER csv -%]
   [% ELSE %]
-    [% row FILTER csv -%],
+    [% row FILTER csv -%]
   [% END %]
   [% FOREACH col = col_names %]
+    [% colsepchar %]
     [% IF data.$tbl AND data.$tbl.$col AND data.$tbl.$col.$row %]
-      [% data.$tbl.$col.$row -%],
+      [% data.$tbl.$col.$row -%]
     [% ELSE %]
-      [% -%]0,
+      [% -%]0
     [% END %]
   [% END %]
 

userprefs.cgi

@@ -152,17 +152,15 @@ sub SaveSettings {
     foreach my $name (@setting_list) {
         next if ! ($settings->{$name}->{'is_enabled'});
         my $value = $cgi->param($name);
+        my $setting = new Bugzilla::User::Setting($name);
 
-        # de-taint the value.
-        if ($value =~ /^([-\w]+)$/ ) {
-            $value = $1;
-        }
         if ($value eq "${name}-isdefault" ) {
             if (! $settings->{$name}->{'is_default'}) {
                 $settings->{$name}->reset_to_default;
             }
         }
         else {
+            $setting->validate_value($value);
             $settings->{$name}->set($value);
         }
     }