Closed
Bug 556579
Opened 15 years ago
Closed 14 years ago
Quoting protection broken in QuickSearch again
Categories
(Bugzilla :: Query/Bug List, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 4.0
People
(Reporter: wicked, Assigned: mkanat)
References
Details
(Keywords: regression)
Attachments
(1 file)
(deleted),
patch
|
LpSolit
:
review+
|
Details | Diff | Splinter Review |
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. :(
Updated•15 years ago
|
Target Milestone: --- → Bugzilla 3.8
Assignee | ||
Comment 1•15 years ago
|
||
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.
Reporter | ||
Comment 2•15 years ago
|
||
(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?
Assignee | ||
Comment 3•15 years ago
|
||
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.
Comment 4•15 years ago
|
||
This regression is already annoying me very seriously! :)
Severity: normal → major
Updated•14 years ago
|
Flags: blocking4.0+
Assignee | ||
Comment 5•14 years ago
|
||
LpSolit: What specifically are the cases that you're having trouble with, and how is this different from bug 553884?
Assignee | ||
Comment 6•14 years ago
|
||
Okay, I think I may have a clever fix for this. I need to test various combinations, though.
Assignee: query-and-buglist → mkanat
Comment 7•14 years ago
|
||
(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.
Assignee | ||
Comment 8•14 years ago
|
||
Okay, I'm working on a fix, but cases like this make it tricky:
"-6"|summary:"blah blah",foo
Assignee | ||
Comment 9•14 years ago
|
||
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.
Assignee | ||
Comment 10•14 years ago
|
||
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.
Assignee | ||
Comment 11•14 years ago
|
||
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 12•14 years ago
|
||
Comment on attachment 459380 [details] [diff] [review]
v1
r=LpSolit
Attachment #459380 -
Flags: review?(LpSolit) → review+
Updated•14 years ago
|
Flags: approval4.0+
Flags: approval+
Assignee | ||
Comment 13•14 years ago
|
||
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.
Description
•