571451 - Search.pm - illegal values in bug_status and resolution params not properly handled

Status: RESOLVED WORKSFORME

(Bugzilla :: Query/Bug List, defect)

Description

[C Farrell]

User-Agent:       Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; WOW64; Trident/4.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0)
Build Identifier: 3.6/cvs head

bug_status/resolution values are not properly handled, in some situations filtering is incorrectly skipped.


Reproducible: Always

Steps to Reproduce:
1. See https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&short_desc=bug_status&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&bug_status=THIS_WAS_VALID&bug_status=VERIFIED&bug_status=CLOSED&short_desc_type=allwordssubstr

2.  Running head version query 1 is ok.  To break head you have to duplicate a field and ommit another, such as: https://bugzilla.mozilla.org/buglist.cgi?query_format=advanced&short_desc=bug_status&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&bug_status=REOPENED&bug_status=VERIFIED&bug_status=CLOSED&short_desc_type=allwordssubstr
Actual Results:  
RESOLVED bugs are included in results, though it's not requested.  bug_status does not get filtered at all.

Expected Results:  
RESOLVED bugs should not be shown, illegal/duplicate values should be ignored as it has to be assumed that it was a previously deleted status.

URL shows the issue with bug_status filtering.  Same idea with causing failures in resolution filtering.  'THIS_WAS_VALID' doesn't seem to cause failure for bug_status on trunk.

Comment 1

[C Farrell]

Attachment: Fix handling of illegal values for bug_status/resolution

Fix I came up with for this issue which I found while trying to understand Search.pm - the lack of checks for resolution got my attention.

This patch moves some code to a couple sub's to allow for reuse between bug_status and resolution (or maybe others later).  This also reorders handling of __(all|open|closed)__ bug statuses to be checked first instead of last.  The interface doesn't offer an option to select these and normal fields.

The fact that browsers can send resolution = '---' and the database contains a blank string required a slight hack, but I'm assuming changing that label in the DB would be a big deal.

The new sub's do not accept the CGI $params as I'm working on https://bugzilla.mozilla.org/show_bug.cgi?id=398308 so this patch also help's isolate code that is not CGI dependant.

I ran tests with various bug_status/resolution selections, valid and invalid and it seems to do what I think is intended.

Comment 2

[Max Kanat-Alexander]

dkl: This is a dup of a bug you fixed, right?

Comment 3

[David Lawrence [:dkl]]

(In reply to comment #2)
> dkl: This is a dup of a bug you fixed, right?

Sort of. He is extending what I did by creating utility functions to be able to handle bug_status as well as resolution. I only fixed bug_status with my patch. He also is removing duplicates which I did not do so that is a good thing to add.

This may therefore be worth pursuing. 

Dave

Comment 4

[Frédéric Buclin]

Looks like 4.0 is affected, but not 4.1.1.

Comment 5

[Frédéric Buclin]

Marking WFM as this is no longer reproducible in 4.1.1 and above (also tested with 4.5).