After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 719399 - [extensions] AddBlock shows CRITICAL when the regex compilation fails
[extensions] AddBlock shows CRITICAL when the regex compilation fails
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: General
3.8.x (obsolete)
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
: 706481 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-11-27 09:19 UTC by Andres Gomez
Modified: 2014-01-02 20:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
uri-tester: Compile regex in JavaScript compatibility mode (1.17 KB, patch)
2013-11-27 10:12 UTC, Andres Gomez
reviewed Details | Review
Simple test case showing the WARNING (637 bytes, text/x-csrc)
2013-11-27 10:19 UTC, Andres Gomez
  Details
uri-tester: Compile regex in JavaScript compatibility mode (1018 bytes, patch)
2013-11-27 10:48 UTC, Andres Gomez
accepted-commit_now Details | Review

Description Andres Gomez 2013-11-27 09:19:56 UTC
Debian Testing

$ dpkg -l| grep epi
ii  epiphany-browser                     3.8.2-4                       amd64        Intuitive GNOME web browser
ii  epiphany-browser-data                3.8.2-4                       all          Data files for the GNOME web browser
ii  epiphany-browser-dbg                 3.8.2-4                       amd64        Debugging symbols for the GNOME web browser


Everytime we launch ephy we get:
** (WebKitWebProcess:6193): WARNING **: uri_tester_compile_regexp: Error while compiling regular expression /cdn-cgi/pe/bag\?r[]=.*cpalead.com at char 34: missing terminating ] for character class

