Bug 10131 - System preference search doesn't work for search terms starting with *
Summary: System preference search doesn't work for search terms starting with *
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: Jonathan Druart
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-26 14:59 UTC by Katrin Fischer
Modified: 2015-12-03 22:02 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 10131: Add a fallback if the pref search is not a valid regex (1.26 KB, patch)
2015-04-14 11:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10131: Add a fallback if the pref search is not a valid regex (1.48 KB, patch)
2015-04-14 14:49 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 10131: Add a fallback if the pref search is not a valid regex (1.48 KB, patch)
2015-04-14 14:50 UTC, M. Tompsett
Details | Diff | Splinter Review
[PASSED QA] Bug 10131: Add a fallback if the pref search is not a valid regex (1.55 KB, patch)
2015-04-15 20:42 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2013-04-26 14:59:07 UTC
If you try searching for a system preference with something like *black* you will get an ugly error message:

Software error:

Quantifier follows nothing in regex; marked by <-- HERE in m/* <-- HERE notes/ at /usr/share/koha/intranet/cgi-bin/admin/preferences.pl line 255.
Comment 1 Jonathan Druart 2015-04-14 11:49:38 UTC Comment hidden (obsolete)
Comment 2 M. Tompsett 2015-04-14 13:09:27 UTC
(In reply to Jonathan Druart from comment #1)
> 1/ Search for sysprefs with "notes.*", note the number of results
> 2/ Search for *notes*, boom
> 3/ Apply the patch
> 4/ Repeat 1 and confirm you get the same number of results
> 5/ Repeat 2 and confirm you don't get the error anymore

Actually, I've tried:
black     -- works (1 result)
black.*   -- works (1 result)
.*black   -- works (1 result)
.*black.* -- works (1 result)
black*    -- works (1 result)
*black    -- runs  (0 results)
*black*   -- runs  (0 results)

Perl 5.18.2 on Ubuntu 14.04 if that matters.
Why the two with 0 results?
Comment 3 M. Tompsett 2015-04-14 13:11:29 UTC
Since bugzilla didn't tell me that Jonathan got my question, and since he provided the patch, I changed the bug so he is the assigned person.
Comment 4 Jonathan Druart 2015-04-14 13:25:15 UTC
(In reply to M. Tompsett from comment #2)
> (In reply to Jonathan Druart from comment #1)
> > 1/ Search for sysprefs with "notes.*", note the number of results
> > 2/ Search for *notes*, boom
> > 3/ Apply the patch
> > 4/ Repeat 1 and confirm you get the same number of results
> > 5/ Repeat 2 and confirm you don't get the error anymore
> 
> Actually, I've tried:
> black     -- works (1 result)
> black.*   -- works (1 result)
> .*black   -- works (1 result)
> .*black.* -- works (1 result)
> black*    -- works (1 result)
> *black    -- runs  (0 results)
> *black*   -- runs  (0 results)
> 
> Perl 5.18.2 on Ubuntu 14.04 if that matters.
> Why the two with 0 results?

Because it's not a well formatted regex.
Comment 5 M. Tompsett 2015-04-14 14:49:08 UTC Comment hidden (obsolete)
Comment 6 M. Tompsett 2015-04-14 14:50:12 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2015-04-15 20:42:40 UTC
Created attachment 37920 [details] [review]
[PASSED QA] Bug 10131: Add a fallback if the pref search is not a valid regex

It's possible to search prefs using a regex.
But it the regex is not correctly written, the app explodes.
We should provide a fallback.

Test plan:
0/ Does not apply the patch
1/ Search for sysprefs with "notes.*", note the number of results
2/ Search for *notes*, boom
3/ Apply the patch
4/ Repeat 1 and confirm you get the same number of results
5/ Repeat 2 and confirm you don't get the error anymore

NOTE: As noted on comment #4, the kaboom is because of the
      leading * and not the following *, because 's*' is a valid
      regular expression, while '*n' is not.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Tomás Cohen Arazi 2015-04-21 16:55:57 UTC
Patch pushed to master.

Thanks Jonathan!
Comment 9 Chris Cormack 2015-04-22 05:52:19 UTC
Pushed to 3.18.x will be in 3.18.6