@mattab opened this Issue on August 7th 2014 Owner

Reproduce:

  • go to Actions report,
  • search for ?param and the search issues a warning message:
Warning: preg_match() [function.preg-match]: Compilation failed: nothing to repeat at offset 0 in /home/piwik-demo/www/demo.piwik.org/core/DataTable/Filter/Pattern.php on line 72

Backtrace -->

<a href='/0'>#0</a> Piwik\Error::errorHandler(...) called at [:]
<a href='/1'>#1</a> preg_match(...) called at [/home/piwik-demo/www/demo.piwik.org/core/DataTable/Filter/Pattern.php:72]
<a href='/2'>#2</a> Piwik\DataTable\Filter\Pattern::match(...) called at [/home/piwik-demo/www/demo.piwik.org/core/DataTable/Filter/PatternRecursive.php:80]
<a href='/3'>#3</a> Piwik\DataTable\Filter\PatternRecursive->filter(...) called at [/home/piwik-demo/www/demo.piwik.org/core/DataTable.php:425]

and then

Warning: preg_match() [function.preg-match]: Compilation failed: nothing to repeat at offset 0 in /home/piwik-demo/www/demo.piwik.org/core/DataTable/Filter/Pattern.php on line 72
@mattab commented on August 10th 2014 Owner

this was also reported in forum

@tsteur commented on August 11th 2014 Owner

At first I wanted to fix this issue in the PHP pattern filter by escaping those quantifiers as I did in JavaScript but then no quantifiers would ever work again there as the filter might be also used in another context. Therefore did it in JavaScript. Also escaped + and * which caused the same warning.

The side effect is that you could no longer write a regex in the search box like (nn)+ since the + would be escaped to (nn)\+. I am not sure if this was actually a feature and meant to be like this? If so, we could probably only escape those characters if it is the first character. Since less users are aware of regex I would assume people would rather want + to be interpreted as \+ as it is the case now

@mattab commented on August 11th 2014 Owner

I am not sure if this was actually a feature and meant to be like this?

it is a feature to let users write regular expressions in the search bar (and in API request via filter_pattern), eg. you can search for (en|de)\.wikipedia (and any other regex including * and +)

do you know if it's possible to detect whether a given search string is a regular expression or not?

(if we could detect this, then we could enable regular expression only when input string is indeed regular expression...)

I wonder how Google Analytics handles this case. After doing some quick test it seems their datatable search input box works with both regular expressions and simple search. For example if I search for + then it returns all URLs containing + character. But if I input regular expression containing + it still work as regular expression.

Maybe the best solution is to:

  • run the search as "plain text search"
  • run the search as regular expression (if the search is a valid regex string)

and then we could return the "union" of both searches?

@tsteur commented on August 12th 2014 Owner

As mentioned above I would only escape the first character if it is a quantifier in this case. I think it is very tricky to detect it otherwise. Checking for brackets on the left or right or so wouldn't work since even jpe?g is valid.

@tsteur commented on August 12th 2014 Owner

I don't think executing a pattern twice is the best idea since a user might get results that he doesn't want. Autodetecting/magic can be very annoying in case the auto detect does not work correctly in a case and you will never know which result you get.

Documentation says nothing about regexp:

filter_pattern; defines the text you want to search for in the filter_column. Only the row with the given column matching the pattern will be returned.

Maybe we should mention it there and then it is ok. For API users which are usually advanced users this behavior is ok. Best case would be maybe to have two different attributes (filter_pattern and filter_text for instance). This would be clear.

For regular users that use the UI I would personally prefer not to apply a regex by default and treat it as text. IDEs and text editors solve this problem by having a checkbox "Regex". But this makes the UI more complicated again. A solution could be to force advanced users to write regex within a leading and ending slash /$myRegex/ and show this as a tooltip on hover (title)

@tsteur commented on August 15th 2014 Owner

I'll close it for now as it fixes the reported issue. Maybe we can otherwise open a new ticket if needed?

@mattab commented on August 15th 2014 Owner

maybe we could mention regex in the API documentation for filter_pattern?
otherwise looks good to me

@tsteur commented on August 15th 2014 Owner

I did it this morning see previous commit ;)

This Issue was closed on August 15th 2014
Powered by GitHub Issue Mirror