From 874e96c2423c772564c9dc63254baa99e86f270b Mon Sep 17 00:00:00 2001 From: Dylan William Hardison Date: Sat, 12 Nov 2016 11:41:08 -0500 Subject: Bug 1314201 - ThrowUserError and ThrowCodeError should print headers if headers have not already been printed --- Bugzilla/CGI.pm | 23 +++++++++++++++++++---- Bugzilla/Error.pm | 4 ++-- Bugzilla/Template.pm | 13 ++++++++++++- template/en/default/global/code-error.html.tmpl | 4 ++++ 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm index 78987ab71..ec8b8e52b 100644 --- a/Bugzilla/CGI.pm +++ b/Bugzilla/CGI.pm @@ -21,6 +21,7 @@ use Bugzilla::Search::Recent; use File::Basename; use URI; +use Carp qw(cluck); BEGIN { if (ON_WINDOWS) { @@ -306,7 +307,6 @@ sub check_etag { return 0; } -# Overwrite to ensure nph doesn't get set, and unset HEADERS_ONCE sub multipart_init { my $self = shift; @@ -330,6 +330,7 @@ sub multipart_init { # CGI.pm's header() sets nph according to a param or $CGI::NPH, which # is the desired behaviour. + $self->{_bz_multipart} = 1; return $self->header( %param, ) . "WARNING: YOUR BROWSER DOESN'T SUPPORT THIS SERVER-PUSH TECHNOLOGY." . $self->multipart_end; @@ -384,7 +385,12 @@ sub header { my %headers; my $user = Bugzilla->user; - # If there's only one parameter, then it's a Content-Type. + if ($self->{_bz_headers_sent} && !$self->{_bz_multipart}) { + # cluck for the warning log so we can see where this was called. + cluck "attempt to send headers after headers already sent!"; + ThrowCodeError("headers_already_sent"); + } + if (scalar(@_) == 1) { %headers = ('-type' => shift(@_)); } @@ -460,9 +466,18 @@ sub header { Bugzilla::Hook::process('cgi_headers', { cgi => $self, headers => \%headers } ); - $self->{_header_done} = 1; - return $self->SUPER::header(%headers) || ""; + my $headers = $self->SUPER::header(%headers) || ""; + if ($headers && Bugzilla->usage_mode != USAGE_MODE_XMLRPC) { + $self->{_bz_headers_sent} = 1; + } + + return $headers; +} + +sub sent_headers { + my ($self) = @_; + return $self->{_bz_headers_sent}; } sub param { diff --git a/Bugzilla/Error.pm b/Bugzilla/Error.pm index fc0e4812e..226078f9c 100644 --- a/Bugzilla/Error.pm +++ b/Bugzilla/Error.pm @@ -112,7 +112,7 @@ sub _throw_error { } my $cgi = Bugzilla->cgi; - $cgi->close_standby_message('text/html', 'inline', 'error', 'html'); + $cgi->close_standby_message('text/html', 'inline', 'error', 'html') unless $cgi->sent_headers; $template->process($name, $vars) || ThrowTemplateError($template->error()); print $cgi->multipart_final() if $cgi->{_multipart_in_progress}; @@ -279,7 +279,7 @@ sub ThrowErrorPage { my $template = Bugzilla->template; my $vars = {}; $vars->{message} = $message; - print $cgi->header(); + print $cgi->header() unless $cgi->sent_headers(); $template->process($template_name, $vars) || ThrowTemplateError($template->error()); exit; diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm index eb1496fca..dec9885c2 100644 --- a/Bugzilla/Template.pm +++ b/Bugzilla/Template.pm @@ -653,10 +653,21 @@ our $is_processing = 0; sub process { my $self = shift; + my ($template, undef, $output) = @_; + + if (!$output && Bugzilla->usage_mode == USAGE_MODE_BROWSER) { + # if $output is not passed, this will print. + my $cgi = Bugzilla->cgi; + unless ($cgi->sent_headers) { + warn "attempted to process $template before sending headers!"; + print $cgi->header(); + } + } + # All of this current_langs stuff allows template_inner to correctly # determine what-language Template object it should instantiate. my $current_langs = Bugzilla->request_cache->{template_current_lang} ||= []; - unshift(@$current_langs, $self->context->{bz_language}); + unshift @$current_langs, $self->context->{bz_language}; local $is_processing = 1; my $retval = $self->SUPER::process(@_); shift @$current_langs; diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl index 18650de7d..f78c954d7 100644 --- a/template/en/default/global/code-error.html.tmpl +++ b/template/en/default/global/code-error.html.tmpl @@ -198,6 +198,10 @@ [% title = "Field Type Not Specified" %] You must specify a type when creating a custom field. + [% ELSIF error == "headers_already_sent" %] + [% title = "Tried to send headers twice" %] + Somewhere inside [% terms.Bugzilla %], code attempted to sent HTTP headers more than once. + [% ELSIF error == "illegal_content_type_method" %] Your form submission got corrupted somehow. The content method field, which specifies how the content type gets determined, -- cgit v1.2.3-24-g4f1b