Bug 201816 - use CGI.pm for header output

r=joel, a=justdave

6 files changed, 103 insertions, 27 deletions

diff --git a/Bugzilla/Auth/CGI.pm b/Bugzilla/Auth/CGI.pm
index 034013bda..3588b7037 100644
--- a/Bugzilla/Auth/CGI.pm
+++ b/Bugzilla/Auth/CGI.pm
@@ -70,9 +70,13 @@ sub login {
                  undef,
                  $userid, $ipaddr);
         my $logincookie = $dbh->selectrow_array("SELECT LAST_INSERT_ID()");
-        my $cookiepath = Param("cookiepath");
-        print "Set-Cookie: Bugzilla_login=$userid ; path=$cookiepath; expires=Sun, 30-Jun-2029 00:00:00 GMT\n";
-        print "Set-Cookie: Bugzilla_logincookie=$logincookie ; path=$cookiepath; expires=Sun, 30-Jun-2029 00:00:00 GMT\n";
+
+        $cgi->send_cookie(-name => 'Bugzilla_login',
+                          -value => $userid,
+                          -expires => 'Fri, 01-Jan-2038 00:00:00 GMT');
+        $cgi->send_cookie(-name => 'Bugzilla_logincookie',
+                          -value => $logincookie,
+                          -expires => 'Fri, 01-Jan-2038 00:00:00 GMT');
 
         # compat code. The cookie value is used for logouts, and that
         # isn't generic yet.
