Bug 147833 - start using CGI.pm

r=gerv, justdave

12 files changed, 327 insertions, 300 deletions

diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm
new file mode 100644
index 000000000..6a9730bc6
--- /dev/null
+++ b/Bugzilla/CGI.pm
@@ -0,0 +1,149 @@
+# -*- Mode: perl; indent-tabs-mode: nil -*-
+#
+# The contents of this file are subject to the Mozilla Public
+# License Version 1.1 (the "License"); you may not use this file
+# except in compliance with the License. You may obtain a copy of
+# the License at http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS
+# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+# implied. See the License for the specific language governing
+# rights and limitations under the License.
+#
+# The Original Code is the Bugzilla Bug Tracking System.
+#
+# The Initial Developer of the Original Code is Netscape Communications
+# Corporation. Portions created by Netscape are
+# Copyright (C) 1998 Netscape Communications Corporation. All
+# Rights Reserved.
+#
+# Contributor(s): Bradley Baetz <bbaetz@student.usyd.edu.au>
+
+use strict;
+
+package Bugzilla::CGI;
+
+use CGI qw(-no_xhtml -oldstyle_urls :private_tempfiles);
+
+use base qw(CGI);
+
+use Bugzilla::Util;
+
+# CGI.pm uses AUTOLOAD, but explicitly defines a DESTROY sub.
+# We need to do so, too, otherwise perl dies when the object is destroyed
+# and we don't have a DESTROY method (because CGI.pm's AUTOLOAD will |die|
+# on getting an unknown sub to try to call)
+sub DESTROY {};
+
+sub new {
+    my ($invocant, @args) = @_;
+    my $class = ref($invocant) || $invocant;
+
+    my $self = $class->SUPER::new(@args);
+
+    # Check for errors
+    # All of the Bugzilla code wants to do this, so do it here instead of
+    # in each script
+
+    my $err = $self->cgi_error;
+
+    if ($err) {
+        # XXX - under mod_perl we can use the request object to
+        # enable the apache ErrorDocument stuff, which is localisable
+        # (and localised by default under apache2).
+        # This doesn't appear to be possible under mod_cgi.
+        # Under mod_perl v2, though, this happens automatically, and the
+        # message body is ignored.
+
+        # Note that this error block is only triggered by CGI.pm for malformed
+        # multipart requests, and so should never happen unless there is a
+        # browser bug.
+
+        # Using CGI.pm to do this means that ThrowCodeError prints the
+        # content-type again...
+        #print $self->header(-status => $err);
+        print "Status: $err\n";
+
+        my $vars = {};
+        if ($err =~ m/(\d{3})\s(.*)/) {
+            $vars->{http_error_code} = $1;
+            $vars->{http_error_string} = $2;
+        } else {
+            $vars->{http_error_string} = $err;
+        }
+
+        &::ThrowCodeError("cgi_error", $vars);
+    }
+
+    return $self;
+}
+
+# We want this sorted plus the ability to exclude certain params
+sub canonicalise_query {
+    my ($self, @exclude) = @_;
+
+    # Reconstruct the URL by concatenating the sorted param=value pairs
+    my @parameters;
+    foreach my $key (sort($self->param())) {
+        # Leave this key out if it's in the exclude list
+        next if lsearch(\@exclude, $key) != -1;
+
+        my $esc_key = url_quote($key);
+
+        foreach my $value ($self->param($key)) {
+            if ($value) {
+                my $esc_value = url_quote($value);
+
+                push(@parameters, "$esc_key=$esc_value");
+            }
+        }
+    }
+
+    return join("&", @parameters);
+}
+
+1;
+
+__END__
+
+=head1 NAME
+
+  Bugzilla::CGI - CGI handling for Bugzilla
+
+=head1 SYNOPSIS
+
+  use Bugzilla::CGI;
+
+  my $cgi = new Bugzilla::CGI();
+
+=head1 DESCRIPTION
+
+This package inherits from the standard CGI module, to provide additional
+Bugzilla-specific functionality. In general, see L<the CGI.pm docs|CGI> for
+documention.
+
+=head1 CHANGES FROM L<CGI.PM|CGI>
+
+Bugzilla::CGI has some differences from L<CGI.pm|CGI>.
+
+=over 4
+
+=item C<cgi_error> is automatically checked
+
+After creating the CGI object, C<Bugzilla::CGI> automatically checks
+I<cgi_error>, and throws a CodeError if a problem is detected.
+
+=back
+
+=head1 ADDITIONAL FUNCTIONS
+
+I<Bugzilla::CGI> also includes additional functions.
+
+=over 4
+
+=item C<canonicalise_query(@exclude)>
+
+This returns a sorted string of the paramaters, suitable for use in a url.
+Values in C<@exclude> are not included in the result.
+
+=back
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index 36311d6c4..9376a09fc 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -40,6 +40,7 @@ use Date::Format;
 use Date::Parse;
 
 # Create a new Search