(WebKitWebProcess:6193): GLib-CRITICAL **: g_regex_unref: assertion `regex != NULL' failed
Comment 1 Andres Gomez 2013-11-27 09:25:11 UTC
The failing uri comes from:
https://easylist-downloads.adblockplus.org/easylist.txt

Although the compilation shouldn't fail, I suppose this is a bug in the addblock list itself or, maybe, lack of using the G_REGEX_JAVASCRIPT_COMPAT GRegexCompileFlags[1]

In any case, the GLib-CRITICAL should be avoided.

[1] https://developer.gnome.org/glib/2.38/glib-Perl-compatible-regular-expressions.html#G-REGEX-JAVASCRIPT-COMPAT:CAPS
Comment 2 Yosef Or Boczko 2013-11-27 09:27:23 UTC
It fixed in 79db933d667fe0208aaf1fc58f8bd84d6a0dc162 commit.
I not get more this error with epiphany from master.
Comment 3 Andres Gomez 2013-11-27 10:12:16 UTC
Created attachment 262918 [details] [review]
uri-tester: Compile regex in JavaScript compatibility mode

AddBlock regex now are compiled in JavaScript compatibility mode and
the resulting pointer is not tried to be cleared if there is also a
returning error as it will always be NULL in that case.
Comment 4 Yosef Or Boczko 2013-11-27 10:13:10 UTC
You attached a patch, so reopened.
Comment 5 Andres Gomez 2013-11-27 10:14:01 UTC
(In reply to comment #2)
> It fixed in 79db933d667fe0208aaf1fc58f8bd84d6a0dc162 commit.
> I not get more this error with epiphany from master.

Is this even related?
Comment 6 Yosef Or Boczko 2013-11-27 10:17:09 UTC
(In reply to comment #5)
> (In reply to comment #2)
> > It fixed in 79db933d667fe0208aaf1fc58f8bd84d6a0dc162 commit.
> > I not get more this error with epiphany from master.
> 
> Is this even related?

I think it relate to this:
** (WebKitWebProcess:6193): WARNING **: uri_tester_compile_regexp: Error while
compiling regular expression /cdn-cgi/pe/bag\?r[]=.*cpalead.com at char 34:
missing terminating ] for character class

bot not to this:
(WebKitWebProcess:6193): GLib-CRITICAL **: g_regex_unref: assertion `regex !=
NULL' failed
Comment 7 Andres Gomez 2013-11-27 10:19:05 UTC
Created attachment 262919 [details]
Simple test case showing the WARNING
Comment 8 Claudio Saavedra 2013-11-27 10:25:38 UTC
(In reply to comment #2)
> It fixed in 79db933d667fe0208aaf1fc58f8bd84d6a0dc162 commit.
> I not get more this error with epiphany from master.

That's a different critical warning I fixed. If you don't see the warning it is for a different reason.
Comment 9 Claudio Saavedra 2013-11-27 10:32:43 UTC
Review of attachment 262918 [details] [review]:

::: embed/uri-tester.c
@@ +478,2 @@
   /* TODO: Play with optimization flags */
+  regex = g_regex_new (patt, G_REGEX_OPTIMIZE | G_REGEX_JAVASCRIPT_COMPAT,

Why is this necessary?

@@ -483,3 @@
       g_warning ("%s: %s", G_STRFUNC, error->message);
       g_error_free (error);
-      g_clear_pointer (&regex, g_regex_unref);

If this was already the case, then this cleanup is not related to the bug.
Comment 10 Claudio Saavedra 2013-11-27 10:35:11 UTC
Basically it's two different warnings, so please two patches.
Comment 11 Andres Gomez 2013-11-27 10:40:23 UTC
(In reply to comment #9)
> Review of attachment 262918 [details] [review]:
> 
> ::: embed/uri-tester.c
> @@ +478,2 @@
>    /* TODO: Play with optimization flags */
> +  regex = g_regex_new (patt, G_REGEX_OPTIMIZE | G_REGEX_JAVASCRIPT_COMPAT,
> 
> Why is this necessary?
...

Because without it the compilation fails, as it is shown in the description of
the bug:

(In reply to comment #0)
> ** (WebKitWebProcess:6193): WARNING **: uri_tester_compile_regexp: Error while
> compiling regular expression /cdn-cgi/pe/bag\?r[]=.*cpalead.com at char 34:
> missing terminating ] for character class

With it, all the compilations coming from
https://easylist-downloads.adblockplus.org/easylist.txt succeed.

(In reply to comment #9)
> @@ -483,3 @@
>        g_warning ("%s: %s", G_STRFUNC, error->message);
>        g_error_free (error);
> -      g_clear_pointer (&regex, g_regex_unref);
> 
> If this was already the case, then this cleanup is not related to the bug.

That cleanup spits the CRITICAL shown in the description of the bug:

(In reply to comment #0)
> (WebKitWebProcess:6193): GLib-CRITICAL **: g_regex_unref: assertion `regex !=
> NULL' failed

Since it tries to do a "g_regex_unref" on a NULL pointer, happening because the
compilation fails with the WARNING.
Comment 12 Claudio Saavedra 2013-11-27 10:43:16 UTC
> ...
> 
> Because without it the compilation fails, as it is shown in the description of
> the bug:
> 

Why does it fail without it?

> 
> Since it tries to do a "g_regex_unref" on a NULL pointer, happening because the
> compilation fails with the WARNING.

Yes, but the compilation could fail for many other reasons, and in any such cases the g_regex_unref() call is unnecessary.
Comment 13 Andres Gomez 2013-11-27 10:48:47 UTC
Created attachment 262929 [details] [review]
uri-tester: Compile regex in JavaScript compatibility mode

AddBlock regexes now are compiled in JavaScript compatibility mode to
avoid the errors happening from using the patterns from
https://easylist-downloads.adblockplus.org/easylist.txt .
Comment 14 Andres Gomez 2013-11-27 10:56:10 UTC
Moved the second part of the previous patch to bug 719405.
Comment 15 Claudio Saavedra 2013-11-27 11:12:22 UTC
Review of attachment 262929 [details] [review]:

Okay, I suppose, from reading in adblockplus.org, that the lists compiled are meant to be used from JS, so in that case it makes sense to use a JS-compatibility mode. Please commit to gnome-3-10 too.
Comment 16 Andres Gomez 2013-11-27 11:49:29 UTC
Pushed.
Comment 17 Claudio Saavedra 2014-01-02 20:35:29 UTC
*** Bug 706481 has been marked as a duplicate of this bug. ***