aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rwxr-xr-xBugzilla/Bug.pm166
-rw-r--r--Bugzilla/Object.pm47
-rwxr-xr-xpost_bug.cgi176
3 files changed, 238 insertions, 151 deletions
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 8c61a657b..642a71d3f 100755
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -96,6 +96,34 @@ sub DB_COLUMNS {
Bugzilla->custom_field_names;
}
+use constant REQUIRED_CREATE_FIELDS => qw(
+ bug_severity
+ component
+ creation_ts
+ op_sys
+ priority
+ product
+ rep_platform
+ short_desc
+ version
+);
+
+# There are also other, more complex validators that are called
+# from run_create_validators.
+use constant VALIDATORS => {
+ alias => \&_check_alias,
+ bug_file_loc => \&_check_bug_file_loc,
+ bug_severity => \&_check_bug_severity,
+ deadline => \&_check_deadline,
+ estimated_time => \&_check_estimated_time,
+ op_sys => \&_check_op_sys,
+ priority => \&_check_priority,
+ remaining_time => \&_check_remaining_time,
+ rep_platform => \&_check_rep_platform,
+ short_desc => \&_check_short_desc,
+ status_whiteboard => \&_check_status_whiteboard,
+};
+
# Used in LogActivityEntry(). Gives the max length of lines in the
# activity table.
use constant MAX_LINE_LENGTH => 254;
@@ -157,6 +185,79 @@ sub new {
return $self;
}
+# Docs for create() (there's no POD in this file yet, but we very
+# much need this documented right now):
+#
+# The same as Bugzilla::Object->create. Parameters are only required
+# if they say so below.
+#
+# Params:
+#
+# C<product> - B<Required> The name of the product this bug is being
+# filed against.
+# C<component> - B<Required> The name of the component this bug is being
+# filed against.
+#
+# C<bug_severity> - B<Required> The severity for the bug, a string.
+# C<creation_ts> - B<Required> A SQL timestamp for when the bug was created.
+# C<short_desc> - B<Required> A summary for the bug.
+# C<op_sys> - B<Required> The OS the bug was found against.
+# C<priority> - B<Required> The initial priority for the bug.
+# C<rep_platform> - B<Required> The platform the bug was found against.
+# C<version> - B<Required> The version of the product the bug was found in.
+#
+# C<alias> - An alias for this bug. Will be ignored if C<usebugaliases>
+# is off.
+# C<target_milestone> - When this bug is expected to be fixed.
+# C<status_whiteboard> - A string.
+# C<bug_status> - The initial status of the bug, a string.
+# C<bug_file_loc> - The URL field.
+#
+# C<assigned_to> - The full login name of the user who the bug is
+# initially assigned to.
+# C<qa_contact> - The full login name of the QA Contact for this bug.
+# Will be ignored if C<useqacontact> is off.
+#
+# C<estimated_time> - For time-tracking. Will be ignored if
+# C<timetrackinggroup> is not set, or if the current
+# user is not a member of the timetrackinggroup.
+# C<deadline> - For time-tracking. Will be ignored for the same
+# reasons as C<estimated_time>.
+
+sub run_create_validators {
+ my $class = shift;
+ my $params = shift;
+
+ my $product = _check_product($params->{product});
+ $params->{product_id} = $product->id;
+ delete $params->{product};
+
+ ($params->{bug_status}, $params->{everconfirmed})
+ = _check_bug_status($product, $params->{bug_status});
+
+ $params->{target_milestone} = _check_target_milestone($product,
+ $params->{target_milestone});
+
+ $params->{version} = _check_version($product, $params->{version});
+
+ my $component = _check_component($product, $params->{component});
+ $params->{component_id} = $component->id;
+ delete $params->{component};
+
+ $params->{assigned_to} =
+ _check_assigned_to($component, $params->{assigned_to});
+ $params->{qa_contact} =
+ _check_qa_contact($component, $params->{qa_contact});
+ # Callers cannot set Reporter, currently.
+ $params->{reporter} = Bugzilla->user->id;
+
+ $params->{delta_ts} = $params->{creation_ts};
+ $params->{remaining_time} = $params->{estimated_time};
+
+ unshift @_, $params;
+ return $class->SUPER::run_create_validators(@_);
+}
+
# This is the correct way to delete bugs from the DB.
# No bug should be deleted from anywhere else except from here.
#
@@ -233,12 +334,13 @@ sub remove_from_db {
sub _check_alias {
my ($alias) = @_;
$alias = trim($alias);
- ValidateBugAlias($alias) if (defined $alias && $alias ne '');
+ return undef if (!Bugzilla->params->{'usebugaliases'} || !$alias);
+ ValidateBugAlias($alias);
return $alias;
}
sub _check_assigned_to {
- my ($name, $component) = @_;
+ my ($component, $name) = @_;
my $user = Bugzilla->user;
$name = trim($name);
@@ -254,9 +356,6 @@ sub _check_assigned_to {
sub _check_bug_file_loc {
my ($url) = @_;
- if (!defined $url) {
- ThrowCodeError('undefined_field', { field => 'bug_file_loc' });
- }
# If bug_file_loc is "http://", the default, use an empty value instead.
$url = '' if $url eq 'http://';
return $url;
@@ -270,7 +369,7 @@ sub _check_bug_severity {
}
sub _check_bug_status {
- my ($status, $product) = @_;
+ my ($product, $status) = @_;
my $user = Bugzilla->user;
my @valid_statuses = VALID_ENTRY_STATUS;
@@ -293,7 +392,7 @@ sub _check_bug_status {
shift @valid_statuses if !$product->votes_to_confirm;
check_field('bug_status', $status, \@valid_statuses);
- return $status;
+ return ($status, $status eq 'UNCONFIRMED' ? 0 : 1);
}
sub _check_cc {
@@ -331,7 +430,7 @@ sub _check_comment {
}
sub _check_component {
- my ($name, $product) = @_;
+ my ($product, $name) = @_;
$name = trim($name);
$name || ThrowUserError("require_component");
my $obj = Bugzilla::Component::check_component($product, $name);
@@ -341,6 +440,18 @@ sub _check_component {
return $obj;
}
+sub _check_deadline {
+ my ($date) = @_;
+ $date = trim($date);
+ my $tt_group = Bugzilla->params->{"timetrackinggroup"};
+ return undef unless $date && $tt_group
+ && Bugzilla->user->in_group($tt_group);
+ validate_date($date)
+ || ThrowUserError('illegal_date', { date => $date,
+ format => 'YYYY-MM-DD' });
+ return $date;
+}
+
# Takes two comma/space-separated strings and returns arrayrefs
# of valid bug IDs.
sub _check_dependencies {
@@ -370,6 +481,10 @@ sub _check_dependencies {
return ($deps{'dependson'}, $deps{'blocked'});
}
+sub _check_estimated_time {
+ return _check_time($_[0], 'estimated_time');
+}
+
sub _check_keywords {
my ($keyword_string) = @_;
$keyword_string = trim($keyword_string);
@@ -417,6 +532,10 @@ sub _check_priority {
return $priority;
}
+sub _check_remaining_time {
+ return _check_time($_[0], 'remaining_time');
+}
+
sub _check_rep_platform {
my ($platform) = @_;
$platform = trim($platform);
@@ -435,6 +554,8 @@ sub _check_short_desc {
return $short_desc;
}
+sub _check_status_whiteboard { return defined $_[0] ? $_[0] : ''; }
+
# Unlike other checkers, this one doesn't return anything.
sub _check_strict_isolation {
my ($product, $cc_ids, $assignee_id, $qa_contact_id) = @_;
@@ -466,10 +587,30 @@ sub _check_strict_isolation {
}
}
+sub _check_target_milestone {
+ my ($product, $target) = @_;
+ $target = trim($target);
+ $target = $product->default_milestone if !defined $target;
+ check_field('target_milestone', $target,
+ [map($_->name, @{$product->milestones})]);
+ return $target;
+}
+
+sub _check_time {
+ my ($time, $field) = @_;
+ my $tt_group = Bugzilla->params->{"timetrackinggroup"};
+ return 0 unless $tt_group && Bugzilla->user->in_group($tt_group);
+ $time = trim($time) || 0;
+ ValidateTime($time, $field);
+ return $time;
+}
+
sub _check_qa_contact {
- my ($name, $component) = @_;
+ my ($component, $name) = @_;
my $user = Bugzilla->user;
+ return undef unless Bugzilla->params->{'useqacontact'};
+
$name = trim($name);
my $id;
@@ -483,6 +624,13 @@ sub _check_qa_contact {
return $id;
}
+sub _check_version {
+ my ($product, $version) = @_;
+ $version = trim($version);
+ check_field('version', $version, [map($_->name, @{$product->versions})]);
+ return $version;
+}
+
#####################################################################
# Class Accessors
diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index 219658a92..833cdcd2f 100644
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -123,16 +123,29 @@ sub create {
my ($class, $params) = @_;
my $dbh = Bugzilla->dbh;
- my $required = $class->REQUIRED_CREATE_FIELDS;
- my $validators = $class->VALIDATORS;
- my $table = $class->DB_TABLE;
-
foreach my $field ($class->REQUIRED_CREATE_FIELDS) {
ThrowCodeError('param_required',
{ function => "${class}->create", param => $field })
if !exists $params->{$field};
}
+ my ($field_names, $values) = $class->run_create_validators($params);
+
+ my $qmarks = '?,' x @$values;
+ chop($qmarks);
+ my $table = $class->DB_TABLE;
+ $dbh->do("INSERT INTO $table (" . join(', ', @$field_names)
+ . ") VALUES ($qmarks)", undef, @$values);
+ my $id = $dbh->bz_last_key($table, $class->ID_FIELD);
+
+ return $class->new($id);
+}
+
+sub run_create_validators {
+ my ($class, $params) = @_;
+
+ my $validators = $class->VALIDATORS;
+
my (@field_names, @values);
# We do the sort just to make sure that validation always
# happens in a consistent order.
@@ -144,18 +157,14 @@ sub create {
else {
$value = $params->{$field};
}
- trick_taint($value);
+ # We want people to be able to explicitly set fields to NULL,
+ # and that means they can be set to undef.
+ trick_taint($value) if defined $value;
push(@field_names, $field);
push(@values, $value);
}
- my $qmarks = '?,' x @values;
- chop($qmarks);
- $dbh->do("INSERT INTO $table (" . join(', ', @field_names)
- . ") VALUES ($qmarks)", undef, @values);
- my $id = $dbh->bz_last_key($table, $class->ID_FIELD);
-
- return $class->new($id);
+ return (\@field_names, \@values);
}
sub get_all {
@@ -307,6 +316,20 @@ Notes: In order for this function to work in your subclass,
type in the database. Your subclass also must
define L</REQUIRED_CREATE_FIELDS> and L</VALIDATORS>.
+=item C<run_create_validators($params)>
+
+Description: Runs the validation of input parameters for L</create>.
+ This subroutine exists so that it can be overridden
+ by subclasses who need to do special validations
+ of their input parameters. This method is B<only> called
+ by L</create>.
+
+Params: The same as L</create>.
+
+Returns: Two arrayrefs. The first is an array of database field names.
+ The second is an untainted array of values that should go
+ into those fields (in the same order).
+
=item C<get_all>
Description: Returns all objects in this table from the database.
diff --git a/post_bug.cgi b/post_bug.cgi
index 95621c3ed..2257d543f 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -139,12 +139,6 @@ if (defined $cgi->param('maketemplate')) {
umask 0;
-# Some sanity checking
-my $component =
- Bugzilla::Bug::_check_component(scalar $cgi->param('component'), $product);
-$cgi->param('short_desc',
- Bugzilla::Bug::_check_short_desc($cgi->param('short_desc')));
-
# This has to go somewhere after 'maketemplate'
# or it breaks bookmarks with no comments.
$comment = Bugzilla::Bug::_check_comment($cgi->param('comment'));
@@ -152,81 +146,19 @@ $comment = Bugzilla::Bug::_check_comment($cgi->param('comment'));
# OK except for the fact that it causes e-mail to be suppressed.
$comment = $comment ? $comment : " ";
-$cgi->param('bug_file_loc',
- Bugzilla::Bug::_check_bug_file_loc($cgi->param('bug_file_loc')));
-$cgi->param('assigned_to',
- Bugzilla::Bug::_check_assigned_to(scalar $cgi->param('assigned_to'),
- $component));
-
-
-my @enter_bug_field_names = map {$_->name} Bugzilla->get_fields({ custom => 1,
- obsolete => 0, enter_bug => 1});
-
-my @bug_fields = ("version", "rep_platform",
- "bug_severity", "priority", "op_sys", "assigned_to",
- "bug_status", "everconfirmed", "bug_file_loc", "short_desc",
- "target_milestone", "status_whiteboard",
- @enter_bug_field_names);
-
-if (Bugzilla->params->{"usebugaliases"}) {
- my $alias = Bugzilla::Bug::_check_alias($cgi->param('alias'));
- if ($alias) {
- $cgi->param('alias', $alias);
- push(@bug_fields, "alias");
- }
-}
-
-if (Bugzilla->params->{"useqacontact"}) {
- my $qa_contact = Bugzilla::Bug::_check_qa_contact(
- scalar $cgi->param('qa_contact'), $component);
- if ($qa_contact) {
- $cgi->param('qa_contact', $qa_contact);
- push(@bug_fields, "qa_contact");
- }
-}
-
-$cgi->param('bug_status', Bugzilla::Bug::_check_bug_status(
- scalar $cgi->param('bug_status'), $product));
-
-if (!defined $cgi->param('target_milestone')) {
- $cgi->param(-name => 'target_milestone', -value => $product->default_milestone);
-}
-
-# Some more sanity checking
-$cgi->param(-name => 'priority', -value => Bugzilla::Bug::_check_priority(
- $cgi->param('priority')));
-$cgi->param(-name => 'rep_platform',
- -value => Bugzilla::Bug::_check_rep_platform($cgi->param('rep_platform')));
-$cgi->param(-name => 'bug_severity',
- -value => Bugzilla::Bug::_check_bug_severity($cgi->param('bug_severity')));
-$cgi->param(-name => 'op_sys', -value => Bugzilla::Bug::_check_op_sys(
- $cgi->param('op_sys')));
-
-check_field('version', scalar $cgi->param('version'),
- [map($_->name, @{$product->versions})]);
-check_field('target_milestone', scalar $cgi->param('target_milestone'),
- [map($_->name, @{$product->milestones})]);
-
-my $everconfirmed = ($cgi->param('bug_status') eq 'UNCONFIRMED') ? 0 : 1;
-$cgi->param(-name => 'everconfirmed', -value => $everconfirmed);
-
-my @used_fields;
-foreach my $field (@bug_fields) {
- if (defined $cgi->param($field)) {
- push (@used_fields, $field);
- }
-}
-
-$cgi->param(-name => 'product_id', -value => $product->id);
-push(@used_fields, "product_id");
-$cgi->param(-name => 'component_id', -value => $component->id);
-push(@used_fields, "component_id");
-
my $cc_ids = Bugzilla::Bug::_check_cc([$cgi->param('cc')]);
my @keyword_ids = @{Bugzilla::Bug::_check_keywords($cgi->param('keywords'))};
-Bugzilla::Bug::_check_strict_isolation($product, $cc_ids,
- $cgi->param('assigned_to'), $cgi->param('qa_contact'));
+# XXX These checks are only here until strict_isolation can move fully
+# into Bugzilla::Bug.
+my $component = Bugzilla::Bug::_check_component($product,
+ $cgi->param('component'));
+my $assigned_to_id = Bugzilla::Bug::_check_assigned_to($component,
+ $cgi->param('assigned_to'));
+my $qa_contact_id = Bugzilla::Bug::_check_qa_contact($component,
+ $cgi->param('qa_contact'));
+Bugzilla::Bug::_check_strict_isolation($product, $cc_ids, $assigned_to_id,
+ $qa_contact_id);
my ($depends_on_ids, $blocks_ids) = Bugzilla::Bug::_check_dependencies(
scalar $cgi->param('dependson'), scalar $cgi->param('blocked'));
@@ -234,54 +166,6 @@ my ($depends_on_ids, $blocks_ids) = Bugzilla::Bug::_check_dependencies(
# get current time
my $timestamp = $dbh->selectrow_array(q{SELECT NOW()});
-# Build up SQL string to add bug.
-# creation_ts will only be set when all other fields are defined.
-
-my @fields_values;
-
-foreach my $field (@used_fields) {
- my $value = $cgi->param($field);
- trick_taint($value);
- push (@fields_values, $value);
-}
-
-my $sql_used_fields = join(", ", @used_fields);
-my $sql_placeholders = "?, " x scalar(@used_fields);
-
-my $query = qq{INSERT INTO bugs ($sql_used_fields, reporter, delta_ts,
- estimated_time, remaining_time, deadline)
- VALUES ($sql_placeholders ?, ?, ?, ?, ?)};
-
-push (@fields_values, $user->id);
-push (@fields_values, $timestamp);
-
-my $est_time = 0;
-my $deadline;
-
-# Time Tracking
-if (UserInGroup(Bugzilla->params->{"timetrackinggroup"}) &&
- defined $cgi->param('estimated_time')) {
-
- $est_time = $cgi->param('estimated_time');
- Bugzilla::Bug::ValidateTime($est_time, 'estimated_time');
- trick_taint($est_time);
-
-}
-
-push (@fields_values, $est_time, $est_time);
-
-if ( UserInGroup(Bugzilla->params->{"timetrackinggroup"})
- && $cgi->param('deadline') )
-{
- validate_date($cgi->param('deadline'))
- || ThrowUserError('illegal_date', {date => $cgi->param('deadline'),
- format => 'YYYY-MM-DD'});
- $deadline = $cgi->param('deadline');
- trick_taint($deadline);
-}
-
-push (@fields_values, $deadline);
-
# Groups
my @groupstoadd = ();
my $sth_othercontrol = $dbh->prepare(q{SELECT othercontrol
@@ -339,17 +223,50 @@ foreach my $group (@$groups) {
}
}
+my @bug_fields = map {$_->name} Bugzilla->get_fields(
+ { custom => 1, obsolete => 0, enter_bug => 1});
+push(@bug_fields, qw(
+ product
+ component
+
+ assigned_to
+ qa_contact
+
+ alias
+ bug_file_loc
+ bug_severity
+ bug_status
+ short_desc
+ op_sys
+ priority
+ rep_platform
+ version
+ target_milestone
+ status_whiteboard
+
+ estimated_time
+ deadline
+));
+my %bug_params;
+foreach my $field (@bug_fields) {
+ $bug_params{$field} = $cgi->param($field);
+}
+$bug_params{'creation_ts'} = $timestamp;
+
# Add the bug report to the DB.
$dbh->bz_lock_tables('bugs WRITE', 'bug_group_map WRITE', 'longdescs WRITE',
'cc WRITE', 'keywords WRITE', 'dependencies WRITE',
'bugs_activity WRITE', 'groups READ',
'user_group_map READ', 'group_group_map READ',
- 'keyworddefs READ', 'fielddefs READ');
+ 'keyworddefs READ', 'fielddefs READ',
+ 'products READ', 'versions READ', 'milestones READ',
+ 'components READ', 'profiles READ', 'bug_severity READ',
+ 'op_sys READ', 'priority READ', 'rep_platform READ');
-$dbh->do($query, undef, @fields_values);
+my $bug = Bugzilla::Bug->create(\%bug_params);
# Get the bug ID back.
-my $id = $dbh->bz_last_key('bugs', 'bug_id');
+my $id = $bug->bug_id;
# Add the group restrictions
my $sth_addgroup = $dbh->prepare(q{
@@ -420,7 +337,6 @@ $dbh->do("UPDATE bugs SET creation_ts = ? WHERE bug_id = ?",
$dbh->bz_unlock_tables();
-my $bug = new Bugzilla::Bug($id);
# We don't have to check if the user can see the bug, because a user filing
# a bug can always see it. You can't change reporter_accessible until
# after the bug is filed.