Commit 871de5e2 authored by David Lawrence's avatar David Lawrence

Bug 1240029 - The REST API doesn't work correctly with mod_perl

r=dylan
parent 864d9b8f
...@@ -211,12 +211,18 @@ sub _handle { ...@@ -211,12 +211,18 @@ sub _handle {
my $method = $self->method_name; my $method = $self->method_name;
my $controller = $self->controller; my $controller = $self->controller;
my $params = Bugzilla->input_params; my $params = Bugzilla->input_params;
my $cache = Bugzilla->request_cache;
unless ($controller->can($method)) { unless ($controller->can($method)) {
return $self->return_error(302, "No such a method : '$method'."); return $self->return_error(302, "No such a method : '$method'.");
} }
my $result = eval q| $controller->$method($params) |; # Let Bugzilla::Error know we are inside an eval() for exceptions
$cache->{in_eval} = 1;
my $result = eval { $controller->$method($params) };
$cache->{in_eval} = 0;
return $self->return_error if $self->return_error;
if ($@) { if ($@) {
return $self->return_error(500, "Procedure error: $@"); return $self->return_error(500, "Procedure error: $@");
......
...@@ -23,30 +23,14 @@ use Carp; ...@@ -23,30 +23,14 @@ use Carp;
use Data::Dumper; use Data::Dumper;
use Date::Format; use Date::Format;
# We cannot use $^S to detect if we are in an eval(), because mod_perl
# already eval'uates everything, so $^S = 1 in all cases under mod_perl!
sub _in_eval {
my $in_eval = 0;
for (my $stack = 1; my $sub = (caller($stack))[3]; $stack++) {
last if $sub =~ /^(?:ModPerl|Plack|CGI::Compile)/;
# An eval followed by CGI::Compile is not a "real" eval.
$in_eval = 1 if $sub =~ /^\(eval\)/ && (caller($stack + 1))[3] !~ /^CGI::Compile/;
}
return $in_eval;
}
sub _throw_error { sub _throw_error {
my ($name, $error, $vars) = @_; my ($name, $error, $vars) = @_;
my $dbh = Bugzilla->dbh; my $dbh = Bugzilla->dbh;
my $cache = Bugzilla->request_cache;
$vars ||= {}; $vars ||= {};
$vars->{error} = $error; $vars->{error} = $error;
# Make sure any transaction is rolled back (if supported).
# If we are within an eval(), do not roll back transactions as we are
# eval'uating some test on purpose.
$dbh->bz_rollback_transaction() if ($dbh->bz_in_transaction() && !_in_eval());
my $datadir = bz_locations()->{'datadir'}; my $datadir = bz_locations()->{'datadir'};
# If a writable $datadir/errorlog exists, log error details there. # If a writable $datadir/errorlog exists, log error details there.
if (-w "$datadir/errorlog") { if (-w "$datadir/errorlog") {
...@@ -138,7 +122,7 @@ sub _throw_error { ...@@ -138,7 +122,7 @@ sub _throw_error {
# of JSON::RPC. So, in that circumstance, instead of exiting, # of JSON::RPC. So, in that circumstance, instead of exiting,
# we die with no message. JSON::RPC checks raise_error before # we die with no message. JSON::RPC checks raise_error before
# it checks $@, so it returns the proper error. # it checks $@, so it returns the proper error.
die if _in_eval(); die if $cache->{in_eval};
$server->response($server->error_response_header); $server->response($server->error_response_header);
} }
else { else {
...@@ -146,6 +130,7 @@ sub _throw_error { ...@@ -146,6 +130,7 @@ sub _throw_error {
my %status_code_map = %{ $server->constants->{REST_STATUS_CODE_MAP} }; my %status_code_map = %{ $server->constants->{REST_STATUS_CODE_MAP} };
my $status_code = $status_code_map{$code} || $status_code_map{'_default'}; my $status_code = $status_code_map{$code} || $status_code_map{'_default'};
$server->return_error($status_code, $message, $code); $server->return_error($status_code, $message, $code);
die if $cache->{in_eval};
$server->response; $server->response;
} }
} }
......
...@@ -273,6 +273,9 @@ sub _handle { ...@@ -273,6 +273,9 @@ sub _handle {
my $result = $self->SUPER::_handle(@_); my $result = $self->SUPER::_handle(@_);
# Reset in_eval to so we get normal exceptions from here
Bugzilla->request_cache->{in_eval} = 0;
# Set the ETag if not already set in the webservice methods. # Set the ETag if not already set in the webservice methods.
my $etag = $self->bz_etag; my $etag = $self->bz_etag;
if (!$etag && ref $result) { if (!$etag && ref $result) {
...@@ -425,6 +428,9 @@ sub _argument_type_check { ...@@ -425,6 +428,9 @@ sub _argument_type_check {
$params = [$params]; $params = [$params];
} }
# Let Bugzilla::Error know we are inside an eval() after this point
Bugzilla->request_cache->{in_eval} = 1;
return $params; return $params;
} }
......
...@@ -466,7 +466,7 @@ sub die_handler { ...@@ -466,7 +466,7 @@ sub die_handler {
# If this is inside an eval, then we should just act like...we're # If this is inside an eval, then we should just act like...we're
# in an eval (instead of printing the error and exiting). # in an eval (instead of printing the error and exiting).
die @_ if ($^S // Bugzilla::Error::_in_eval()); die @_ if ($^S // Bugzilla->request_cache->{in_eval});
# We can't depend on the MTA to send an error message, so we have # We can't depend on the MTA to send an error message, so we have
# to generate one properly. # to generate one properly.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment