зеркало из https://github.com/mozilla/pjs.git
Bug 277782: _throw_error should unlock tables when tables are locked, automatically
Patch By Tomas Kopal <Tomas.Kopal@altap.cz> r=travis, r=LpSolit, a=justdave
This commit is contained in:
mkanat%kerio.com
Родитель
2cde039f4b
Коммит
82ff1ef567
|
|
@ -184,7 +184,7 @@ sub login {
|
|||
|
||||
# If we get here, then we've run out of options, which shouldn't happen
|
||||
ThrowCodeError("authres_unhandled", { authres => $authres,
|
||||
type => $type, });
|
||||
type => $type });
|
||||
}
|
||||
|
||||
# This auth style allows the user to log out.
|
||||
|
|
|
|||
|
|
@ -511,21 +511,18 @@ sub ValidateTime {
|
|||
# (allow negatives, though, so people can back out errors in time reporting)
|
||||
if ($time !~ /^-?(?:\d+(?:\.\d*)?|\.\d+)$/) {
|
||||
ThrowUserError("number_not_numeric",
|
||||
{field => "$field", num => "$time"},
|
||||
"abort");
|
||||
{field => "$field", num => "$time"});
|
||||
}
|
||||
|
||||
# Only the "work_time" field is allowed to contain a negative value.
|
||||
if ( ($time < 0) && ($field ne "work_time") ) {
|
||||
ThrowUserError("number_too_small",
|
||||
{field => "$field", num => "$time", min_num => "0"},
|
||||
"abort");
|
||||
{field => "$field", num => "$time", min_num => "0"});
|
||||
}
|
||||
|
||||
if ($time > 99999.99) {
|
||||
ThrowUserError("number_too_large",
|
||||
{field => "$field", num => "$time", max_num => "99999.99"},
|
||||
"abort");
|
||||
{field => "$field", num => "$time", max_num => "99999.99"});
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -32,13 +32,15 @@ use Bugzilla::Util;
|
|||
use Date::Format;
|
||||
|
||||
sub _throw_error {
|
||||
my ($name, $error, $vars, $unlock_tables) = @_;
|
||||
my ($name, $error, $vars) = @_;
|
||||
|
||||
$vars ||= {};
|
||||
|
||||
$vars->{error} = $error;
|
||||
|
||||
Bugzilla->dbh->bz_unlock_tables(UNLOCK_ABORT) if $unlock_tables;
|
||||
# Make sure any locked tables are unlocked
|
||||
# and the transaction is rolled back (if supported)
|
||||
Bugzilla->dbh->bz_unlock_tables(UNLOCK_ABORT);
|
||||
|
||||
# If a writable data/errorlog exists, log error details there.
|
||||
if (-w "data/errorlog") {
|
||||
|
|
@ -95,6 +97,10 @@ sub ThrowCodeError {
|
|||
sub ThrowTemplateError {
|
||||
my ($template_err) = @_;
|
||||
|
||||
# Make sure any locked tables are unlocked
|
||||
# and the transaction is rolled back (if supported)
|
||||
Bugzilla->dbh->bz_unlock_tables(UNLOCK_ABORT);
|
||||
|
||||
my $vars = {};
|
||||
if (Bugzilla->batch) {
|
||||
die("error: template error: $template_err");
|
||||
|
|
@ -149,16 +155,16 @@ Bugzilla::Error - Error handling utilities for Bugzilla
|
|||
ThrowUserError("error_tag",
|
||||
{ foo => 'bar' });
|
||||
|
||||
# supplying "abort" to ensure tables are unlocked
|
||||
ThrowUserError("another_error_tag",
|
||||
{ foo => 'bar' }, 'abort');
|
||||
|
||||
=head1 DESCRIPTION
|
||||
|
||||
Various places throughout the Bugzilla codebase need to report errors to the
|
||||
user. The C<Throw*Error> family of functions allow this to be done in a
|
||||
generic and localisable manner.
|
||||
|
||||
These functions automatically unlock the database tables, if there were any
|
||||
locked. They will also roll back the transaction, if it is supported by
|
||||
the underlying DB.
|
||||
|
||||
=head1 FUNCTIONS
|
||||
|
||||
=over 4
|
||||
|
|
@ -170,12 +176,6 @@ of variables as a second argument. These are used by the
|
|||
I<global/user-error.html.tmpl> template to format the error, using the passed
|
||||
in variables as required.
|
||||
|
||||
An optional third argument may be supplied. If present, the error
|
||||
handling code will unlock the database tables: it is a Bugzilla standard
|
||||
to provide the string "abort" as the argument value. In the long term,
|
||||
this argument will go away, to be replaced by transactional C<rollback>
|
||||
calls. There is no timeframe for doing so, however.
|
||||
|
||||
=item C<ThrowCodeError>
|
||||
|
||||
This function is used when an internal check detects an error of some sort.
|
||||
|
|
|
|||
|
|
@ -112,7 +112,7 @@ sub CheckFormField (\%$;\@) {
|
|||
$field = $fieldname;
|
||||
}
|
||||
|
||||
ThrowCodeError("illegal_field", { field => $field }, "abort");
|
||||
ThrowCodeError("illegal_field", { field => $field });
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -213,7 +213,7 @@ sub CheckEmailSyntax {
|
|||
my ($addr) = (@_);
|
||||
my $match = Param('emailregexp');
|
||||
if ($addr !~ /$match/ || $addr =~ /[\\\(\)<>&,;:"\[\] \t\r\n]/) {
|
||||
ThrowUserError("illegal_email_address", { addr => $addr }, 'abort');
|
||||
ThrowUserError("illegal_email_address", { addr => $addr });
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -300,12 +300,10 @@ if ($action eq 'update') {
|
|||
|
||||
if ($classification ne $classificationold) {
|
||||
unless ($classification) {
|
||||
$dbh->bz_unlock_tables(UNLOCK_ABORT);
|
||||
ThrowUserError("classification_not_specified")
|
||||
}
|
||||
|
||||
if (TestClassification($classification)) {
|
||||
$dbh->bz_unlock_tables(UNLOCK_ABORT);
|
||||
ThrowUserError("classification_already_exists", { name => $classification });
|
||||
}
|
||||
$sth = $dbh->prepare("UPDATE classifications
|
||||
|
|
|
|||
|
|
@ -66,7 +66,7 @@ sub CheckProduct ($)
|
|||
|
||||
# do we have a product?
|
||||
unless ($prod) {
|
||||
ThrowUserError('product_not_specified');
|
||||
ThrowUserError('product_not_specified');
|
||||
exit;
|
||||
}
|
||||
|
||||
|
|
@ -585,7 +585,6 @@ if ($action eq 'update') {
|
|||
|
||||
if ($description ne $descriptionold) {
|
||||
unless ($description) {
|
||||
$dbh->bz_unlock_tables(UNLOCK_ABORT);
|
||||
ThrowUserError('component_blank_description',
|
||||
{'name' => $componentold});
|
||||
exit;
|
||||
|
|
@ -603,7 +602,6 @@ if ($action eq 'update') {
|
|||
|
||||
my $initialownerid = login_to_id($initialowner);
|
||||
unless ($initialownerid) {
|
||||
$dbh->bz_unlock_tables(UNLOCK_ABORT);
|
||||
ThrowUserError('component_need_valid_initialowner',
|
||||
{'name' => $componentold});
|
||||
exit;
|
||||
|
|
@ -621,7 +619,6 @@ if ($action eq 'update') {
|
|||
if (Param('useqacontact') && $initialqacontact ne $initialqacontactold) {
|
||||
my $initialqacontactid = login_to_id($initialqacontact);
|
||||
if (!$initialqacontactid && $initialqacontact ne '') {
|
||||
$dbh->bz_unlock_tables(UNLOCK_ABORT);
|
||||
ThrowUserError('component_need_valid_initialqacontact',
|
||||
{'name' => $componentold});
|
||||
exit;
|
||||
|
|
@ -638,13 +635,11 @@ if ($action eq 'update') {
|
|||
|
||||
if ($component ne $componentold) {
|
||||
unless ($component) {
|
||||
$dbh->bz_unlock_tables(UNLOCK_ABORT);
|
||||
ThrowUserError('component_must_have_a_name',
|
||||
{'name' => $componentold});
|
||||
exit;
|
||||
}
|
||||
if (TestComponent($product, $component)) {
|
||||
$dbh->bz_unlock_tables(UNLOCK_ABORT);
|
||||
ThrowUserError('component_already_exists',
|
||||
{'name' => $component});
|
||||
exit;
|
||||
|
|
|
|||
|
|
@ -59,14 +59,14 @@ sub CheckProduct ($)
|
|||
|
||||
# do we have a product?
|
||||
unless ($product) {
|
||||
&::ThrowUserError('product_not_specified');
|
||||
ThrowUserError('product_not_specified');
|
||||
exit;
|
||||
}
|
||||
|
||||
# Does it exist in the DB?
|
||||
unless (TestProduct $product) {
|
||||
&::ThrowUserError('product_doesnt_exist',
|
||||
{'product' => $product});
|
||||
ThrowUserError('product_doesnt_exist',
|
||||
{'product' => $product});
|
||||
exit;
|
||||
}
|
||||
}
|
||||
|
|
@ -506,12 +506,9 @@ if ($action eq 'update') {
|
|||
my $stored_sortkey = $sortkey;
|
||||
if ($sortkey != $sortkeyold) {
|
||||
if (!detaint_natural($sortkey)) {
|
||||
|
||||
$dbh->bz_unlock_tables(UNLOCK_ABORT);
|
||||
ThrowUserError('milestone_sortkey_invalid',
|
||||
{'name' => $milestone,
|
||||
'sortkey' => $stored_sortkey});
|
||||
|
||||
exit;
|
||||
}
|
||||
|
||||
|
|
@ -532,12 +529,10 @@ if ($action eq 'update') {
|
|||
|
||||
if ($milestone ne $milestoneold) {
|
||||
unless ($milestone) {
|
||||
$dbh->bz_unlock_tables(UNLOCK_ABORT);
|
||||
ThrowUserError('milestone_blank_name');
|
||||
exit;
|
||||
}
|
||||
if (TestMilestone($product, $milestone)) {
|
||||
$dbh->bz_unlock_tables(UNLOCK_ABORT);
|
||||
ThrowUserError('milestone_already_exists',
|
||||
{'name' => $milestone,
|
||||
'product' => $product});
|
||||
|
|
|
|||
|
|
@ -212,8 +212,7 @@ if ($action eq 'search') {
|
|||
canSeeUser($otherUserID)
|
||||
|| ThrowUserError('auth_failure', {reason => "not_visible",
|
||||
action => "modify",
|
||||
object => "user"},
|
||||
'abort');
|
||||
object => "user"});
|
||||
|
||||
# Cleanups
|
||||
my $login = trim($cgi->param('login') || '');
|
||||
|
|
@ -231,11 +230,10 @@ if ($action eq 'search') {
|
|||
|
||||
if ($login ne $loginold) {
|
||||
# Validate, then trick_taint.
|
||||
$login || ThrowUserError('user_login_required', undef, 'abort');
|
||||
$login || ThrowUserError('user_login_required');
|
||||
CheckEmailSyntax($login);
|
||||
is_available_username($login) || ThrowUserError('account_exists',
|
||||
{'email' => $login},
|
||||
'abort');
|
||||
{'email' => $login});
|
||||
trick_taint($login);
|
||||
push(@changedFields, 'login_name');
|
||||
push(@values, $login);
|
||||
|
|
@ -493,19 +491,17 @@ if ($action eq 'search') {
|
|||
'whine_events WRITE');
|
||||
|
||||
Param('allowuserdeletion')
|
||||
|| ThrowUserError('users_deletion_disabled', undef, 'abort');
|
||||
|| ThrowUserError('users_deletion_disabled');
|
||||
$editusers || ThrowUserError('auth_failure',
|
||||
{group => "editusers",
|
||||
action => "delete",
|
||||
object => "users"},
|
||||
'abort');
|
||||
object => "users"});
|
||||
canSeeUser($otherUserID) || ThrowUserError('auth_failure',
|
||||
{reason => "not_visible",
|
||||
action => "delete",
|
||||
object => "user"},
|
||||
'abort');
|
||||
object => "user"});
|
||||
productResponsibilities($otherUserID)
|
||||
&& ThrowUserError('user_has_responsibility', undef, 'abort');
|
||||
&& ThrowUserError('user_has_responsibility');
|
||||
|
||||
Bugzilla->logout_user_by_id($otherUserID);
|
||||
|
||||
|
|
|
|||
|
|
@ -406,16 +406,13 @@ if ($action eq 'update') {
|
|||
|
||||
if ($version ne $versionold) {
|
||||
unless ($version) {
|
||||
$dbh->bz_unlock_tables(UNLOCK_ABORT);
|
||||
ThrowUserError('version_blank_name');
|
||||
exit;
|
||||
}
|
||||
if (TestVersion($product,$version)) {
|
||||
$dbh->bz_unlock_tables(UNLOCK_ABORT);
|
||||
ThrowUserError('version_already_exists',
|
||||
{'name' => $version,
|
||||
'product' => $product});
|
||||
|
||||
exit;
|
||||
}
|
||||
SendSQL("UPDATE bugs
|
||||
|
|
|
|||
|
|
@ -613,11 +613,11 @@ sub ValidatePassword {
|
|||
my ($password, $matchpassword) = @_;
|
||||
|
||||
if (length($password) < 3) {
|
||||
ThrowUserError("password_too_short", undef, 'abort');
|
||||
ThrowUserError("password_too_short");
|
||||
} elsif (length($password) > 16) {
|
||||
ThrowUserError("password_too_long", undef, 'abort');
|
||||
ThrowUserError("password_too_long");
|
||||
} elsif ((defined $matchpassword) && ($password ne $matchpassword)) {
|
||||
ThrowUserError("passwords_dont_match", undef, 'abort');
|
||||
ThrowUserError("passwords_dont_match");
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -649,8 +649,7 @@ sub DBNameToIdAndCheck {
|
|||
return $result;
|
||||
}
|
||||
|
||||
ThrowUserError("invalid_username",
|
||||
{ name => $name }, "abort");
|
||||
ThrowUserError("invalid_username", { name => $name });
|
||||
}
|
||||
|
||||
sub get_classification_id {
|
||||
|
|
|
|||
|
|
@ -318,8 +318,7 @@ if (UserInGroup("editbugs") && defined($::FORM{'dependson'})) {
|
|||
}
|
||||
|
||||
ThrowUserError("dependency_loop_multi",
|
||||
{ both => $both },
|
||||
"abort");
|
||||
{ both => $both });
|
||||
}
|
||||
}
|
||||
my $tmp = $me;
|
||||
|
|
@ -378,14 +377,14 @@ foreach my $b (grep(/^bit-\d*$/, keys %::FORM)) {
|
|||
if ($::FORM{$b}) {
|
||||
my $v = substr($b, 4);
|
||||
$v =~ /^(\d+)$/
|
||||
|| ThrowCodeError("group_id_invalid", undef, "abort");
|
||||
|| ThrowCodeError("group_id_invalid");
|
||||
if (!GroupIsActive($v)) {
|
||||
# Prevent the user from adding the bug to an inactive group.
|
||||
# Should only happen if there is a bug in Bugzilla or the user
|
||||
# hacked the "enter bug" form since otherwise the UI
|
||||
# for adding the bug to the group won't appear on that form.
|
||||
$vars->{'bit'} = $v;
|
||||
ThrowCodeError("inactive_group", undef, "abort");
|
||||
ThrowCodeError("inactive_group");
|
||||
}
|
||||
SendSQL("SELECT user_id FROM user_group_map
|
||||
WHERE user_id = $::userid
|
||||
|
|
|
|||
|
|
@ -109,7 +109,7 @@ foreach my $field ("estimated_time", "work_time", "remaining_time") {
|
|||
if (UserInGroup(Param('timetrackinggroup'))) {
|
||||
my $wk_time = $::FORM{'work_time'};
|
||||
if ($::FORM{'comment'} =~ /^\s*$/ && $wk_time && $wk_time != 0) {
|
||||
ThrowUserError('comment_required', undef, "abort");
|
||||
ThrowUserError('comment_required');
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -241,7 +241,7 @@ if ((($::FORM{'id'} && $::FORM{'product'} ne $::oldproduct)
|
|||
$vars->{'newvalue'} = $::FORM{'product'};
|
||||
$vars->{'field'} = 'product';
|
||||
$vars->{'privs'} = $PrivilegesRequired;
|
||||
ThrowUserError("illegal_change", $vars, "abort");
|
||||
ThrowUserError("illegal_change", $vars);
|
||||
}
|
||||
|
||||
CheckFormField(\%::FORM, 'product', \@::legal_product);
|
||||
|
|
@ -1232,7 +1232,7 @@ foreach my $id (@idlist) {
|
|||
$vars->{'field'} = $col;
|
||||
}
|
||||
$vars->{'privs'} = $PrivilegesRequired;
|
||||
ThrowUserError("illegal_change", $vars, "abort");
|
||||
ThrowUserError("illegal_change", $vars);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1251,13 +1251,13 @@ foreach my $id (@idlist) {
|
|||
$vars->{'newvalue'} = "no keywords";
|
||||
$vars->{'field'} = "keywords";
|
||||
$vars->{'privs'} = $PrivilegesRequired;
|
||||
ThrowUserError("illegal_change", $vars, "abort");
|
||||
ThrowUserError("illegal_change", $vars);
|
||||
}
|
||||
|
||||
$oldhash{'product'} = get_product_name($oldhash{'product_id'});
|
||||
if (!CanEditProductId($oldhash{'product_id'})) {
|
||||
ThrowUserError("product_edit_denied",
|
||||
{ product => $oldhash{'product'} }, "abort");
|
||||
{ product => $oldhash{'product'} });
|
||||
}
|
||||
|
||||
if (defined $::FORM{'product'}
|
||||
|
|
@ -1265,7 +1265,7 @@ foreach my $id (@idlist) {
|
|||
&& $::FORM{'product'} ne $oldhash{'product'}
|
||||
&& !CanEnterProduct($::FORM{'product'})) {
|
||||
ThrowUserError("entry_access_denied",
|
||||
{ product => $::FORM{'product'} }, "abort");
|
||||
{ product => $::FORM{'product'} });
|
||||
}
|
||||
if ($requiremilestone) {
|
||||
# musthavemilestoneonaccept applies only if at least two
|
||||
|
|
@ -1283,7 +1283,7 @@ foreach my $id (@idlist) {
|
|||
# if musthavemilestoneonaccept == 1, then the target
|
||||
# milestone must be different from the default one.
|
||||
if ($value eq $defaultmilestone) {
|
||||
ThrowUserError("milestone_required", { bug_id => $id }, "abort");
|
||||
ThrowUserError("milestone_required", { bug_id => $id });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1319,7 +1319,7 @@ foreach my $id (@idlist) {
|
|||
next if $i eq "";
|
||||
|
||||
if ($id eq $i) {
|
||||
ThrowUserError("dependency_loop_single", undef, "abort");
|
||||
ThrowUserError("dependency_loop_single");
|
||||
}
|
||||
if (!exists $seen{$i}) {
|
||||
push(@{$deptree{$target}}, $i);
|
||||
|
|
@ -1363,8 +1363,7 @@ foreach my $id (@idlist) {
|
|||
}
|
||||
|
||||
ThrowUserError("dependency_loop_multi",
|
||||
{ both => $both },
|
||||
"abort");
|
||||
{ both => $both });
|
||||
}
|
||||
}
|
||||
my $tmp = $me;
|
||||
|
|
@ -1573,6 +1572,7 @@ foreach my $id (@idlist) {
|
|||
shift @oldlist;
|
||||
} else {
|
||||
if ($oldlist[0] != $newlist[0]) {
|
||||
$dbh->bz_unlock_tables(UNLOCK_ABORT);
|
||||
die "Error in list comparing code";
|
||||
}
|
||||
shift @oldlist;
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче