Closed Bug 194394 Opened 22 years ago Closed 22 years ago

Internal error/previous qa still acts as qa_contact, after turning useqacontact off

Categories

(Bugzilla :: Creating/Changing Bugs, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: jussi, Assigned: bbaetz)

References

Details

(Keywords: regression, Whiteboard: [fixed in 2.16.3] [fixed in 2.17.4])

Attachments

(2 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3b) Gecko/20030218
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3b) Gecko/20030218

Bugs with a qa contact cannot be viewed after turning off useqacontact. Trying
to do so, results in the following error message:

Internal Error

undef error - Can't use string ("4") as a HASH ref while "strict refs" in use at
Bug.pm line 382.

qa contact is the user with userid 4. Turning on useqacontact brings the bugs back.

Reproducible: Always

Steps to Reproduce:
1. set useqacontact on
2. Create a bug and assign qa
3. turn useqacontact off
4. try to view the created bug

Actual Results:  
Internal error
Attached patch fix by setting qa_contact to 0 (obsolete) — Splinter Review
This does fix the issue, but I'm not completely sure that this does not have
any side effects :)
Attachment #115153 - Flags: review?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Target Milestone: --- → Bugzilla 2.18
Hmm. What happens in the other case, where the param is turned on, but there
isn't a value?

The qa_contact stuff is so fragile this way....
Keywords: regression
Most of the places seem to check if qa is 0 before using it, so turning on
useqacontact should not have any effects. Problems could come if users are
deleted, or if someone has show_bug or enter_bug form which is created with the
old useqacontact value.

One other problem might be that CanSeeBug does not check useqacontact, so qa can
see the bug even if useqacontact is off. Not a real issue but zeroing the qa
after the CanSeeBug test might be.
Note once we try putting Bugzilla on a database that enforces referential
integrity, 0 won't work, because we don't have a user with that ID.  We'll have
to use NULL instead.
Jussi: a) The show_bug templte doesn't; instead, it relies on |fields()| to do
the pruning b) Thats a bug

dave: Thats a separate issue
Attached patch Try this insteadSplinter Review
This makes the qa_contant field only be used if qa contactas are enabled.

Bug.pm now hides the field's existance (although we may want to revisit that at
some point, and just treat it as |undef| in that case.

Also add a few param checks where they were needed.
Attachment #115153 - Attachment is obsolete: true
Attachment #115153 - Flags: review?
Comment on attachment 115292 [details] [diff] [review]
Try this instead

dave, this is the bug I mentioned to you earlier.
Attachment #115292 - Flags: review?(justdave)
-> me
Assignee: myk → bbaetz
Whiteboard: [wanted for 2.17.4]
Attachment #115292 - Flags: review?(justdave) → review+
Flags: approval+
Fixed.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
.. the checkCanChangeField bug affects 2.16, allowing a previous qa contact to
change bits on the bug which they shouldn't be. -> secure

Patch coming in a sec
Group: security
Summary: Internal error after turning useqacontact off → Internal error/previous qa still acts as qa_contact, after turning useqacontact off
Whiteboard: [wanted for 2.17.4] → [fixed in 2.17.4] [wanted for 2.16.3]
Attached patch 2.16 patchSplinter Review
CanSeeBug is ok in 2.16, because it uses the old SelectVisible routine which
did check this.

show_bug has been totally rewritten since then, so the innternal error isn't an
issue either.
wrong security group - 'oops' :)
Group: security → webtools-security
Attachment #115301 - Flags: review+
putting this back in the approval queue for the security checkin prior to
release (don't check it in yet)
Flags: approval+ → approval?
Thanks for the fix. It's nice to see that the 'not a real issue' was acted upon
this fast :)
Blocks: 190911
Flags: approval? → approval+
BUGZILLA-2_16-BRANCH:

Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.125.2.7; previous revision: 1.125.2.6
done
Whiteboard: [fixed in 2.17.4] [wanted for 2.16.3] → [fixed in 2.16.3] [fixed in 2.17.4]
Security Advisory has been posted, removing security group
Group: webtools-security
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: