Patch for bug 237369: implement relatively simple changes from %FORM to $cgi->param variable; patch by Teemu Mannermaa <wicked@etlicon.fi>; r=kiko, justdave; a=justdave.

10 files changed, 68 insertions, 71 deletions

diff --git a/config.cgi b/config.cgi
index a02ff8c7b..f7cb95ee5 100755
--- a/config.cgi
+++ b/config.cgi
@@ -53,8 +53,6 @@ use vars
     @legal_target_milestone 
     @legal_versions 
     @legal_keywords 
-
-    %FORM 
   );
 
 # Use the global template variables defined in globals.pl 
@@ -92,7 +90,9 @@ $vars->{'field'} = [GetFieldDefs()];
 
 # Determine how the user would like to receive the output; 
 # default is JavaScript.
-my $format = GetFormat("config", $::FORM{'format'}, $::FORM{'ctype'} || "js");
+my $cgi = Bugzilla->cgi;
+my $format = GetFormat("config", scalar($cgi->param('format')),
+                       scalar($cgi->param('ctype')) || "js");
 
 # Return HTTP headers.
 print "Content-Type: $format->{'ctype'}\n\n";
diff --git a/describecomponents.cgi b/describecomponents.cgi
index 05af91949..922a4912e 100755
--- a/describecomponents.cgi
+++ b/describecomponents.cgi
@@ -22,7 +22,6 @@
 #                 Bradley Baetz <bbaetz@student.usyd.edu.au>
 
 use vars qw(
-  %FORM
   %legal_product
   $userid
 );
@@ -41,8 +40,9 @@ quietly_check_login();
 GetVersionTable();
 
 my $cgi = Bugzilla->cgi;
+my $product = $cgi->param('product');
 
