Bug 526158: Make email_in.pl use Bugzilla::Bug->create to create bugs instead of requiring post_bug.cgi

Patch by Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit

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 = <STDIN>;
 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<enter_bug.cgi>. For the list of required field names, see 
-L<Bugzilla::WebService::Bug/Create>. 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<platform> in web is C<@rep_platform> in email
-
-=item *
-
-C<severity> in web is C<@bug_severity> in email
-
-=back
-
-For the list of all field names, see the C<fielddefs> 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<Bugzilla::WebService::Bug/create>.
+(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<account@domain.com> must be a valid Bugzilla account.
+C<account@domain.com> (the value of the C<From> 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<Subject> 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::(.+)') ) %]