Change the default message for errors (for fatalsToBrowser) to use die_error() subroutine. This way errors (and explicitely calling 'die MESSAGE') would generate 'Internal Server Error' error message. Note that call to set_message is intentionally not put in BEGIN block; we set error handler to use die_error() only after we are sure that we can use it, after all needed variables are set. Due to the fact that error handler set via set_message() subroutine from CGI::Carp (in the fatalsToBrowser case) is called after HTTP headers were already printed (with exception of MOD_PERL), gitweb cannot return 'Status: 500 Internal Server Error'. Thanks to the fact that die_error() no longer uses 'exit', errors would be logged by CGI::Carp, independent on whether default error handler is used, or handle_errors_html which uses die_error is used. Signed-off-by: Jakub Narebski <jnareb@xxxxxxxxx> --- Because of the fact that with CGI::Carp::set_message and die_error you cannot return proper HTTP status code, I have decided that caching engine needs support for custom error handler anyway, a la 'on_get_error' and 'on_set_error' in CHI. With this patch most error messages, from fatal errors in gitweb or Perl, should get look of the rest of gitweb. This is good idea even without caching support. gitweb/gitweb.perl | 27 +++++++++++++++++++++++---- 1 files changed, 23 insertions(+), 4 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index ed92dca..e579c14 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -11,7 +11,7 @@ use strict; use warnings; use CGI qw(:standard :escapeHTML -nosticky); use CGI::Util qw(unescape); -use CGI::Carp qw(fatalsToBrowser); +use CGI::Carp qw(fatalsToBrowser set_message); use Encode; use Fcntl ':mode'; use File::Find qw(); @@ -952,6 +952,21 @@ if ($git_avatar eq 'gravatar') { $git_avatar = ''; } +# custom error handler: 'die <message>' is Internal Server Error +sub handle_errors_html { + my $msg = shift; # it is already HTML escaped + + # to avoid infinite loop where error occurs in die_error, + # change handler to default handler, disabling handle_errors_html + set_message("Error occured when inside die_error:\n$msg"); + + # you cannot jump out of die_error when called as error handler; + # the subroutine set via CGI::Carp::set_message is called _after_ + # HTTP headers are already written, so it cannot write them itself + die_error(undef, undef, $msg, -error_handler => 1, -no_http_header => 1); +} +set_message(\&handle_errors_html); + # dispatch if (!defined $action) { if (defined $hash) { @@ -3167,6 +3182,7 @@ sub blob_contenttype { sub git_header_html { my $status = shift || "200 OK"; my $expires = shift; + my %opts = @_; my $title = "$site_name"; if (defined $project) { @@ -3194,7 +3210,8 @@ sub git_header_html { $content_type = 'text/html'; } print $cgi->header(-type=>$content_type, -charset => 'utf-8', - -status=> $status, -expires => $expires); + -status=> $status, -expires => $expires) + unless ($opts{'-no_http_headers'}); my $mod_perl_version = $ENV{'MOD_PERL'} ? " $ENV{'MOD_PERL'}" : ''; print <<EOF; <?xml version="1.0" encoding="utf-8"?> @@ -3411,6 +3428,7 @@ sub die_error { my $status = shift || 500; my $error = esc_html(shift) || "Internal Server Error"; my $extra = shift; + my %opts = @_; my %http_responses = ( 400 => '400 Bad Request', @@ -3419,7 +3437,7 @@ sub die_error { 500 => '500 Internal Server Error', 503 => '503 Service Unavailable', ); - git_header_html($http_responses{$status}); + git_header_html($http_responses{$status}, undef, %opts); print <<EOF; <div class="page_body"> <br /><br /> @@ -3433,7 +3451,8 @@ EOF print "</div>\n"; git_footer_html(); - goto DONE_GITWEB; + goto DONE_GITWEB + unless ($opts{'-error_handler'}); } ## ---------------------------------------------------------------------- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html