+# Note that the param argument may be modified by Bugzilla::Search
 sub new {
     my $invocant = shift;
     my $class = ref($invocant) || $invocant;
@@ -55,7 +56,7 @@ sub new {
 sub init {
     my $self = shift;
     my $fieldsref = $self->{'fields'};
-    my $urlstr = $self->{'url'};
+    my $params = $self->{'params'};
 
     my $debug = 0;
         
@@ -64,9 +65,6 @@ sub init {
     my @wherepart;
     my @having = ("(cntuseringroups = cntbugingroups OR canseeanyway)");
     @fields = @$fieldsref if $fieldsref;
-    my %F;
-    my %M;
-    &::ParseUrlString($urlstr, \%F, \%M);
     my @specialchart;
     my @andlist;
 
@@ -96,8 +94,8 @@ sub init {
     }
 
     my $minvotes;
-    if (defined $F{'votes'}) {
-        my $c = trim($F{'votes'});
+    if (defined $params->param('votes')) {
+        my $c = trim($params->param('votes'));
         if ($c ne "") {
             if ($c !~ /^[0-9]*$/) {
                 $::vars->{'value'} = $c;
@@ -107,12 +105,12 @@ sub init {
         }
     }
 
-    if ($M{'bug_id'}) {
+    if ($params->param('bug_id')) {
         my $type = "anyexact";
-        if ($F{'bugidtype'} && $F{'bugidtype'} eq 'exclude') {
+        if ($params->param('bugidtype') && $params->param('bugidtype') eq 'exclude') {
             $type = "nowords";
         }
-        push(@specialchart, ["bug_id", $type, join(',', @{$M{'bug_id'}})]);
+        push(@specialchart, ["bug_id", $type, join(',', $params->param('bug_id'))]);
     }
 
     my @legal_fields = ("product", "version", "rep_platform", "op_sys",
@@ -120,33 +118,33 @@ sub init {
                         "assigned_to", "reporter", "component",
                         "target_milestone", "bug_group");
 
-    foreach my $field (keys %F) {
+    foreach my $field ($params->param()) {
         if (lsearch(\@legal_fields, $field) != -1) {
             push(@specialchart, [$field, "anyexact",
-                                 join(',', @{$M{$field}})]);
+                                 join(',', $params->param($field))]);
         }
     }
 
-    if ($F{'product'}) {
+    if ($params->param('product')) {
         push(@supptables, "products products_");
         push(@wherepart, "products_.id = bugs.product_id");
         push(@specialchart, ["products_.name", "anyexact",
-                             join(',',@{$M{'product'}})]);
+                             join(',',$params->param('product'))]);
     }
 
-    if ($F{'component'}) {
+    if ($params->param('component')) {
         push(@supptables, "components components_");
         push(@wherepart, "components_.id = bugs.component_id");
         push(@specialchart, ["components_.name", "anyexact",
-                             join(',',@{$M{'component'}})]);
+                             join(',',$params->param('component'))]);
     }
 
-    if ($F{'keywords'}) {
-        my $t = $F{'keywords_type'};
+    if ($params->param('keywords')) {
+        my $t = $params->param('keywords_type');
         if (!$t || $t eq "or") {
             $t = "anywords";
         }
-        push(@specialchart, ["keywords", $t, $F{'keywords'}]);
+        push(@specialchart, ["keywords", $t, $params->param('keywords')]);
     }
 
     if (lsearch($fieldsref, "(SUM(ldtime.work_time)*COUNT(DISTINCT ldtime.bug_when)/COUNT(bugs.bug_id)) AS actual_time") != -1) {
@@ -155,14 +153,14 @@ sub init {
     }
 
     foreach my $id ("1", "2") {
-        if (!defined ($F{"email$id"})) {
+        if (!defined ($params->param("email$id"))) {
             next;
         }
-        my $email = trim($F{"email$id"});
+        my $email = trim($params->param("email$id"));
         if ($email eq "") {
             next;
         }
-        my $type = $F{"emailtype$id"};
+        my $type = $params->param("emailtype$id");
         if ($type eq "exact") {
             $type = "anyexact";
             foreach my $name (split(',', $email)) {
@@ -175,11 +173,11 @@ sub init {
 
         my @clist;
         foreach my $field ("assigned_to", "reporter", "cc", "qa_contact") {
-            if ($F{"email$field$id"}) {
+            if ($params->param("email$field$id")) {
                 push(@clist, $field, $type, $email);
             }
         }
-        if ($F{"emaillongdesc$id"}) {
+        if ($params->param("emaillongdesc$id")) {
             my $table = "longdescs_";
             push(@supptables, "longdescs $table");
             push(@wherepart, "$table.bug_id = bugs.bug_id");
@@ -197,8 +195,8 @@ sub init {
     }
 
 
-    if (defined $F{'changedin'}) {
-        my $c = trim($F{'changedin'});
+    if (defined $params->param('changedin')) {
+        my $c = trim($params->param('changedin'));
         if ($c ne "") {
             if ($c !~ /^[0-9]*$/) {
                 $::vars->{'value'} = $c;
@@ -209,15 +207,15 @@ sub init {
         }
     }
 
-    my $ref = $M{'chfield'};
+    my @chfield = $params->param('chfield');
 
-    if (defined $ref) {
-        my $which = lsearch($ref, "[Bug creation]");
+    if (@chfield) {
+        my $which = lsearch(\@chfield, "[Bug creation]");
         if ($which >= 0) {
-            splice(@$ref, $which, 1);
+            splice(@chfield, $which, 1);
             push(@specialchart, ["creation_ts", "greaterthan",
-                                 SqlifyDate($F{'chfieldfrom'})]);
-            my $to = $F{'chfieldto'};
+                                 SqlifyDate($params->param('chfieldfrom'))]);
+            my $to = $params->param('chfieldto');
             if (defined $to) {
                 $to = trim($to);
                 if ($to ne "" && $to !~ /^now$/i) {
@@ -228,18 +226,18 @@ sub init {
         }
     }
 
-    if (defined $ref && 0 < @$ref) {
+    if (@chfield) {
         push(@supptables, "bugs_activity actcheck");
 
         my @list;
-        foreach my $f (@$ref) {
+        foreach my $f (@chfield) {
             push(@list, "\nactcheck.fieldid = " . &::GetFieldID($f));
         }
         push(@wherepart, "actcheck.bug_id = bugs.bug_id");
         push(@wherepart, "(" . join(' OR ', @list) . ")");
         push(@wherepart, "actcheck.bug_when >= " .
-             &::SqlQuote(SqlifyDate($F{'chfieldfrom'})));
-        my $to = $F{'chfieldto'};
+             &::SqlQuote(SqlifyDate($params->param('chfieldfrom'))));
+        my $to = $params->param('chfieldto');
         if (defined $to) {
             $to = trim($to);
             if ($to ne "" && $to !~ /^now$/i) {
@@ -247,7 +245,7 @@ sub init {
                      &::SqlQuote(SqlifyDate($to)));
             }
         }
-        my $value = $F{'chfieldvalue'};
+        my $value = $params->param('chfieldvalue');
         if (defined $value) {
             $value = trim($value);
             if ($value ne "") {
@@ -259,12 +257,12 @@ sub init {
 
     foreach my $f ("short_desc", "long_desc", "bug_file_loc",
                    "status_whiteboard") {
-        if (defined $F{$f}) {
-            my $s = trim($F{$f});
+        if (defined $params->param($f)) {
+            my $s = trim($params->param($f));
             if ($s ne "") {
                 my $n = $f;
                 my $q = &::SqlQuote($s);
-                my $type = $F{$f . "_type"};
+                my $type = $params->param($f . "_type");
                 push(@specialchart, [$f, $type, $s]);
             }
         }
@@ -516,7 +514,7 @@ sub init {
                  if ($t eq "anywords") {
                      $term = $haveawordterm;
                  } elsif ($t eq "allwords") {
-                     $ref = $funcsbykey{",$t"};
+                     my $ref = $funcsbykey{",$t"};
                      &$ref;
                      if ($term && $haveawordterm) {
                          $term = "(($term) AND $haveawordterm)";
@@ -533,7 +531,7 @@ sub init {
                 my $table = "dependson_" . $chartid;
                 push(@supptables, "dependencies $table");
                 $ff = "$table.$f";
-                $ref = $funcsbykey{",$t"};
+                my $ref = $funcsbykey{",$t"};
                 &$ref;
                 push(@wherepart, "$table.blocked = bugs.bug_id");
          },
@@ -542,7 +540,7 @@ sub init {
                 my $table = "blocked_" . $chartid;
                 push(@supptables, "dependencies $table");
                 $ff = "$table.$f";
-                $ref = $funcsbykey{",$t"};
+                my $ref = $funcsbykey{",$t"};
                 &$ref;
                 push(@wherepart, "$table.dependson = bugs.bug_id");
          },
@@ -672,9 +670,9 @@ sub init {
 
     # first we delete any sign of "Chart #-1" from the HTML form hash
     # since we want to guarantee the user didn't hide something here
-    my @badcharts = grep /^(field|type|value)-1-/, (keys %F);
+    my @badcharts = grep /^(field|type|value)-1-/, $params->param();
     foreach my $field (@badcharts) {
-        delete $F{$field};
+        $params->delete($field);
     }
 
     # now we take our special chart and stuff it into the form hash
@@ -683,11 +681,11 @@ sub init {
     foreach my $ref (@specialchart) {
         my $col = 0;
         while (@$ref) {
-            $F{"field$chart-$row-$col"} = shift(@$ref);
-            $F{"type$chart-$row-$col"} = shift(@$ref);
-            $F{"value$chart-$row-$col"} = shift(@$ref);
+            $params->param("field$chart-$row-$col", shift(@$ref));
+            $params->param("type$chart-$row-$col", shift(@$ref));
+            $params->param("value$chart-$row-$col", shift(@$ref));
             if ($debug) {
-                print qq{<p>$F{"field$chart-$row-$col"} | $F{"type$chart-$row-$col"} | $F{"value$chart-$row-$col"}*</p>\n};
+                print qq{<p>$params->param("field$chart-$row-$col") | $params->param("type$chart-$row-$col") | $params->param("value$chart-$row-$col")*</p>\n};
             }
             $col++;
 
@@ -786,19 +784,19 @@ sub init {
 
     $row = 0;
     for ($chart=-1 ;
-         $chart < 0 || exists $F{"field$chart-0-0"} ;
+         $chart < 0 || $params->param("field$chart-0-0") ;
          $chart++) {
         $chartid = $chart >= 0 ? $chart : "";
         for ($row = 0 ;
-             exists $F{"field$chart-$row-0"} ;
+             $params->param("field$chart-$row-0") ;
              $row++) {
             my @orlist;
             for (my $col = 0 ;
-                 exists $F{"field$chart-$row-$col"} ;
+                 $params->param("field$chart-$row-$col") ;
                  $col++) {
-                $f = $F{"field$chart-$row-$col"} || "noop";
-                $t = $F{"type$chart-$row-$col"} || "noop";
-                $v = $F{"value$chart-$row-$col"};
+                $f = $params->param("field$chart-$row-$col") || "noop";
+                $t = $params->param("type$chart-$row-$col") || "noop";
+                $v = $params->param("value$chart-$row-$col");
                 $v = "" if !defined $v;
                 $v = trim($v);
                 if ($f eq "noop" || $t eq "noop" || $v eq "") {
@@ -841,8 +839,8 @@ sub init {
                 }
                 else {
                     # This field and this type don't work together.
-                    $::vars->{'field'} = $F{"field$chart-$row-$col"};
-                    $::vars->{'type'} = $F{"type$chart-$row-$col"};
+                    $::vars->{'field'} = $params->param("field$chart-$row-$col");
+                    $::vars->{'type'} = $params->param("type$chart-$row-$col");
                     &::ThrowCodeError("field_type_mismatch");
                 }
             }
diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index 4d1fc3aa6..f87c6fbc6 100644
--- a/Bugzilla/Util.pm
+++ b/Bugzilla/Util.pm
@@ -27,7 +27,7 @@ package Bugzilla::Util;
 
 use base qw(Exporter);
 @Bugzilla::Util::EXPORT = qw(is_tainted trick_taint detaint_natural
-                             html_quote value_quote
+                             html_quote url_quote value_quote
                              lsearch max min
                              trim);
 
@@ -64,6 +64,13 @@ sub html_quote {
     return $var;
 }
 
+# This orignally came from CGI.pm, by Lincoln D. Stein
+sub url_quote {
+    my ($toencode) = (@_);
+    $toencode =~ s/([^a-zA-Z0-9_\-.])/uc sprintf("%%%02x",ord($1))/eg;
+    return $toencode;
+}
+
 sub value_quote {
     my ($var) = (@_);
     $var =~ s/\&/\&amp;/g;
@@ -134,6 +141,7 @@ Bugzilla::Util - Generic utility functions for bugzilla
 
   # Functions for quoting
   html_quote($var);
+  url_quote($var);
   value_quote($var);
 
   # Functions for searching
@@ -200,6 +208,10 @@ be done in the template where possible.
 Returns a value quoted for use in HTML, with &, E<lt>, E<gt>, and E<34> being
 replaced with their appropriate HTML entities.
 
+=item C<url_quote($val)>
+
+Quotes characters so that they may be included as part of a url.
+
 =item C<value_quote($val)>
 
 As well as escaping html like C<html_quote>, this routine converts newlines
diff --git a/CGI.pl b/CGI.pl
index 6ca5f2588..d6dca3a39 100644
--- a/CGI.pl
+++ b/CGI.pl
@@ -46,7 +46,6 @@ use Bugzilla::Config;
 
 sub CGI_pl_sillyness {
     my $zz;
-    $zz = %::MFORM;
     $zz = %::dontchange;
 }
 
@@ -83,151 +82,6 @@ sub url_decode {
     return $todecode;
 }
 
-# Quotify a string, suitable for putting into a URL.
-sub url_quote {
-    my($toencode) = (@_);
-    $toencode=~s/([^a-zA-Z0-9_\-.])/uc sprintf("%%%02x",ord($1))/eg;
-    return $toencode;
-}
-
-sub ParseUrlString {
-    my ($buffer, $f, $m) = (@_);
-    undef %$f;
-    undef %$m;
-
-    my %isnull;
-
-    # We must make sure that the CGI params remain tainted.
-    # This means that if for some reason you want to make this code
-    # use a regexp and $1, $2, ... (or use a helper function which does so)
-    # you must |use re 'taint'| _and_ make sure that you don't run into
-    # http://bugs.perl.org/perlbug.cgi?req=bug_id&bug_id=20020704.001
-    my @args = split('&', $buffer);
-    foreach my $arg (@args) {
-        my ($name, $value) = split('=', $arg, 2);
-        $value = '' if not defined $value;
-
-        $name = url_decode($name);
-        $value = url_decode($value);
-
-        if ($value ne "") {
-            if (defined $f->{$name}) {
-                $f->{$name} .= $value;
-                my $ref = $m->{$name};
-                push @$ref, $value;
-            } else {
-                $f->{$name} = $value;
-                $m->{$name} = [$value];
-            }
-        } else {
-            $isnull{$name} = 1;
-        }
-    }
-    if (%isnull) {
-        foreach my $name (keys(%isnull)) {
-            if (!defined $f->{$name}) {
-                $f->{$name} = "";
-                $m->{$name} = [];
-            }
-        }
-    }
-}
-
-sub ProcessFormFields {
-    my ($buffer) = (@_);
-    return ParseUrlString($buffer, \%::FORM, \%::MFORM);
-}
-
-sub ProcessMultipartFormFields {
-    my ($boundary) = @_;
-
-    # Initialize variables that store whether or not we are parsing a header,
-    # the name of the part we are parsing, and its value (which is incomplete
-    # until we finish parsing the part).
-    my $inheader = 1;
-    my $fieldname = "";
-    my $fieldvalue = "";
-
-    # Read the input stream line by line and parse it into a series of parts,
-    # each one containing a single form field and its value and each one
-    # separated from the next by the value of $boundary.
-    my $remaining = $ENV{"CONTENT_LENGTH"};
-    while ($remaining > 0 && ($_ = <STDIN>)) {
-        $remaining -= length($_);
-
-        # If the current input line is a boundary line, save the previous
-        # form value and reset the storage variables.
-        if ($_ =~ m/^-*\Q$boundary\E/) {
-            if ( $fieldname ) {
-                chomp($fieldvalue);
-                $fieldvalue =~ s/\r$//;
-                if ( defined $::FORM{$fieldname} ) {
-                    $::FORM{$fieldname} .= $fieldvalue;
-                    push @{$::MFORM{$fieldname}}, $fieldvalue;
-                } else {
-                    $::FORM{$fieldname} = $fieldvalue;
-                    $::MFORM{$fieldname} = [$fieldvalue];
-                }
-            }
-
-            $inheader = 1;
-            $fieldname = "";
-            $fieldvalue = "";
-
-        # If the current input line is a header line, look for a blank line
-        # (meaning the end of the headers), a Content-Disposition header
-        # (containing the field name and, for uploaded file parts, the file 
-        # name), or a Content-Type header (containing the content type for 
-        # file parts).
-        } elsif ( $inheader ) {
-            if (m/^\s*$/) {
-                $inheader = 0;
-            } elsif (m/^Content-Disposition:\s*form-data\s*;\s*name\s*=\s*"([^\"]+)"/i) {
-                $fieldname = $1;
-                if (m/;\s*filename\s*=\s*"([^\"]+)"/i) {
-                    $::FILE{$fieldname}->{'filename'} = $1;
-                }
-            } elsif ( m|^Content-Type:\s*([^/]+/[^\s;]+)|i ) {
-                $::FILE{$fieldname}->{'contenttype'} = $1;
-            }
-
-        # If the current input line is neither a boundary line nor a header,
-        # it must be part of the field value, so append it to the value.
-        } else {
-          $fieldvalue .= $_;
-        }
-    }
-}
-
-sub CanonicaliseParams {
-    my ($buffer, $exclude) = (@_);
-    my %pieces;
-    
-    # Split the buffer up into key/value pairs, and store the non-empty ones
-    my @args = split('&', $buffer);
-        
-    foreach my $arg (@args) {
-        my ($name, $value) = split('=', $arg, 2);
-
-        if ($value) {
-            push(@{$pieces{$name}}, $value);
-        }
-    }
-
-    # Reconstruct the URL by concatenating the sorted param=value pairs
-    my @parameters;
-    foreach my $key (sort keys %pieces) {
-        # Leave this key out if it's in the exclude list
-        next if lsearch($exclude, $key) != -1; 
-        
-        foreach my $value (@{$pieces{$key}}) {
-            push(@parameters, "$key=$value");
-        }
-    }
-    
-    return join("&", @parameters);
-}
-
 # check and see if a given field exists, is non-empty, and is set to a 
 # legal value.  assume a browser bug and abort appropriately if not.
 # if $legalsRef is not passed, just check to make sure the value exists and 
@@ -1020,52 +874,31 @@ sub GetBugActivity {
     return(\@operations, $incomplete_data);
 }
 
-
 ############# Live code below here (that is, not subroutine defs) #############
 
-$| = 1;
+use Bugzilla::CGI();
 
-# Uncommenting this next line can help debugging.
-# print "Content-type: text/html\n\nHello mom\n";
+# XXX - mod_perl, this needs to move into all the scripts individually
+# Once we do that, look into setting DISABLE_UPLOADS, and overriding
+# on a per-script basis
+$::cgi = new Bugzilla::CGI();
 
-# foreach my $k (sort(keys %ENV)) {
-#     print "$k $ENV{$k}<br>\n";
-# }
+# Set up stuff for compatibility with the old CGI.pl code
+# This code will be removed as soon as possible, in favour of
+# using the CGI.pm stuff directly
 
-if (defined $ENV{"REQUEST_METHOD"}) {
-    if ($ENV{"REQUEST_METHOD"} eq "GET") {
-        if (defined $ENV{"QUERY_STRING"}) {
-            $::buffer = $ENV{"QUERY_STRING"};
-        } else {
-            $::buffer = "";
-        }
-        ProcessFormFields $::buffer;
-    } else {
-        if (exists($ENV{"CONTENT_TYPE"}) && $ENV{"CONTENT_TYPE"} =~
-            m@multipart/form-data; boundary=\s*([^; ]+)@) {
-            ProcessMultipartFormFields($1);
-            $::buffer = "";
-        } else {
-            read STDIN, $::buffer, $ENV{"CONTENT_LENGTH"} ||
-                die "Couldn't get form data";
-            ProcessFormFields $::buffer;
-        }
-    }
+# XXX - mod_perl - reset these between runs
+
+foreach my $name ($::cgi->param()) {
+    my @val = $::cgi->param($name);
+    $::FORM{$name} = join('', @val);
+    $::MFORM{$name} = \@val;
 }
 
-if (defined $ENV{"HTTP_COOKIE"}) {
-    # Don't trust anything which came in as a cookie
-    use re 'taint';
-    foreach my $pair (split(/;/, $ENV{"HTTP_COOKIE"})) {
-        $pair = trim($pair);
-        if ($pair =~ /^([^=]*)=(.*)$/) {
-            if (!exists($::COOKIE{$1})) {
-                $::COOKIE{$1} = $2;
-            }
-        } else {
-            $::COOKIE{$pair} = "";
-        }
-    }
+$::buffer = $::cgi->query_string();
+
+foreach my $name ($::cgi->cookie()) {
+    $::COOKIE{$name} = $::cgi->cookie($name);
 }
 
 1;
diff --git a/attachment.cgi b/attachment.cgi
index b185312c6..6e9379af1 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -33,16 +33,11 @@ use strict;
 use lib qw(.);
 
 use vars qw(
+  $cgi
   $template
   $vars
 );
 
-# Win32 specific hack to avoid a hang when creating/showing an attachment
-if ($^O eq 'MSWin32') {
-    binmode(STDIN);
-    binmode(STDOUT);
-}
-
 # Include the Bugzilla CGI and general utility library.
 require "CGI.pl";
 
@@ -89,12 +84,12 @@ elsif ($action eq "insert")
   ValidateBugID($::FORM{'bugid'});
   ValidateComment($::FORM{'comment'});
   validateFilename();
-  validateData();
-  validateDescription();
   validateIsPatch();
+  my $data = validateData();
+  validateDescription();
   validateContentType() unless $::FORM{'ispatch'};
   validateObsolete() if $::FORM{'obsolete'};
-  insert();
+  insert($data);
 }
 elsif ($action eq "edit") 
 { 
@@ -198,13 +193,14 @@ sub validateContentType
   }
   elsif ($::FORM{'contenttypemethod'} eq 'autodetect')
   {
+    my $contenttype = $cgi->uploadInfo($cgi->param('data'))->{'Content-Type'};
     # The user asked us to auto-detect the content type, so use the type
     # specified in the HTTP request headers.
-    if ( !$::FILE{'data'}->{'contenttype'} )
+    if ( !$contenttype )
     {
       ThrowUserError("missing_content_type");
     }
-    $::FORM{'contenttype'} = $::FILE{'data'}->{'contenttype'};
+    $::FORM{'contenttype'} = $contenttype;
   }
   elsif ($::FORM{'contenttypemethod'} eq 'list')
   {
@@ -247,29 +243,40 @@ sub validatePrivate
 
 sub validateData
 {
-  $::FORM{'data'}
-    || ThrowUserError("zero_length_file");
+  my $maxsize = $::FORM{'ispatch'} ? Param('maxpatchsize') : Param('maxattachmentsize');
+  $maxsize *= 1024; # Convert from K
 
-  my $len = length($::FORM{'data'});
+  my $fh = $cgi->upload('data');
+  my $data;
 
-  my $maxpatchsize = Param('maxpatchsize');
-  my $maxattachmentsize = Param('maxattachmentsize');
-  
-  # Makes sure the attachment does not exceed either the "maxpatchsize" or 
-  # the "maxattachmentsize" parameter.
-  if ( $::FORM{'ispatch'} && $maxpatchsize && $len > $maxpatchsize*1024 )
+  # We could get away with reading only as much as required, except that then
+  # we wouldn't have a size to print to the error handler below.
   {
-    $vars->{'filesize'} = sprintf("%.0f", $len/1024);
-    ThrowUserError("patch_too_large");
-  } elsif ( !$::FORM{'ispatch'} && $maxattachmentsize && $len > $maxattachmentsize*1024 ) {
-    $vars->{'filesize'} = sprintf("%.0f", $len/1024);
-    ThrowUserError("file_too_large");
+      # enable 'slurp' mode
+      local $/;
+      $data = <$fh>;
   }
+
+  $data
+    || ThrowUserError("zero_length_file");
+
+  # Make sure the attachment does not exceed the maximum permitted size
+  my $len = length($data);
+  if ($maxsize && $len > $maxsize) {
+      $vars->{'filesize'} = sprintf("%.0f", $len/1024);
+      if ( $::FORM{'ispatch'} ) {
+          ThrowUserError("patch_too_large");
+      } else {
+          ThrowUserError("file_too_large");
+      }
+  }
+
+  return $data;
 }
 
 sub validateFilename
 {
-  defined $::FILE{'data'}
+  defined $cgi->upload('data')
     || ThrowUserError("file_not_specified");
 }
 
@@ -428,13 +435,15 @@ sub enter
 
 sub insert
 {
+  my ($data) = @_;
+
   # Insert a new attachment into the database.
 
   # Escape characters in strings that will be used in SQL statements.
-  my $filename = SqlQuote($::FILE{'data'}->{'filename'});
+  my $filename = SqlQuote($cgi->param('data'));
   my $description = SqlQuote($::FORM{'description'});
   my $contenttype = SqlQuote($::FORM{'contenttype'});
-  my $thedata = SqlQuote($::FORM{'data'});
+  my $thedata = SqlQuote($data);
   my $isprivate = $::FORM{'isprivate'} ? 1 : 0;
 
   # Insert the attachment into the database.
diff --git a/buglist.cgi b/buglist.cgi
index 74015bc2a..684b7dfe0 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -33,7 +33,7 @@ use strict;
 
 use lib qw(.);
 
-use vars qw($template $vars);
+use vars qw($cgi $template $vars);
 
 use Bugzilla::Search;
 
@@ -229,13 +229,17 @@ if ($::FORM{'cmdtype'} eq "runnamed") {
     $::FORM{'remaction'} = "run";
 }
 
+# The params object to use for the actual query itsself
+# This will be modified, so make a copy
+my $params = new Bugzilla::CGI($cgi);
+
 # Take appropriate action based on user's request.
 if ($::FORM{'cmdtype'} eq "dorem") {  
     if ($::FORM{'remaction'} eq "run") {
-        $::buffer = LookupNamedQuery($::FORM{"namedcmd"});
+        my $query = LookupNamedQuery($::FORM{"namedcmd"});
         $vars->{'title'} = "Bug List: $::FORM{'namedcmd'}";
-        ProcessFormFields($::buffer);
-        $order = $::FORM{'order'} || $order;
+        $params = new Bugzilla::CGI($query);
+        $order = $params->param('order') || $order;
     }
     elsif ($::FORM{'remaction'} eq "load") {
         my $url = "query.cgi?" . LookupNamedQuery($::FORM{"namedcmd"});
@@ -391,14 +395,14 @@ DefineColumn("percentage_complete","(100*((SUM(ldtime.work_time)*COUNT(DISTINCT
 # Determine the columns that will be displayed in the bug list via the 
 # columnlist CGI parameter, the user's preferences, or the default.
 my @displaycolumns = ();
-if (defined $::FORM{'columnlist'}) {
-    if ($::FORM{'columnlist'} eq "all") {
+if (defined $params->param('columnlist')) {
+    if ($params->param('columnlist') eq "all") {
         # If the value of the CGI parameter is "all", display all columns,
         # but remove the redundant "summaryfull" column.
         @displaycolumns = grep($_ ne 'summaryfull', keys(%$columns));
     }
     else {
-        @displaycolumns = split(/[ ,]+/, $::FORM{'columnlist'});
+        @displaycolumns = split(/[ ,]+/, $params->param('columnlist'));
     }
 }
 elsif (defined $::COOKIE{'COLUMNLIST'}) {
@@ -424,9 +428,10 @@ else {
 # number of votes and the votes column is not already on the list.
 
 # Some versions of perl will taint 'votes' if this is done as a single
-# statement, because $::FORM{'votes'} is tainted at this point
-$::FORM{'votes'} ||= "";
-if (trim($::FORM{'votes'}) && !grep($_ eq 'votes', @displaycolumns)) {
+# statement, because the votes param is tainted at this point
+my $votes = $params->param('votes');
+$votes ||= "";
+if (trim($votes) && !grep($_ eq 'votes', @displaycolumns)) {
     push(@displaycolumns, 'votes');
 }
 
@@ -479,7 +484,7 @@ my @selectnames = map($columns->{$_}->{'name'}, @selectcolumns);
 
 # Generate the basic SQL query that will be used to generate the bug list.
 my $search = new Bugzilla::Search('fields' => \@selectnames, 
-                                  'url' => $::buffer);
+                                  'params' => $params);
 my $query = $search->getSQL();
 
 
@@ -489,7 +494,7 @@ my $query = $search->getSQL();
 
 # Add to the query some instructions for sorting the bug list.
 if ($::COOKIE{'LASTORDER'} && (!$order || $order =~ /^reuse/i)) {
-    $order = url_decode($::COOKIE{'LASTORDER'});
+    $order = $::COOKIE{'LASTORDER'};
     $order_from_cookie = 1;
 }
 
diff --git a/checksetup.pl b/checksetup.pl
index 9f22ae1f5..1acec457c 100755
--- a/checksetup.pl
+++ b/checksetup.pl
@@ -179,6 +179,13 @@ sub have_vers {
   $vnum = ${"${pkg}::VERSION"} || ${"${pkg}::Version"} || 0;
   $vnum = -1 if $@;
 
+  # CGI's versioning scheme went 2.75, 2.751, 2.752, 2.753, 2.76
+  # That breaks the standard version tests, so we need to manually correct
+  # the version
+  if ($pkg eq 'CGI' && $vnum =~ /(2\.7\d)(\d+)/) {
+      $vnum = $1 . "." . $2;
+  }
+
   if ($vnum eq "-1") { # string compare just in case it's non-numeric
     $vstr = "not found";
   }
@@ -201,8 +208,8 @@ my $modules = [
         version => '1.52' 
     }, 
     { 
-        name => 'CGI::Carp', 
-        version => '0' 
+        name => 'CGI', 
+        version => '2.88' 
     }, 
     {
         name => 'Data::Dumper', 
diff --git a/globals.pl b/globals.pl
index da954181a..bee0ed9ff 100644
--- a/globals.pl
+++ b/globals.pl
@@ -1584,7 +1584,7 @@ $::template ||= Template->new(
         # characters NOT in the regex set: [a-zA-Z0-9_\-.].  The 'uri'
         # filter should be used for a full URL that may have
         # characters that need encoding.
-        url_quote => \&url_quote ,
+        url_quote => \&Bugzilla::Util::url_quote,
         
         # In CSV, quotes are doubled, and any value containing a quote or a
         # comma is enclosed in quotes.
diff --git a/process_bug.cgi b/process_bug.cgi
index 54ed0dc8f..47f038e52 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -697,7 +697,11 @@ if (Param("usebugaliases") && defined($::FORM{'alias'})) {
         # with that value.
         DoComma();
         $::query .= "alias = ";
-        $::query .= ($alias eq "") ? "NULL" : SqlQuote($alias);
+        if ($alias eq "") {
+            $::query .= "NULL";
+        } else {
+            $::query .= SqlQuote($alias);
+        }
     }
 }
 
diff --git a/report.cgi b/report.cgi
index 9e60c1dc9..f4cb74dad 100755
--- a/report.cgi
+++ b/report.cgi
@@ -26,7 +26,7 @@ use lib ".";
 
 require "CGI.pl";
 
-use vars qw($template $vars);
+use vars qw($cgi $template $vars);
 
 use Bugzilla::Search;
 
@@ -77,11 +77,13 @@ my @axis_fields = ($row_field, $col_field, $tbl_field);
 
 my @selectnames = map($columns{$_}, @axis_fields);
 
+# Clone the params, so that Bugzilla::Search can modify them
+my $params = new Bugzilla::CGI($cgi);
 my $search = new Bugzilla::Search('fields' => \@selectnames, 
-                                  'url' => $::buffer);
+                                  'params' => $params);
 my $query = $search->getSQL();
 
-SendSQL($query, $::userid);
+SendSQL($query);
 
 # We have a hash of hashes for the data itself, and a hash to hold the 
 # row/col/table names.
@@ -108,12 +110,14 @@ $vars->{'names'} = \%names;
 $vars->{'data'} = \%data;
 $vars->{'time'} = time();
 
-$::buffer =~ s/format=[^&]*&?//g;
+$cgi->delete('format');
 
 # Calculate the base query URL for the hyperlinked numbers
-$vars->{'buglistbase'} = CanonicaliseParams($::buffer, 
-                ["x_axis_field", "y_axis_field", "z_axis_field", @axis_fields]);
-$vars->{'buffer'} = $::buffer;
+$vars->{'querybase'} = $cgi->canonicalise_query("x_axis_field",
+                                                "y_axis_field",
+                                                "z_axis_field",
+                                                @axis_fields);
+$vars->{'query'} = $cgi->query_string();
 
 # Generate and return the result from the appropriate template.
 my $format = GetFormat("reports/report", $::FORM{'format'}, $::FORM{'ctype'});
diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl
index 1ec1c4626..baad2f5f0 100644
--- a/template/en/default/global/code-error.html.tmpl
+++ b/template/en/default/global/code-error.html.tmpl
@@ -47,6 +47,11 @@
   [% ELSIF error == "attachment_already_obsolete" %]
     Attachment #[% attachid FILTER html %] ([% description FILTER html %]) 
     is already obsolete.
+
+  [% ELSIF error == "cgi_error" %]
+    [% title = "CGI Error" %]
+    Bugzilla has had trouble interpreting your CGI request;
+    [%+ Param('browserbugmessage') %]
     
   [% ELSIF error == "chart_data_not_generated" %]
     The tool which gathers bug counts has not been run yet.
@@ -236,7 +241,7 @@
   <pre>
 Variables:
   [% FOREACH key = variables.keys %]
-    [%+ key %]: [%+ variables.$key %]
+    [%+ key FILTER html %]: [%+ variables.$key FILTER html %]
   [% END %]
   </pre>
 [% END %]
diff --git a/template/en/default/reports/report-table.html.tmpl b/template/en/default/reports/report-table.html.tmpl
index 97dae5b48..9767f5030 100644
--- a/template/en/default/reports/report-table.html.tmpl
+++ b/template/en/default/reports/report-table.html.tmpl
@@ -21,7 +21,8 @@
   #%]
 
 [%# INTERFACE:
-  # basequery: The base query for this table, in URL form
+  # querybase: The base query for this table, in URL form
+  # query: The query for this table, in URL form
   # data: hash of hash of hash of numbers. Bug counts.
   # names: hash of hash of strings. Names of tables, rows and columns.
   # col_field: string. Name of the field being plotted as columns.
@@ -149,7 +150,7 @@
         [% col_idx = 1 - col_idx %]
         <td class="[% classes.$row_idx.$col_idx %]" align="center">
           [% IF data.$tbl.$col.$row AND data.$tbl.$col.$row > 0 %]
-            <a href="buglist.cgi?[% buglistbase %]&
+            <a href="buglist.cgi?[% querybase FILTER html %]&amp;
               [% tbl_field FILTER url_quote %]=[% tbl FILTER url_quote %]&amp;
               [% row_field FILTER url_quote %]=[% row FILTER url_quote %]&amp;
               [% col_field FILTER url_quote %]=[% col FILTER url_quote %]">
@@ -160,7 +161,7 @@
         </td>
       [% END %] 
       <td class="ttotal" align="right">
-        <a href="buglist.cgi?[% buglistbase %]&
+        <a href="buglist.cgi?[% querybase FILTER html %]&amp;
           [% tbl_field FILTER url_quote %]=[% tbl FILTER url_quote %]&amp;
           [% row_field FILTER url_quote %]=[% row FILTER url_quote %]">
         [% row_total %]</a>
@@ -178,7 +179,7 @@
       [% NEXT IF col == "" %]
       
       <td class="ttotal" align="center">
-        <a href="buglist.cgi?[% buglistbase %]&
+        <a href="buglist.cgi?[% querybase FILTER html %]&amp;
           [% tbl_field FILTER url_quote %]=[% tbl FILTER url_quote %]&amp;
           [% col_field FILTER url_quote %]=[% col FILTER url_quote %]">
         [% col_totals.$col %]</a>
@@ -187,7 +188,7 @@
     [% END %]
     <td class="ttotal" align="right">
       <strong>
-        <a href="buglist.cgi?[% buglistbase %]">[% grand_total %]</a>
+        <a href="buglist.cgi?[% querybase FILTER html %]">[% grand_total %]</a>
       </strong>
     </td>
   </tr>
@@ -202,7 +203,7 @@
 
 [% END %]
   
-  <a href="query.cgi?[% buffer %]&format=report-table">Edit this report</a>
+  <a href="query.cgi?[% query FILTER html %]&amp;format=report-table">Edit this report</a>
 </div>
 
 <br>