-if (!defined $::FORM{'product'}) {
+if (!$product) {
     # Reference to a subset of %::proddesc, which the user is allowed to see
     my %products;
 
@@ -73,11 +73,9 @@ if (!defined $::FORM{'product'}) {
         exit;
     }
 
-    $::FORM{'product'} = (keys %products)[0];
+    $product = (keys %products)[0];
 }
 
-my $product = $::FORM{'product'};
-
 # Make sure the user specified a valid product name.  Note that
 # if the user specifies a valid product name but is not authorized
 # to access that product, they will receive a different error message
diff --git a/doeditparams.cgi b/doeditparams.cgi
index 3a89a0fd0..4799c99c0 100755
--- a/doeditparams.cgi
+++ b/doeditparams.cgi
@@ -30,8 +30,6 @@ use Bugzilla::Config qw(:DEFAULT :admin $datadir);
 
 require "CGI.pl";
 
-use vars %::MFORM;
-
 ConnectToDatabase();
 confirm_login();
 
@@ -52,13 +50,13 @@ my $howto = "";
 
 foreach my $i (GetParamList()) {
     my $name = $i->{'name'};
-    my $value = $::FORM{$name};
-    if (exists $::FORM{"reset-$name"}) {
+    my $value = $cgi->param($name);
+    if (defined $cgi->param("reset-$name")) {
         $value = $i->{'default'};
     } else {
         if ($i->{'type'} eq 'm') {
             # This simplifies the code below
-            $value = \@{$::MFORM{$name}};
+            $value = [ $cgi->param($name) ];
         } else {
             # Get rid of windows/mac-style line endings.
             $value =~ s/\r\n?/\n/g;
diff --git a/duplicates.cgi b/duplicates.cgi
index 5da90e7b9..aa627fc40 100755
--- a/duplicates.cgi
+++ b/duplicates.cgi
@@ -32,8 +32,6 @@ use lib qw(.);
 require "globals.pl";
 require "CGI.pl";
 
-use vars qw($buffer);
-
 use Bugzilla;
 use Bugzilla::Search;
 use Bugzilla::Config qw(:DEFAULT $datadir);
@@ -44,8 +42,8 @@ my $cgi = Bugzilla->cgi;
 # Go directly to the XUL version of the duplicates report (duplicates.xul)
 # if the user specified ctype=xul.  Adds params if they exist, and directs
 # the user to a signed copy of the script in duplicates.jar if it exists.
-if ($::FORM{'ctype'} && $::FORM{'ctype'} eq "xul") {
-    my $params = CanonicaliseParams($::buffer, ["format", "ctype"]);
+if (defined $cgi->param('ctype') && $cgi->param('ctype') eq "xul") {
+    my $params = CanonicaliseParams($cgi->query_string(), ["format", "ctype"]);
     my $url = (-e "duplicates.jar" ? "duplicates.jar!/" : "") . 
           "duplicates.xul" . ($params ? "?$params" : "") . "\n\n";
 
@@ -71,7 +69,7 @@ else {
 
 Bugzilla->switch_to_shadow_db();
 
-use vars qw (%FORM $userid @legal_product);
+use vars qw ($userid @legal_product);
 
 my %dbmcount;
 my %count;
@@ -80,7 +78,7 @@ my %before;
 # Get params from URL
 sub formvalue {
     my ($name, $default) = (@_);
-    return $FORM{$name} || $default || "";
+    return $cgi->param($name) || $default || "";
 }
 
 my $sortby = formvalue("sortby");
@@ -218,7 +216,7 @@ if (scalar(%count)) {
     }
 
     # Restrict to product if requested
-    if ($::FORM{'product'}) {
+    if ($cgi->param('product')) {
         $params->param('product', join(',', @query_products));
     }
 
@@ -267,13 +265,13 @@ $vars->{'changedsince'} = $changedsince;
 $vars->{'maxrows'} = $maxrows;
 $vars->{'openonly'} = $openonly;
 $vars->{'reverse'} = $reverse;
-$vars->{'format'} = $::FORM{'format'};
+$vars->{'format'} = $cgi->param('format');
 $vars->{'query_products'} = \@query_products;
 $vars->{'products'} = \@::legal_product;
 
 
-my $format = 
-  GetFormat("reports/duplicates", $::FORM{'format'}, $::FORM{'ctype'});
+my $format = GetFormat("reports/duplicates", scalar($cgi->param('format')),
+                       scalar($cgi->param('ctype')));
 
 print $cgi->header($format->{'ctype'});
 
diff --git a/globals.pl b/globals.pl
index 771d81b31..aef84f2a6 100644
--- a/globals.pl
+++ b/globals.pl
@@ -1474,7 +1474,8 @@ sub FormatTimeUnit {
 
 # Constructs a format object from URL parameters. You most commonly call it 
 # like this:
-# my $format = GetFormat("foo/bar", $::FORM{'format'}, $::FORM{'ctype'});
+# my $format = GetFormat("foo/bar", scalar($cgi->param('format')),
+#                        scalar($cgi->param('ctype')));
 
 sub GetFormat {
     my ($template, $format, $ctype) = @_;
diff --git a/long_list.cgi b/long_list.cgi
index 1f8126d19..72c579ac4 100755
--- a/long_list.cgi
+++ b/long_list.cgi
@@ -28,7 +28,7 @@ use Bugzilla;
 
 require "CGI.pl";
 
-use vars qw($userid @legal_keywords %FORM);
+use vars qw($userid @legal_keywords);
 
 # Use global template variables.
 use vars qw($template $vars);
@@ -69,9 +69,9 @@ my $generic_query = "
   WHERE assign.userid = bugs.assigned_to AND report.userid = bugs.reporter
     AND bugs.product_id=products.id AND bugs.component_id=components.id";
 
-my $buglist = $::FORM{'buglist'} || 
-              $::FORM{'bug_id'}  || 
-              $::FORM{'id'}      || "";
+my $buglist = $cgi->param('buglist') || 
+              $cgi->param('bug_id')  || 
+              $cgi->param('id')      || "";
 
 my @bugs;
 
diff --git a/report.cgi b/report.cgi
index 72e69a0fd..2c0c430a9 100755
--- a/report.cgi
+++ b/report.cgi
@@ -88,7 +88,7 @@ if (defined($height)) {
 # These shenanigans are necessary to make sure that both vertical and 
 # horizontal 1D tables convert to the correct dimension when you ask to
 # display them as some sort of chart.
-if ($::FORM{'format'} && $::FORM{'format'} eq "table") {
+if (defined $cgi->param('format') && $cgi->param('format') eq "table") {
     if ($col_field && !$row_field) {    
         # 1D *tables* should be displayed vertically (with a row_field only)
         $row_field = $col_field;
@@ -256,7 +256,7 @@ $vars->{'width'} = $width if $width;
 $vars->{'height'} = $height if $height;
 
 $vars->{'query'} = $query;
-$vars->{'debug'} = $::FORM{'debug'};
+$vars->{'debug'} = $cgi->param('debug');
 
 my $formatparam = $cgi->param('format');
 
@@ -306,7 +306,7 @@ my $format = GetFormat("reports/report", $formatparam, $cgi->param('ctype'));
 # If we get a template or CGI error, it comes out as HTML, which isn't valid
 # PNG data, and the browser just displays a "corrupt PNG" message. So, you can
 # set debug=1 to always get an HTML content-type, and view the error.
-$format->{'ctype'} = "text/html" if $::FORM{'debug'};
+$format->{'ctype'} = "text/html" if $cgi->param('debug');
 
 my @time = localtime(time());
 my $date = sprintf "%04d-%02d-%02d", 1900+$time[5],$time[4]+1,$time[3];
@@ -316,7 +316,7 @@ print $cgi->header(-type => $format->{'ctype'},
 
 # Problems with this CGI are often due to malformed data. Setting debug=1
 # prints out both data structures.
-if ($::FORM{'debug'}) {
+if ($cgi->param('debug')) {
     require Data::Dumper;
     print "<pre>data hash:\n";
     print Data::Dumper::Dumper(%data) . "\n\n";
diff --git a/reports.cgi b/reports.cgi
index 67274a6d5..b863249d6 100755
--- a/reports.cgi
+++ b/reports.cgi
@@ -40,7 +40,6 @@ use lib qw(.);
 use Bugzilla::Config qw(:DEFAULT $datadir);
 
 require "CGI.pl";
-use vars qw(%FORM); # globals from CGI.pl
 
 require "globals.pl";
 use vars qw(@legal_product); # globals from er, globals.pl
@@ -71,7 +70,7 @@ my @myproducts;
 push( @myproducts, "-All-");
 push( @myproducts, GetSelectableProducts());
 
-if (! defined $FORM{'product'}) {
+if (! defined $cgi->param('product')) {
 
     print $cgi->header();
     PutHeader("Bug Charts");
@@ -79,29 +78,29 @@ if (! defined $FORM{'product'}) {
     PutFooter();
 
 } else {
+    my $product = $cgi->param('product');
 
     # For security and correctness, validate the value of the "product" form variable.
     # Valid values are those products for which the user has permissions which appear
     # in the "product" drop-down menu on the report generation form.
-    grep($_ eq $FORM{'product'}, @myproducts)
-      || ThrowUserError("invalid_product_name", {product => $FORM{'product'}});
+    grep($_ eq $product, @myproducts)
+      || ThrowUserError("invalid_product_name", {product => $product});
 
     # We don't want people to be able to view
     # reports for products they don't have permissions for...
-    if (($FORM{'product'} ne '-All-')
-      && (!CanEnterProduct($FORM{'product'}))) {
+    if (($product ne '-All-') && (!CanEnterProduct($product))) {
         ThrowUserError("report_access_denied");
     }
           
     # We've checked that the product exists, and that the user can see it
     # This means that is OK to detaint
-    trick_taint($FORM{'product'});
+    trick_taint($product);
 
     print $cgi->header(-Content_Disposition=>'inline; filename=bugzilla_report.html');
 
     PutHeader("Bug Charts");
 
-    show_chart();
+    show_chart($product);
 
     PutFooter();
 }
@@ -189,21 +188,25 @@ sub daily_stats_filename {
 }
 
 sub show_chart {
-    if (! $FORM{datasets}) {
+    my ($product) = @_;
+
+    if (! defined $cgi->param('datasets')) {
         ThrowUserError("missing_datasets");
     }
+    my $datasets = join('', $cgi->param('datasets'));
 
   print <<FIN;
 <center>
 FIN
 
     my $type = chart_image_type();
-    my $data_file = daily_stats_filename($FORM{product});
-    my $image_file = chart_image_name($data_file, $type);
+    my $data_file = daily_stats_filename($product);
+    my $image_file = chart_image_name($data_file, $type, $datasets);
     my $url_image = "$graph_dir/" . url_quote($image_file);
 
     if (! -e "$graph_dir/$image_file") {
-        generate_chart("$dir/$data_file", "$graph_dir/$image_file", $type);
+        generate_chart("$dir/$data_file", "$graph_dir/$image_file", $type,
+                       $product, $datasets);
     }
     
     print <<FIN;
@@ -223,7 +226,7 @@ sub chart_image_type {
 }
 
 sub chart_image_name {
-    my ($data_file, $type) = @_;
+    my ($data_file, $type, $datasets) = @_;
 
     # This routine generates a filename from the requested fields. The problem
     # is that we have to check the safety of doing this. We can't just require
@@ -232,15 +235,16 @@ sub chart_image_name {
     # Instead, just require that each field name consists only of letters
     # and number
 
-    if ($FORM{'datasets'} !~ m/[A-Za-z0-9:]/) {
-        die "Invalid datasets $FORM{'datasets'}";
+    if ($datasets !~ m/[A-Za-z0-9:]/) {
+        die "Invalid datasets $datasets";
     }
+
     # Since we pass the tests, consider it OK
-    trick_taint($FORM{'datasets'});
+    trick_taint($datasets);
 
     # Cache charts by generating a unique filename based on what they
     # show. Charts should be deleted by collectstats.pl nightly.
-    my $id = join ("_", split (":", $FORM{datasets}));
+    my $id = join ("_", split (":", $datasets));
 
     return "${data_file}_${id}.$type";
 }
@@ -253,7 +257,7 @@ sub day_of_year {
 }
 
 sub generate_chart {
-    my ($data_file, $image_file, $type) = @_;
+    my ($data_file, $image_file, $type, $product, $datasets) = @_;
     
     if (! open FILE, $data_file) {
         ThrowCodeError("chart_data_not_generated");
@@ -261,7 +265,7 @@ sub generate_chart {
 
     my @fields;
     my @labels = qw(DATE);
-    my %datasets = map { $_ => 1 } split /:/, $FORM{datasets};
+    my %datasets = map { $_ => 1 } split /:/, $datasets;
 
     my %data = ();
     while (<FILE>) {
@@ -318,7 +322,7 @@ sub generate_chart {
 
     my %settings =
         (
-         "title" => "Status Counts for $FORM{'product'}",
+         "title" => "Status Counts for $product",
          "x_label" => "Dates",
          "y_label" => "Bug Counts",
          "legend_labels" => \@labels,
diff --git a/showdependencygraph.cgi b/showdependencygraph.cgi
index 6496bbc8e..a863df142 100755
--- a/showdependencygraph.cgi
+++ b/showdependencygraph.cgi
@@ -100,11 +100,11 @@ sub AddLink {
     }
 }
 
-$::FORM{'rankdir'} = "LR" if !defined $::FORM{'rankdir'};
+my $rankdir = $cgi->param('rankdir') || "LR";
 
-if (!defined($::FORM{'id'}) && !defined($::FORM{'doall'})) {
+if (!defined $cgi->param('id') && !defined $cgi->param('doall')) {
     ThrowCodeError("missing_bug_id");
-}    
+}
 
 my ($fh, $filename) = File::Temp::tempfile("XXXXXXXXXX",
                                            SUFFIX => '.dot',
@@ -113,13 +113,13 @@ my $urlbase = Param('urlbase');
 
 print $fh "digraph G {";
 print $fh qq{
-graph [URL="${urlbase}query.cgi", rankdir=$::FORM{'rankdir'}, size="64,64"]
+graph [URL="${urlbase}query.cgi", rankdir=$rankdir, size="64,64"]
 node [URL="${urlbase}show_bug.cgi?id=\\N", style=filled, color=lightgrey]
 };
 
 my %baselist;
 
-if ($::FORM{'doall'}) {
+if ($cgi->param('doall')) {
     SendSQL("SELECT blocked, dependson FROM dependencies");
 
     while (MoreSQLData()) {
@@ -127,7 +127,7 @@ if ($::FORM{'doall'}) {
         AddLink($blocked, $dependson, $fh);
     }
 } else {
-    foreach my $i (split('[\s,]+', $::FORM{'id'})) {
+    foreach my $i (split('[\s,]+', $cgi->param('id'))) {
         $i = trim($i);
         ValidateBugID($i);
         $baselist{$i} = 1;
@@ -179,7 +179,7 @@ foreach my $k (keys(%seen)) {
 
     my @params;
 
-    if ($summary ne "" && $::FORM{'showsummary'}) {
+    if ($summary ne "" && $cgi->param('showsummary')) {
         $summary =~ s/([\\\"])/\\$1/g;
         push(@params, qq{label="$k\\n$summary"});
     }
@@ -205,7 +205,7 @@ foreach my $k (keys(%seen)) {
 
     # Show the bug summary in tooltips only if not shown on 
     # the graph and it is non-empty (the user can see the bug)
-    if (!$::FORM{'showsummary'} && $summary ne "") {
+    if (!$cgi->param('showsummary') && $summary ne "") {
         $bugtitles{$k} .= " - $summary";
     }
 }
@@ -271,11 +271,11 @@ foreach my $f (@files)
     }
 }
 
-$vars->{'bug_id'} = $::FORM{'id'};
-$vars->{'multiple_bugs'} = ($::FORM{'id'} =~ /[ ,]/);
-$vars->{'doall'} = $::FORM{'doall'};
-$vars->{'rankdir'} = $::FORM{'rankdir'};
-$vars->{'showsummary'} = $::FORM{'showsummary'};
+$vars->{'bug_id'} = $cgi->param('id');
+$vars->{'multiple_bugs'} = ($cgi->param('id') =~ /[ ,]/);
+$vars->{'doall'} = $cgi->param('doall');
+$vars->{'rankdir'} = $rankdir;
+$vars->{'showsummary'} = $cgi->param('showsummary');
 
 # Generate and return the UI (HTML page) from the appropriate template.
 print $cgi->header();
diff --git a/showdependencytree.cgi b/showdependencytree.cgi
index d9f642a3e..b82443226 100755
--- a/showdependencytree.cgi
+++ b/showdependencytree.cgi
@@ -31,8 +31,6 @@ require "CGI.pl";
 # Use global template variables.
 use vars qw($template $vars);
 
-use vars %::FORM;
-
 ConnectToDatabase();
 
 quietly_check_login();
@@ -52,12 +50,12 @@ $::userid = $::userid;
 
 # Make sure the bug ID is a positive integer representing an existing
 # bug that the user is authorized to access.
-ValidateBugID($::FORM{'id'});
-my $id = $::FORM{'id'};
+my $id = $cgi->param('id');
+ValidateBugID($id);
 
-my $hide_resolved = $::FORM{'hide_resolved'} ? 1 : 0;
+my $hide_resolved = $cgi->param('hide_resolved') ? 1 : 0;
 
-my $maxdepth = $::FORM{'maxdepth'} || 0;
+my $maxdepth = $cgi->param('maxdepth') || 0;
 if ($maxdepth !~ /^\d+$/) { $maxdepth = 0 };
 
 ################################################################################