Last Comment Bug 412943 - When email_in.pl calls process_bug.cgi in Bugzilla 3.0, no qa_contact field causes update failure
: When email_in.pl calls process_bug.cgi in Bugzilla 3.0, no qa_contact field c...
Status: RESOLVED FIXED
[3.0.x branch only][3.2 not affected]
:
Product: Bugzilla
Classification: Server Software
Component: Creating/Changing Bugs (show other bugs)
: 3.0.3
: All All
: -- normal (vote)
: Bugzilla 3.0
Assigned To: Frédéric Buclin
: default-qa
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-01-18 06:23 PST by Alex Schuilenburg
Modified: 2008-02-03 13:51 PST (History)
1 user (show)
mkanat: approval3.0+
mkanat: blocking3.0.4+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Fix to only define $formhash{'qa_contact'} when $qacontact is also deinfed (836 bytes, patch)
2008-01-18 06:25 PST, Alex Schuilenburg
LpSolit: review-
Details | Diff | Review
patch, v2 (792 bytes, patch)
2008-02-03 12:53 PST, Frédéric Buclin
mkanat: review+
Details | Diff | Review

Description Alex Schuilenburg 2008-01-18 06:23:18 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11
Build Identifier: 3.0.3

With Param('useqacontact') set, when adding only a comment to an existing bug with email_in.pl, process_bug.cgi fails to update when sender is not in caneditbugs group with the error:

You tried to change the
    QA Contact field
      from <Component QA Contact>
    , but only
      the assignee
      of the bug, or
    a sufficiently empowered user may change that field.

<Component QA Contact> is obvioulsy the QA contact for the component.



Reproducible: Always

Steps to Reproduce:
1. Run email_in.pl either directly or through email alias pipe
2. Input or send email with From: field set to a basic user which created the bug and Subject: field set to [Bug XXX] appropriately, with message content a simple comment
3. 
Actual Results:  
View output or receive error that says:
You tried to change the
    QA Contact field
      from <Component QA Contact>
    , but only
      the assignee
      of the bug, or
    a sufficiently empowered user may change that field.

Expected Results:  
Updating Bug XXX

The bug is actually in process_bug.cgi.  

$formhash{'qa_contact'} is set to $qa_contact which may never be defined (e.g. by email_bug.pl), and the later test "exists $formhash{'qa_contact'}" will be true as the value will be undef.

The solution is to only set $formhash{'qa_contact'} when Param('useqacontact') is set AND qa_contact is defined.  See attached patch.
Comment 1 Alex Schuilenburg 2008-01-18 06:25:40 PST
Created attachment 297761 [details] [diff] [review]
Fix to only define $formhash{'qa_contact'} when $qacontact is also deinfed
Comment 2 Frédéric Buclin 2008-01-18 08:58:00 PST
Confirming. This bug also occurs if you hack the URL and omit the QA field.
Comment 3 Frédéric Buclin 2008-01-18 09:47:06 PST
Comment on attachment 297761 [details] [diff] [review]
Fix to only define $formhash{'qa_contact'} when $qacontact is also deinfed

>-    $formhash{'qa_contact'} = $qacontact if Bugzilla->params->{'useqacontact'};
>+    $formhash{'qa_contact'} = $qacontact if (Bugzilla->params->{'useqacontact'} && defined($qacontact));

This fixes the problem with email_in.pl, but introduces a new regression: if a product has no QA contact by default but a QA contact is set on a bug, then your fix lets unprivileged users remove it by hacking the POST data (delete the QA field and set knob = 'reassignbycomponent'). I think I know how to fix the problem. Alex, do you mind if I take the bug and fix it myself, or do you want to give another shot?
Comment 4 Frédéric Buclin 2008-01-18 10:21:44 PST
Thinking about it a bit more, I wonder if I couldn't already hack the data to reproduce my testcase. Investigating...
Comment 5 Alex Schuilenburg 2008-01-18 11:56:01 PST
You are most welcome to take the bug and fix it yourself :-)
Comment 6 Max Kanat-Alexander 2008-01-19 11:13:39 PST

*** This bug has been marked as a duplicate of bug 391669 ***
Comment 7 Max Kanat-Alexander 2008-01-19 11:15:40 PST
Ah, nevermind. I see that this bug applies to 3.0, and the other bug applies to tip, and they're different enough now (that is, process_bug is so different between the releases) that they should have separate bugs.
Comment 8 Max Kanat-Alexander 2008-01-19 14:56:52 PST
And yeah, we should definitely fix this before 3.0.4.
Comment 9 Frédéric Buclin 2008-02-03 12:53:30 PST
Created attachment 301160 [details] [diff] [review]
patch, v2

Rather than trying to fix process_bug.cgi and introducing unwanted regressions, I'm fixing email_in.pl to set qa_contact in all cases. If useqacontact is off, $bug->qa_contact is undefined and will be correctly ignored by process_bug.cgi.
Comment 10 Max Kanat-Alexander 2008-02-03 13:28:42 PST
Comment on attachment 301160 [details] [diff] [review]
patch, v2

Okay. If you've tested this, this looks sensible.
Comment 11 Frédéric Buclin 2008-02-03 13:51:00 PST
Checking in email_in.pl;
/cvsroot/mozilla/webtools/bugzilla/email_in.pl,v  <--  email_in.pl
new revision: 1.5.2.6; previous revision: 1.5.2.5
done

Note You need to log in before you can comment on or make changes to this bug.