From b63fd277afedfb5d101ce4700058609e81199855 Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Mon, 4 Sep 2006 23:19:07 +0000 Subject: Bug 349741: Make Bugzilla::Bug able to do basic bug creation, and have post_bug.cgi use it Patch By Max Kanat-Alexander r=bkor, a=myk --- Bugzilla/Bug.pm | 166 +++++++++++++++++++++++++++++++++++++++++++++++--- Bugzilla/Object.pm | 47 ++++++++++---- post_bug.cgi | 176 ++++++++++++++--------------------------------------- 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 - B The name of the product this bug is being +# filed against. +# C - B The name of the component this bug is being +# filed against. +# +# C - B The severity for the bug, a string. +# C - B A SQL timestamp for when the bug was created. +# C - B A summary for the bug. +# C - B The OS the bug was found against. +# C - B The initial priority for the bug. +# C - B The platform the bug was found against. +# C - B The version of the product the bug was found in. +# +# C - An alias for this bug. Will be ignored if C +# is off. +# C - When this bug is expected to be fixed. +# C - A string. +# C - The initial status of the bug, a string. +# C - The URL field. +# +# C - The full login name of the user who the bug is +# initially assigned to. +# C - The full login name of the QA Contact for this bug. +# Will be ignored if C is off. +# +# C - For time-tracking. Will be ignored if +# C is not set, or if the current +# user is not a member of the timetrackinggroup. +# C - For time-tracking. Will be ignored for the same +# reasons as C. + +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 and L. +=item C + +Description: Runs the validation of input parameters for L. + 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 called + by L. + +Params: The same as L. + +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 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. -- cgit v1.2.3-24-g4f1b