Bug 257767: Option to have fields in CSV files separated by semicolons instead of commas - Patch by Marc Schumann <wurblzap@gmail.com> r=LpSolit a=myk

10 files changed, 80 insertions, 44 deletions

diff --git a/Bugzilla/User/Setting.pm b/Bugzilla/User/Setting.pm
index df1ba28f8..a5ce9411f 100644
--- a/Bugzilla/User/Setting.pm
+++ b/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 
diff --git a/checksetup.pl b/checksetup.pl
index 3ac8eb5e3..7e5152628 100755
--- a/checksetup.pl
+++ b/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--
 ###########################################################################
diff --git a/editsettings.cgi b/editsettings.cgi
index b5e810ba9..80a8921d5 100755
--- a/editsettings.cgi
+++ b/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) ) {
diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl
index eb6d32aef..cf98ea8e2 100644
--- a/template/en/default/filterexceptions.pl
+++ b/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' => [
diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl
index 5ad057061..9d9422ad5 100644
--- a/template/en/default/global/code-error.html.tmpl
+++ b/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.
 
diff --git a/template/en/default/global/setting-descs.none.tmpl b/template/en/default/global/setting-descs.none.tmpl
index 85edc1949..9957cf6a4 100644
--- a/template/en/default/global/setting-descs.none.tmpl
+++ b/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",
diff --git a/template/en/default/list/list.csv.tmpl b/template/en/default/list/list.csv.tmpl
index 2731b6613..76df4f4da 100644
--- a/template/en/default/list/list.csv.tmpl
+++ b/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 %]
diff --git a/template/en/default/reports/chart.csv.tmpl b/template/en/default/reports/chart.csv.tmpl
index 83620bf08..87866dfd6 100644
--- a/template/en/default/reports/chart.csv.tmpl
+++ b/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 %]
diff --git a/template/en/default/reports/report-table.csv.tmpl b/template/en/default/reports/report-table.csv.tmpl
index 42fcd6686..96dca3d58 100644
--- a/template/en/default/reports/report-table.csv.tmpl
+++ b/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 %]
-    [% tbl_field_disp FILTER csv %]: [% tbl FILTER csv %]
-
-  [% END %]
-  [% IF row_field %]
-    [% row_field_disp FILTER csv %]
-  [% END %]
-  [% " / " IF col_field AND row_field %]
-  [% col_field_disp FILTER csv %]
+[% IF tbl_field %]
+  [% tbl_field_disp FILTER csv %]: [% tbl FILTER csv %]
 [% END %]
-
-[% title %],
+[% IF row_field %]
+  [% row_field_disp FILTER csv %]
+[% END %]
+[% " / " IF col_field AND row_field %]
+[% col_field_disp FILTER csv %]
 [% IF col_field -%]
-[% FOREACH col = col_names -%]
-  [% IF col_field == 'bug_status' %]
-    [% status_descs.$col FILTER csv -%]
-  [% ELSIF col_field == 'resolution' %]
-    [% resolution_descs.$col FILTER csv -%]
-  [% ELSE %]
-    [% col FILTER csv -%],
-  [% END %]
-[% END -%]
+  [% 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 -%]
+    [% 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 %]
 
diff --git a/userprefs.cgi b/userprefs.cgi
index 07042beac..5f52a3ca7 100755
--- a/userprefs.cgi
+++ b/userprefs.cgi
@@ -152,18 +152,16 @@ 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;
+                $settings->{$name}->reset_to_default;
             }
         }
         else {
-           $settings->{$name}->set($value);
+            $setting->validate_value($value);
+            $settings->{$name}->set($value);
         }
     }
     $vars->{'settings'} = Bugzilla->user->settings(1);