@@ -120,7 +124,7 @@ sub login {
     if ($authres == AUTH_NODATA && $type == LOGIN_REQUIRED) {
         # Throw up the login page
 
-        print "Content-Type: text/html\n\n";
+        print Bugzilla->cgi->header();
 
         my $template = Bugzilla->template;
         $template->process("account/auth/login.html.tmpl",
@@ -152,9 +156,12 @@ sub login {
     # The account may be disabled
     if ($authres == AUTH_DISABLED) {
         # Clear the cookie
-        my $cookiepath = Param("cookiepath");
-        print "Set-Cookie: Bugzilla_login= ; path=$cookiepath; expires=Sun, 30-Jun-80 00:00:00 GMT\n";
-        print "Set-Cookie: Bugzilla_logincookie= ; path=$cookiepath; expires=Sun, 30-Jun-80 00:00:00 GMT\n";
+
+        $cgi->send_cookie(-name => 'Bugzilla_login',
+                          -expires => "Tue, 15-Sep-1998 21:49:00 GMT");
+        $cgi->send_cookie(-name => 'Bugzilla_logincookie',
+                          -expires => "Tue, 15-Sep-1998 21:49:00 GMT");
+
         # and throw a user error
         &::ThrowUserError("account_disabled",
                           {'disabled_reason' => $extra});
diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm
index 3c00ed347..e87c89a5e 100644
--- a/Bugzilla/CGI.pm
+++ b/Bugzilla/CGI.pm
@@ -23,11 +23,12 @@ use strict;
 
 package Bugzilla::CGI;
 
-use CGI qw(-no_xhtml -oldstyle_urls :private_tempfiles);
+use CGI qw(-no_xhtml -oldstyle_urls :private_tempfiles :unique_headers);
 
 use base qw(CGI);
 
 use Bugzilla::Util;
+use Bugzilla::Config;
 
 # We need to disable output buffering - see bug 179174
 $| = 1;
@@ -44,6 +45,9 @@ sub new {
 
     my $self = $class->SUPER::new(@args);
 
+    # Make sure that we don't send any charset headers
+    $self->charset('');
+
     # Check for errors
     # All of the Bugzilla code wants to do this, so do it here instead of
     # in each script
@@ -62,20 +66,18 @@ sub new {
         # 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);
+        print $self->header(-status => $err);
+
+        # ThrowCodeError wants to print the header, so it grabs Bugzilla->cgi
+        # which creates a new Bugzilla::CGI object, which fails again, which
+        # ends up here, and calls ThrowCodeError, and then recurses forever.
+        # So don't use it.
+        # In fact, we can't use templates at all, because we need a CGI object
+        # to determine the template lang as well as the current url (from the
+        # template)
+        # Since this is an internal error which indicates a severe browser bug,
+        # just die.
+        die "CGI parsing error: $err";
     }
 
     return $self;
@@ -105,6 +107,46 @@ sub canonicalise_query {
     return join("&", @parameters);
 }
 
+# CGI.pm makes this nph, but apache doesn't like that
+sub multipart_init {
+    my $self = shift;
+
+    unshift(@_, '-nph' => undef);
+
+    return $self->SUPER::multipart_init(@_);
+}
+
+sub cookie {
+    my $self = shift;
+
+    # Add the default path in, but only if we're fetching stuff
+    # (This test fails for |$cgi->cookie(-name=>'x')| which _is_ meant to
+    # fetch, but thats an ugly notation for the fetch case which we shouldn't
+    # be using)
+    unshift(@_, '-path' => Param('cookiepath')) if scalar(@_)>1;
+
+    return $self->SUPER::cookie(@_);
+}
+
+# The various parts of Bugzilla which create cookies don't want to have to
+# pass them arround to all of the callers. Instead, store them locally here,
+# and then output as required from |headers|.
+# This is done instead of just printing the result from the script, because
+# we need to use |$r->header_out| under mod_perl (which is what CGI.pm
+# does, and we need to match, plus if we don't |print| anything, we can turn
+# off mod_perl/Apache's header parsing for a small perf gain)
+sub send_cookie {
+    my $self = shift;
+
+    my $cookie = $self->cookie(@_);
+
+    # XXX - mod_perl
+    print "Set-Cookie: $cookie\r\n";
+
+    return;
+}
+
+
 1;
 
 __END__
@@ -149,4 +191,21 @@ I<Bugzilla::CGI> also includes additional functions.
 This returns a sorted string of the parameters, suitable for use in a url.
 Values in C<@exclude> are not included in the result.
 
+=item C<cookie>
+
+Identical to the CGI.pm C<cookie> routine, except that the cookie path is
+automatically added.
+
+=item C<send_cookie>
+
+This routine is identical to CGI.pm's C<cookie> routine, except that the cookie
+is sent to the browser, rather than returned. This should be used by all
+Bugzilla code (instead of C<cookie> or the C<-cookie> argument to C<header>),
+so that under mod_perl the headers can be sent correctly, using C<print> or
+the mod_perl APIs as appropriate.
+
 =back
+
+=head1 SEE ALSO
+
+L<CGI|CGI>, L<CGI::Cookie|CGI::Cookie>
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 5e6b5365d..a1bf74ba0 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -47,7 +47,8 @@ use base qw(Exporter);
     LOGIN_NORMAL
     LOGIN_REQUIRED
 );
-   
+
+@Bugzilla::Constants::EXPORT_OK = qw(contenttypes);
 
 # CONSTANTS
 #
@@ -94,4 +95,14 @@ use constant LOGIN_OPTIONAL => 0;
 use constant LOGIN_NORMAL => 1;
 use constant LOGIN_REQUIRED => 2;
 
+use constant contenttypes =>
+  {
+   "html" => "text/html" , 
+   "rdf" => "application/xml" , 
+   "xml" => "text/xml" , 
+   "js" => "application/x-javascript" , 
+   "csv" => "text/plain" ,
+   "png" => "image/png" ,
+  };
+
 1;
diff --git a/Bugzilla/Error.pm b/Bugzilla/Error.pm
index 64314121a..485646274 100644
--- a/Bugzilla/Error.pm
+++ b/Bugzilla/Error.pm
@@ -39,8 +39,7 @@ sub ThrowUserError {
 
     Bugzilla->dbh->do("UNLOCK TABLES") if $unlock_tables;
 
-    # XXX - mod_perl
-    print "Content-type: text/html\n\n" if !$::vars->{'header_done'};
+    print Bugzilla->cgi->header();
 
     my $template = Bugzilla->template;
     $template->process("global/user-error.html.tmpl", $vars)
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm
index a327f2922..f8eb8a4a4 100644
--- a/Bugzilla/Flag.pm
+++ b/Bugzilla/Flag.pm
@@ -587,7 +587,7 @@ sub notify {
     my $rv = 
       $::template->process($template_file, $::vars, \$message);
     if (!$rv) {
-        print "Content-Type: text/html\n\n" unless $::vars->{'header_done'};
+        Bugzilla->cgi->header();
         &::ThrowTemplateError($::template->error());
     }
     
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 46f520b77..fde9d336b 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -366,7 +366,7 @@ sub match_field {
     $vars->{'matches'}       = $matches; # matches that were made
     $vars->{'matchsuccess'}  = $matchsuccess; # continue or fail
 
-    print "Content-type: text/html\n\n";
+    print Bugzilla->cgi->header();
 
     $::template->process("global/confirm-user-match.html.tmpl", $vars)
       || &::ThrowTemplateError($::template->error());