Closed Bug 556579 Opened 15 years ago Closed 14 years ago

Quoting protection broken in QuickSearch again

Categories

(Bugzilla :: Query/Bug List, defect)

defect
Not set
major

Tracking

()

RESOLVED FIXED
Bugzilla 4.0

People

(Reporter: wicked, Assigned: mkanat)

References

Details

(Keywords: regression)

Attachments

(1 file)

There was a reason url_quote/url_decode was used in splitWords. They were used to protect special characters inside quotes from getting interpreted. Unfortunately this protection was removed in bug 554819 so we have now reregressed bug 366120 and subsequently bug 366121. :(
Target Milestone: --- → Bugzilla 3.8
  Ah, ha, so that's what's causing bug 553884 as well, then? But that's happening on all branches. In any case, this can be resolved, but I think that url-quoting and then decoding stuff may not be the best way, architecturally, to solve it.
(In reply to comment #1)
>   Ah, ha, so that's what's causing bug 553884 as well, then? But that's
> happening on all branches. In any case, this can be resolved, but I think that

I was thinking that but that one does indeed affect older branches too and this regression seems to only affect trunk.

> url-quoting and then decoding stuff may not be the best way, architecturally,
> to solve it.

Perhaps, that was just how it worked in the JS version and I just copied it back to the Perl-version after it went missing during the conversion. Feel free to come up with a better way. ;) Maybe by giving some clever options to the Text Parser -module?
  Yeah, there's an option to keep quotes, and then when we get things back that are quoted, we could do something clever with them. Not sure what, but something.
This regression is already annoying me very seriously! :)
Severity: normal → major
Flags: blocking4.0+
LpSolit: What specifically are the cases that you're having trouble with, and how is this different from bug 553884?
Okay, I think I may have a clever fix for this. I need to test various combinations, though.
Assignee: query-and-buglist → mkanat
(In reply to comment #5)
> LpSolit: What specifically are the cases that you're having trouble with, and
> how is this different from bug 553884?

Bug 553884 is about negation in quotes. This bug is about everything else. For instance, you no longer can look for bugs with sg:cri in the status whiteboard, because QS detects colons in quotes, and treats them as the field "sg" with value "cri", while it's really "sg:cri". The same problem happens with commas, AND and OR in quotes.
Okay, I'm working on a fix, but cases like this make it tricky:

  "-6"|summary:"blah blah",foo
All right, I think the fix is to move the AND/OR/NOT stuff into a subroutine that also handles splitting words, and do the word-splitting a bit differently, so that we can mark certain words as being literal without having to worry about whether we're inside an OR.
  Okay, I suspect that the simplest solution will just be to go back to using splitWords, for now. I don't like it, because we shouldn't have to urldecode/urlencode things--we should have a more explicit way of marking things as literal, but that's a lot of refactoring to do, and we're too close to the freeze for 4.0.
Attached patch v1Splinter Review
This just directly reverses the patch from bug 554819.

I will have to come up with a better way to handle this, which will probably involve making quicksearch more object-oriented, and then also having some sort of "QuicksearchLiteral" object or something that lets me know that a string was quoted.
Attachment #459380 - Flags: review?(LpSolit)
Comment on attachment 459380 [details] [diff] [review]
v1

r=LpSolit
Attachment #459380 - Flags: review?(LpSolit) → review+
Flags: approval4.0+
Flags: approval+
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Search/Quicksearch.pm
Committed revision 7395.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/Search/Quicksearch.pm
Committed revision 7339.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: