From 59a35402749a8b20db50b810ce29c1479594dced Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Thu, 10 Dec 2009 22:18:05 +0000 Subject: Bug 526158: Make email_in.pl use Bugzilla::Bug->create to create bugs instead of requiring post_bug.cgi Patch by Max Kanat-Alexander r=LpSolit, a=LpSolit --- Bugzilla/Bug.pm | 56 ++++++++++++++- Bugzilla/User.pm | 9 ++- Bugzilla/WebService/Bug.pm | 43 ++---------- email_in.pl | 92 +++++++++---------------- post_bug.cgi | 11 +-- template/en/default/global/user-error.html.tmpl | 2 + 6 files changed, 104 insertions(+), 109 deletions(-) diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index d55549cd6..83e95aecd 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -236,6 +236,27 @@ use constant UPDATE_COMMENT_COLUMNS => qw( # activity table. use constant MAX_LINE_LENGTH => 254; +# This maps the names of internal Bugzilla bug fields to things that would +# make sense to somebody who's not intimately familiar with the inner workings +# of Bugzilla. (These are the field names that the WebService and email_in.pl +# use.) +use constant FIELD_MAP => { + creation_time => 'creation_ts', + description => 'comment', + id => 'bug_id', + last_change_time => 'delta_ts', + platform => 'rep_platform', + severity => 'bug_severity', + status => 'bug_status', + summary => 'short_desc', + url => 'bug_file_loc', + whiteboard => 'status_whiteboard', + + # These are special values for the WebService Bug.search method. + limit => 'LIMIT', + offset => 'OFFSET', +}; + ##################################################################### sub new { @@ -557,7 +578,6 @@ sub create { return $bug; } - sub run_create_validators { my $class = shift; my $params = $class->SUPER::run_create_validators(@_); @@ -1168,6 +1188,9 @@ sub _check_cc { my ($invocant, $component, $ccs) = @_; return [map {$_->id} @{$component->initial_cc}] unless $ccs; + # Allow comma-separated input as well as arrayrefs. + $ccs = [split(/[\s,]+/, $ccs)] if !ref $ccs; + my %cc_ids; foreach my $person (@$ccs) { next unless $person; @@ -1732,6 +1755,17 @@ sub _check_freetext_field { sub _check_multi_select_field { my ($invocant, $values, $field) = @_; + + # Allow users (mostly email_in.pl) to specify multi-selects as + # comma-separated values. + if (defined $values and !ref $values) { + # We don't split on spaces because multi-select values can and often + # do have spaces in them. (Theoretically they can have commas in them + # too, but that's much less common and people should be able to work + # around it pretty cleanly, if they want to use email_in.pl.) + $values = [split(',', $values)]; + } + return [] if !$values; my @checked_values; foreach my $value (@$values) { @@ -1865,6 +1899,7 @@ sub set_component { } sub set_custom_field { my ($self, $field, $value) = @_; + if (ref $value eq 'ARRAY' && $field->type != FIELD_TYPE_MULTI_SELECT) { $value = $value->[0]; } @@ -3177,6 +3212,25 @@ sub LogActivityEntry { } } +# Convert WebService API and email_in.pl field names to internal DB field +# names. +sub map_fields { + my ($params) = @_; + + my %field_values; + foreach my $field (keys %$params) { + my $field_name = FIELD_MAP->{$field} || $field; + $field_values{$field_name} = $params->{$field}; + } + + # This protects the WebService Bug.search method. + unless (Bugzilla->user->is_timetracker) { + delete @field_values{qw(estimated_time remaining_time deadline)}; + } + + return \%field_values; +} + # CountOpenDependencies counts the number of open dependent bugs for a # list of bugs and returns a list of bug_id's and their dependency count # It takes one parameter: diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 7c458388c..3e6f3b6ba 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -716,7 +716,14 @@ sub can_enter_product { my $dbh = Bugzilla->dbh; $warn ||= 0; - if (!defined $input) { + $input = trim($input) if !ref $input; + if (!defined $input or $input eq '') { + return unless $warn == THROW_ERROR; + ThrowUserError('object_not_specified', + { class => 'Bugzilla::Product' }); + } + + if (!scalar @{ $self->get_enterable_products }) { return unless $warn == THROW_ERROR; ThrowUserError('no_products'); } diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm index ad4bb9f30..33135f059 100644 --- a/Bugzilla/WebService/Bug.pm +++ b/Bugzilla/WebService/Bug.pm @@ -37,24 +37,6 @@ use Bugzilla::Util qw(trim); # Constants # ############# -# This maps the names of internal Bugzilla bug fields to things that would -# make sense to somebody who's not intimately familiar with the inner workings -# of Bugzilla. (These are the field names that the WebService uses.) -use constant FIELD_MAP => { - creation_time => 'creation_ts', - description => 'comment', - id => 'bug_id', - last_change_time => 'delta_ts', - platform => 'rep_platform', - severity => 'bug_severity', - status => 'bug_status', - summary => 'short_desc', - url => 'bug_file_loc', - whiteboard => 'status_whiteboard', - limit => 'LIMIT', - offset => 'OFFSET', -}; - use constant PRODUCT_SPECIFIC_FIELDS => qw(version target_milestone component); use constant DATE_FIELDS => { @@ -251,7 +233,7 @@ sub search { { param => 'limit', function => 'Bug.search()' }); } - $params = _map_fields($params); + $params = Bugzilla::Bug::map_fields($params); delete $params->{WHERE}; # Do special search types for certain fields. @@ -286,7 +268,7 @@ sub search { sub create { my ($self, $params) = @_; Bugzilla->login(LOGIN_REQUIRED); - $params = _map_fields($params); + $params = Bugzilla::Bug::map_fields($params); my $bug = Bugzilla::Bug->create($params); Bugzilla::BugMail::Send($bug->bug_id, { changer => $bug->reporter->login }); return { id => $self->type('int', $bug->bug_id) }; @@ -294,7 +276,8 @@ sub create { sub legal_values { my ($self, $params) = @_; - my $field = FIELD_MAP->{$params->{field}} || $params->{field}; + my $field = Bugzilla::Bug::FIELD_MAP->{$params->{field}} + || $params->{field}; my @global_selects = Bugzilla->get_fields( {type => [FIELD_TYPE_SINGLE_SELECT, FIELD_TYPE_MULTI_SELECT]}); @@ -520,24 +503,6 @@ sub _attachment_to_hash { }; } -# Convert WebService API field names to internal DB field names. -# Used by create() and search(). -sub _map_fields { - my ($params) = @_; - - my %field_values; - foreach my $field (keys %$params) { - my $field_name = FIELD_MAP->{$field} || $field; - $field_values{$field_name} = $params->{$field}; - } - - unless (Bugzilla->user->is_timetracker) { - delete @field_values{qw(estimated_time remaining_time deadline)}; - } - - return \%field_values; -} - 1; __END__ diff --git a/email_in.pl b/email_in.pl index 2d0ebe571..cb9492a1e 100755 --- a/email_in.pl +++ b/email_in.pl @@ -45,12 +45,13 @@ use Encode; use Bugzilla; use Bugzilla::Bug; +use Bugzilla::BugMail; use Bugzilla::Constants qw(USAGE_MODE_EMAIL); use Bugzilla::Error; use Bugzilla::Mailer; +use Bugzilla::Token; use Bugzilla::User; use Bugzilla::Util; -use Bugzilla::Token; ############# # Constants # @@ -74,9 +75,6 @@ sub parse_mail { my %fields; - # Email::Address->parse returns an array - my ($reporter) = Email::Address->parse($input_email->header('From')); - $fields{'reporter'} = $reporter->address; my $summary = $input_email->header('Subject'); if ($summary =~ /\[\S+ (\d+)\](.*)/i) { $fields{'bug_id'} = $1; @@ -107,19 +105,8 @@ sub parse_mail { # Otherwise, we stop parsing fields on the first blank line. $line = trim($line); last if !$line; - - if ($line =~ /^@(\S+)\s*=\s*(.*)\s*/) { + if ($line =~ /^\@(\S+)\s*(?:=|\s)\s*(.*)\s*/) { $current_field = lc($1); - # It's illegal to pass the reporter field as you could - # override the "From:" field of the message and bypass - # authentication checks, such as PGP. - if ($current_field eq 'reporter') { - # We reset the $current_field variable to something - # post_bug and process_bug will ignore, in case the - # attacker splits the reporter field on several lines. - $current_field = 'illegal_field'; - next; - } $fields{$current_field} = $2; } else { @@ -128,6 +115,10 @@ sub parse_mail { } } + %fields = %{ Bugzilla::Bug::map_fields(\%fields) }; + + my ($reporter) = Email::Address->parse($input_email->header('From')); + $fields{'reporter'} = $reporter->address; # The summary line only affects us if we're doing a post_bug. # We have to check it down here because there might have been @@ -150,24 +141,24 @@ sub parse_mail { } sub post_bug { - my ($fields_in) = @_; - my %fields = %$fields_in; - + my ($fields) = @_; debug_print('Posting a new bug...'); - my $cgi = Bugzilla->cgi; - foreach my $field (keys %fields) { - $cgi->param(-name => $field, -value => $fields{$field}); + # Bugzilla::Bug->create throws a confusing CodeError if + # the REQUIRED_CREATE_FIELDS are missing, but much more + # sensible errors if the fields exist but are just undef. + foreach my $field (Bugzilla::Bug::REQUIRED_CREATE_FIELDS) { + $fields->{$field} = undef if !exists $fields->{$field}; } - $cgi->param(-name => 'inbound_email', -value => 1); - - require 'post_bug.cgi'; + my $bug = Bugzilla::Bug->create($fields); + debug_print("Created bug " . $bug->id); + Bugzilla::BugMail::Send($bug->id, { changer => $bug->reporter->login }); + debug_print("Sent bugmail"); } sub process_bug { my ($fields_in) = @_; - my %fields = %$fields_in; my $bug_id = $fields{'bug_id'}; @@ -340,7 +331,6 @@ pod2usage({-verbose => 0, -exitval => 1}) if $switch{'help'}; Bugzilla->usage_mode(USAGE_MODE_EMAIL); - my @mail_lines = ; my $mail_text = join("", @mail_lines); my $mail_fields = parse_mail($mail_text); @@ -351,9 +341,7 @@ if (my $suffix = Bugzilla->params->{'emailsuffix'}) { $username =~ s/\Q$suffix\E$//i; } -my $user = Bugzilla::User->new({ name => $username }) - || ThrowUserError('invalid_username', { name => $username }); - +my $user = Bugzilla::User->check($username); Bugzilla->set_user($user); if ($mail_fields->{'bug_id'}) { @@ -391,9 +379,9 @@ The script expects to read an email with the following format: From: account@domain.com Subject: Bug Summary - @product = ProductName - @component = ComponentName - @version = 1.0 + @product ProductName + @component ComponentName + @version 1.0 This is a bug description. It will be entered into the bug exactly as written here. @@ -404,39 +392,25 @@ The script expects to read an email with the following format: This is a signature line, and will be removed automatically, It will not be included in the bug description. -The C<@> labels can be any valid field name in Bugzilla that can be -set on C. For the list of required field names, see -L. Note, that there is some difference -in the names of the required input fields between web and email interfaces, -as listed below: - -=over - -=item * - -C in web is C<@rep_platform> in email - -=item * - -C in web is C<@bug_severity> in email - -=back - -For the list of all field names, see the C table in the database. +For the list of valid field names for the C<@> fields, including +a list of which ones are required, see L. +(Note, however, that you cannot specify C<@description> as a field-- +you just add a comment by adding text after the C<@> fields.) The values for the fields can be split across multiple lines, but note that a newline will be parsed as a single space, for the value. So, for example: - @short_desc = This is a very long + @summary This is a very long description Will be parsed as "This is a very long description". -If you specify C<@short_desc>, it will override the summary you specify +If you specify C<@summary>, it will override the summary you specify in the Subject header. -C must be a valid Bugzilla account. +C (the value of the C header) must be a valid +Bugzilla account. Note that signatures must start with '-- ', the standard signature border. @@ -453,11 +427,11 @@ Your subject starts with [Bug 123456] -- then it modifies bug 123456. =item * -You include C<@bug_id = 123456> in the first lines of the email. +You include C<@id 123456> in the first lines of the email. =back -If you do both, C<@bug_id> takes precedence. +If you do both, C<@id> takes precedence. You send your email in the same format as for creating a bug, except that you only specify the fields you want to change. If the very @@ -466,7 +440,7 @@ will be assumed that you are only adding a comment to the bug. Note that when updating a bug, the C header is ignored, except for getting the bug ID. If you want to change the bug's summary, -you have to specify C<@short_desc> as one of the fields to change. +you have to specify C<@summary> as one of the fields to change. Please remember not to include any extra text in your emails, as that text will also be added as a comment. This includes any text that your @@ -476,8 +450,6 @@ another email. =head3 Adding/Removing CCs To add CCs, you can specify them in a comma-separated list in C<@cc>. -For backward compatibility, C<@newcc> can also be used. If both are -present, C<@cc> takes precedence. To remove CCs, specify them as a comma-separated list in C<@removecc>. diff --git a/post_bug.cgi b/post_bug.cgi index ef0f40d4d..749acb492 100755 --- a/post_bug.cgi +++ b/post_bug.cgi @@ -264,13 +264,8 @@ if ($token) { ("createbug:$id", $token)); } -if (Bugzilla->usage_mode == USAGE_MODE_EMAIL) { - Bugzilla::BugMail::Send($id, $vars->{'mailrecipients'}); -} -else { - print $cgi->header(); - $template->process("bug/create/created.html.tmpl", $vars) - || ThrowTemplateError($template->error()); -} +print $cgi->header(); +$template->process("bug/create/created.html.tmpl", $vars) + || ThrowTemplateError($template->error()); 1; diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index 7dea01452..c4602f7d8 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -1741,6 +1741,8 @@ flagtype [% ELSIF class == "Bugzilla::Field" %] field + [% ELSIF class == "Bugzilla::Product" %] + product [% ELSIF class == "Bugzilla::Search::Saved" %] saved search [% ELSIF ( matches = class.match('^Bugzilla::Field::Choice::(.+)') ) %] -- cgit v1.2.3-24-g4f1b