The default bug view has changed. See this FAQ.

Software error when doing anywords or allwords search that starts or ends with a space

RESOLVED FIXED in Bugzilla 4.2

Status

()

Bugzilla
Query/Bug List
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Simon Green, Assigned: Simon Green)

Tracking

({regression})

4.2.1
Bugzilla 4.2
regression
Bug Flags:
approval +
blocking4.4 +
approval4.2 +
blocking4.2.2 +

Details

Attachments

(1 attachment, 1 obsolete attachment)

787 bytes, patch
glob
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
In the advance search page, you can get a software error if you do an anywords or allwords search that starts or ends with whitespace.

For example:
https://landfill.bugzilla.org/bugzilla-4.2-branch/buglist.cgi?query_format=advanced&longdesc=%20test&longdesc_type=allwordssubstr

This obviously isn't desired behaviour and should be fixed.
(Assignee)

Comment 1

5 years ago
Created attachment 628688 [details] [diff] [review]
v1 patch

This patch is against trunk. It will also apply cleanly (albeit with offsets) against Bugzilla 4.2 branch.
Assignee: query-and-buglist → sgreen+mozilla
Status: NEW → UNCONFIRMED
Ever confirmed: false
Attachment #628688 - Flags: review?
(Assignee)

Comment 2

5 years ago
It was a toss up between putting this change in the _substring_terms sub, or _handle_chart sub. Considering that _handle_chart trims the value if it was an array, I think this is the correct place to put it.
(Assignee)

Updated

5 years ago

Updated

5 years ago
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: blocking4.2.2+
Keywords: regression
OS: Linux → All
Hardware: x86 → All
Target Milestone: --- → Bugzilla 4.2
i'm not sure about this fix; i'm concerned about possible side-effects of trimming all values via __handle_chart().

to me it looks like the problem is in _multiselect_multiple:

    my @words = $operator eq 'anyexact' ? $self->_all_values($args)
                                        : split(/[\s,]+/, $value);
    my @terms;
    foreach my $word (@words) {
        $args->{value} = $word;
        $args->{quoted} = $dbh->quote($word);
        push(@terms, $self->_multiselect_term($args));
    }

this loop should skip empty $word's...

    foreach my $word (@words) {
        next if $word eq '';
        $args->{value} = $word;
        ...
(Assignee)

Comment 4

5 years ago
(In reply to Byron Jones ‹:glob› from comment #3)
> i'm not sure about this fix; i'm concerned about possible side-effects of
> trimming all values via __handle_chart().

As I mentioned in comment #2, it was a bit of a toss up about where to put this. The reason I put it where it is is due to the fact we were using trim on value if it was passed in as an array.

I'd be interested to know of a use cases where this could go wrong.
(In reply to Simon Green from comment #4)
> As I mentioned in comment #2, it was a bit of a toss up about where to put
> this. The reason I put it where it is is due to the fact we were using trim
> on value if it was passed in as an array.

after sleeping on this, i think your approach is better, and i retract my comment :)

> I'd be interested to know of a use cases where this could go wrong.

well, i was surprised to see a sql error generated as a result of a value /not/ being trimmed; even after the rewrite, which simplified things a lot, it's still a complex module :)
Comment on attachment 628688 [details] [diff] [review]
v1 patch

r=glob
Attachment #628688 - Flags: review? → review+
Flags: approval?
Flags: approval4.2?

Comment 7

5 years ago
Comment on attachment 628688 [details] [diff] [review]
v1 patch

You are introducing another bug. To prevent "bug" from matching "bugzilla", one can type "bug " (a trick which I also use when I'm looking for flag but not flagtype), but due to trim(), this trick will no longer work. The issue is not about untrimmed values. The problem is about empty strings. So we should really fix this problem only.
Attachment #628688 - Flags: review-

Updated

5 years ago
Flags: approval?
Flags: approval4.2?

Updated

5 years ago
Duplicate of this bug: 761300

Updated

5 years ago
Flags: blocking4.4+
(Assignee)

Comment 9

5 years ago
Created attachment 630784 [details] [diff] [review]
v2 patch

Updated patch
Attachment #628688 - Attachment is obsolete: true
Attachment #630784 - Flags: review?
Comment on attachment 630784 [details] [diff] [review]
v2 patch

r=glob
Attachment #630784 - Flags: review? → review+
Flags: approval?
Flags: approval4.2?

Comment 11

5 years ago
Thanks for this fix. :)
Flags: approval?
Flags: approval4.2?
Flags: approval4.2+
Flags: approval+
Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bugzilla/4.2/
modified Bugzilla/Search.pm
Committed revision 8095.

Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Search.pm
Committed revision 8259.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.