зеркало из https://github.com/mozilla/gecko-dev.git
Bug 315605: Bugzilla::Field::check_form_field() should not take a CGI object as parameter - Patch by Fr�d�ric Buclin <LpSolit@gmail.com> r=wicked a=justdave
This commit is contained in:
lpsolit%gmail.com
Родитель
1aa866e66d
Коммит
52a7c378ca
|
|
@ -13,7 +13,7 @@
|
|||
# The Original Code is the Bugzilla Bug Tracking System.
|
||||
#
|
||||
# Contributor(s): Dan Mosedale <dmose@mozilla.org>
|
||||
# Frédéric Buclin <LpSolit@gmail.com>
|
||||
# Frédéric Buclin <LpSolit@gmail.com>
|
||||
# Myk Melez <myk@mozilla.org>
|
||||
|
||||
=head1 NAME
|
||||
|
|
@ -28,7 +28,7 @@ Bugzilla::Field - a particular piece of information about bugs
|
|||
|
||||
# Display information about all fields.
|
||||
print Dumper(Bugzilla->get_fields());
|
||||
|
||||
|
||||
# Display information about non-obsolete custom fields.
|
||||
print Dumper(Bugzilla->get_fields({ obsolete => 1, custom => 1 }));
|
||||
|
||||
|
|
@ -41,11 +41,11 @@ Bugzilla::Field - a particular piece of information about bugs
|
|||
# Bugzilla->get_fields() is a wrapper around Bugzilla::Field::match(),
|
||||
# so both methods take the same arguments.
|
||||
print Dumper(Bugzilla::Field::match({ obsolete => 1, custom => 1 }));
|
||||
|
||||
|
||||
# Create a custom field.
|
||||
my $field = Bugzilla::Field::create("hilarity", "Hilarity");
|
||||
print "$field->{description} is a custom field\n";
|
||||
|
||||
|
||||
# Instantiate a Field object for an existing field.
|
||||
my $field = new Bugzilla::Field('qacontact_accessible');
|
||||
if ($field->{obsolete}) {
|
||||
|
|
@ -53,8 +53,7 @@ Bugzilla::Field - a particular piece of information about bugs
|
|||
}
|
||||
|
||||
# Validation Routines
|
||||
check_form_field($cgi, $fieldname, \@legal_values);
|
||||
check_form_field_defined($cgi, $fieldname);
|
||||
check_field($name, $value, \@legal_values, $no_warn);
|
||||
$fieldid = get_field_id($fieldname);
|
||||
|
||||
=head1 DESCRIPTION
|
||||
|
|
@ -71,8 +70,7 @@ package Bugzilla::Field;
|
|||
use strict;
|
||||
|
||||
use base qw(Exporter);
|
||||
@Bugzilla::Field::EXPORT = qw(check_form_field check_form_field_defined
|
||||
get_field_id);
|
||||
@Bugzilla::Field::EXPORT = qw(check_field get_field_id);
|
||||
|
||||
use Bugzilla::Util;
|
||||
use Bugzilla::Constants;
|
||||
|
|
@ -286,66 +284,45 @@ sub match {
|
|||
|
||||
=over
|
||||
|
||||
=item C<check_form_field($cgi, $fieldname, \@legal_values)>
|
||||
=item C<check_field($name, $value, \@legal_values, $no_warn)>
|
||||
|
||||
Description: Makes sure the field $fieldname is defined and its value
|
||||
Description: Makes sure the field $name is defined and its $value
|
||||
is non empty. If @legal_values is defined, this routine
|
||||
also checks whether its value is one of the legal values
|
||||
associated with this field. If the test fails, an error
|
||||
is thrown.
|
||||
associated with this field. If the test is successful,
|
||||
the function returns 1. If the test fails, an error
|
||||
is thrown (by default), unless $no_warn is true, in which
|
||||
case the function returns 0.
|
||||
|
||||
Params: $cgi - a CGI object
|
||||
$fieldname - the field name to check
|
||||
Params: $name - the field name
|
||||
$value - the field value
|
||||
@legal_values - (optional) ref to a list of legal values
|
||||
$no_warn - (optional) do not throw an error if true
|
||||
|
||||
Returns: nothing
|
||||
Returns: 1 on success; 0 on failure if $no_warn is true (else an
|
||||
error is thrown).
|
||||
|
||||
=back
|
||||
|
||||
=cut
|
||||
|
||||
sub check_form_field {
|
||||
my ($cgi, $fieldname, $legalsRef) = @_;
|
||||
sub check_field {
|
||||
my ($name, $value, $legalsRef, $no_warn) = @_;
|
||||
my $dbh = Bugzilla->dbh;
|
||||
|
||||
if (!defined $cgi->param($fieldname)
|
||||
|| trim($cgi->param($fieldname)) eq ""
|
||||
|| (defined($legalsRef)
|
||||
&& lsearch($legalsRef, $cgi->param($fieldname)) < 0))
|
||||
if (!defined($value)
|
||||
|| trim($value) eq ""
|
||||
|| (defined($legalsRef) && lsearch($legalsRef, $value) < 0))
|
||||
{
|
||||
trick_taint($fieldname);
|
||||
my ($result) = $dbh->selectrow_array("SELECT description FROM fielddefs
|
||||
WHERE name = ?", undef, $fieldname);
|
||||
|
||||
my $field = $result || $fieldname;
|
||||
ThrowCodeError("illegal_field", { field => $field });
|
||||
}
|
||||
}
|
||||
|
||||
=pod
|
||||
|
||||
=over
|
||||
|
||||
=item C<check_form_field_defined($cgi, $fieldname)>
|
||||
|
||||
Description: Makes sure the field $fieldname is defined and its value
|
||||
is non empty. Else an error is thrown.
|
||||
|
||||
Params: $cgi - a CGI object
|
||||
$fieldname - the field name to check
|
||||
|
||||
Returns: nothing
|
||||
|
||||
=back
|
||||
|
||||
=cut
|
||||
|
||||
sub check_form_field_defined {
|
||||
my ($cgi, $fieldname) = @_;
|
||||
|
||||
if (!defined $cgi->param($fieldname)) {
|
||||
ThrowCodeError("undefined_field", { field => $fieldname });
|
||||
return 0 if $no_warn; # We don't want an error to be thrown; return.
|
||||
trick_taint($name);
|
||||
my ($result) = $dbh->selectrow_array('SELECT description FROM fielddefs
|
||||
WHERE name = ?', undef, $name);
|
||||
|
||||
my $field = $result || $name;
|
||||
ThrowCodeError('illegal_field', { field => $field });
|
||||
}
|
||||
return 1;
|
||||
}
|
||||
|
||||
=pod
|
||||
|
|
|
|||
|
|
@ -197,18 +197,21 @@ if (!Param('letsubmitterchoosepriority')) {
|
|||
GetVersionTable();
|
||||
|
||||
# Some more sanity checking
|
||||
check_form_field($cgi, 'product', \@::legal_product);
|
||||
check_form_field($cgi, 'rep_platform', \@::legal_platform);
|
||||
check_form_field($cgi, 'bug_severity', \@::legal_severity);
|
||||
check_form_field($cgi, 'priority', \@::legal_priority);
|
||||
check_form_field($cgi, 'op_sys', \@::legal_opsys);
|
||||
check_form_field($cgi, 'bug_status', ['UNCONFIRMED', 'NEW']);
|
||||
check_form_field($cgi, 'version', $::versions{$product});
|
||||
check_form_field($cgi, 'component', $::components{$product});
|
||||
check_form_field($cgi, 'target_milestone', $::target_milestone{$product});
|
||||
check_form_field_defined($cgi, 'assigned_to');
|
||||
check_form_field_defined($cgi, 'bug_file_loc');
|
||||
check_form_field_defined($cgi, 'comment');
|
||||
check_field('product', scalar $cgi->param('product'), \@::legal_product);
|
||||
check_field('rep_platform', scalar $cgi->param('rep_platform'), \@::legal_platform);
|
||||
check_field('bug_severity', scalar $cgi->param('bug_severity'), \@::legal_severity);
|
||||
check_field('priority', scalar $cgi->param('priority'), \@::legal_priority);
|
||||
check_field('op_sys', scalar $cgi->param('op_sys'), \@::legal_opsys);
|
||||
check_field('bug_status', scalar $cgi->param('bug_status'), ['UNCONFIRMED', 'NEW']);
|
||||
check_field('version', scalar $cgi->param('version'), $::versions{$product});
|
||||
check_field('component', scalar $cgi->param('component'), $::components{$product});
|
||||
check_field('target_milestone', scalar $cgi->param('target_milestone'),
|
||||
$::target_milestone{$product});
|
||||
|
||||
foreach my $field_name ('assigned_to', 'bug_file_loc', 'comment') {
|
||||
defined($cgi->param($field_name))
|
||||
|| ThrowCodeError('undefined_field', { field => $field_name });
|
||||
}
|
||||
|
||||
my $everconfirmed = ($cgi->param('bug_status') eq 'UNCONFIRMED') ? 0 : 1;
|
||||
$cgi->param(-name => 'everconfirmed', -value => $everconfirmed);
|
||||
|
|
|
|||
|
|
@ -234,10 +234,10 @@ if ($cgi->cookie("BUGLIST") && defined $cgi->param('id')) {
|
|||
|
||||
GetVersionTable();
|
||||
|
||||
check_form_field_defined($cgi, 'product');
|
||||
check_form_field_defined($cgi, 'version');
|
||||
check_form_field_defined($cgi, 'component');
|
||||
|
||||
foreach my $field_name ('product', 'component', 'version') {
|
||||
defined($cgi->param($field_name))
|
||||
|| ThrowCodeError('undefined_field', { field => $field_name });
|
||||
}
|
||||
|
||||
# This function checks if there is a comment required for a specific
|
||||
# function and tests, if the comment was given.
|
||||
|
|
@ -325,7 +325,9 @@ if (((defined $cgi->param('id') && $cgi->param('product') ne $oldproduct)
|
|||
|
||||
my $mok = 1; # so it won't affect the 'if' statement if milestones aren't used
|
||||
if ( Param("usetargetmilestone") ) {
|
||||
check_form_field_defined($cgi, 'target_milestone');
|
||||
defined($cgi->param('target_milestone'))
|
||||
|| ThrowCodeError('undefined_field', { field => 'target_milestone' });
|
||||
|
||||
$mok = lsearch($::target_milestone{$prod},
|
||||
$cgi->param('target_milestone')) >= 0;
|
||||
}
|
||||
|
|
@ -595,22 +597,25 @@ if (defined $cgi->param('id')) {
|
|||
# values that have been changed instead of submitting all the new values.
|
||||
# (XXX those error checks need to happen too, but implementing them
|
||||
# is more work in the current architecture of this script...)
|
||||
#
|
||||
check_form_field($cgi, 'product', \@::legal_product);
|
||||
check_form_field($cgi, 'component',
|
||||
\@{$::components{$cgi->param('product')}});
|
||||
check_form_field($cgi, 'version', \@{$::versions{$cgi->param('product')}});
|
||||
check_field('product', scalar $cgi->param('product'), \@::legal_product);
|
||||
check_field('component', scalar $cgi->param('component'),
|
||||
\@{$::components{$cgi->param('product')}});
|
||||
check_field('version', scalar $cgi->param('version'),
|
||||
\@{$::versions{$cgi->param('product')}});
|
||||
if ( Param("usetargetmilestone") ) {
|
||||
check_form_field($cgi, 'target_milestone',
|
||||
\@{$::target_milestone{$cgi->param('product')}});
|
||||
check_field('target_milestone', scalar $cgi->param('target_milestone'),
|
||||
\@{$::target_milestone{$cgi->param('product')}});
|
||||
}
|
||||
check_field('rep_platform', scalar $cgi->param('rep_platform'), \@::legal_platform);
|
||||
check_field('op_sys', scalar $cgi->param('op_sys'), \@::legal_opsys);
|
||||
check_field('priority', scalar $cgi->param('priority'), \@::legal_priority);
|
||||
check_field('bug_severity', scalar $cgi->param('bug_severity'), \@::legal_severity);
|
||||
|
||||
# Those fields only have to exist. We don't validate their value here.
|
||||
foreach my $field_name ('bug_file_loc', 'short_desc', 'longdesclength') {
|
||||
defined($cgi->param($field_name))
|
||||
|| ThrowCodeError('undefined_field', { field => $field_name });
|
||||
}
|
||||
check_form_field($cgi, 'rep_platform', \@::legal_platform);
|
||||
check_form_field($cgi, 'op_sys', \@::legal_opsys);
|
||||
check_form_field($cgi, 'priority', \@::legal_priority);
|
||||
check_form_field($cgi, 'bug_severity', \@::legal_severity);
|
||||
check_form_field_defined($cgi, 'bug_file_loc');
|
||||
check_form_field_defined($cgi, 'short_desc');
|
||||
check_form_field_defined($cgi, 'longdesclength');
|
||||
$cgi->param('short_desc', clean_text($cgi->param('short_desc')));
|
||||
|
||||
if (trim($cgi->param('short_desc')) eq "") {
|
||||
|
|
@ -1105,7 +1110,8 @@ SWITCH: for ($cgi->param('knob')) {
|
|||
};
|
||||
/^resolve$/ && CheckonComment( "resolve" ) && do {
|
||||
# Check here, because its the only place we require the resolution
|
||||
check_form_field($cgi, 'resolution', \@::settable_resolution);
|
||||
check_field('resolution', scalar $cgi->param('resolution'),
|
||||
\@::settable_resolution);
|
||||
|
||||
# don't resolve as fixed while still unresolved blocking bugs
|
||||
if (Param("noresolveonopenblockers")
|
||||
|
|
@ -1189,7 +1195,9 @@ SWITCH: for ($cgi->param('knob')) {
|
|||
}
|
||||
|
||||
# Make sure we can change the original bug (issue A on bug 96085)
|
||||
check_form_field_defined($cgi, 'dup_id');
|
||||
defined($cgi->param('dup_id'))
|
||||
|| ThrowCodeError('undefined_field', { field => 'dup_id' });
|
||||
|
||||
$duplicate = $cgi->param('dup_id');
|
||||
ValidateBugID($duplicate, 'dup_id');
|
||||
$cgi->param('dup_id', $duplicate);
|
||||
|
|
@ -2063,7 +2071,6 @@ foreach my $id (@idlist) {
|
|||
" has been marked as a duplicate of this bug. ***",
|
||||
0, $timestamp);
|
||||
|
||||
check_form_field_defined($cgi,'comment');
|
||||
SendSQL("INSERT INTO duplicates VALUES ($duplicate, " .
|
||||
$cgi->param('id') . ")");
|
||||
}
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче