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 306746 - unreacheable code in css_matcher_apply_rule() reached.
unreacheable code in css_matcher_apply_rule() reached.
Status: RESOLVED FIXED
Product: gtkhtml2
Classification: Deprecated
Component: CSS Parser
2.6.x
Other All
: High critical
: ---
Assigned To: Rodney Dawes
Rodney Dawes
: 315366 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2005-06-07 07:42 UTC by Pawel Salek
Modified: 2005-09-29 13:32 UTC
See Also:
GNOME target: ---
GNOME version: 2.7/2.8


Attachments
test case (4.12 KB, text/html)
2005-06-07 07:43 UTC, Pawel Salek
  Details
Proposed patch (708 bytes, patch)
2005-06-07 11:56 UTC, Pawel Salek
none Details | Review
Same test case attached as text/plain this time (4.12 KB, text/plain)
2005-06-07 12:37 UTC, Pawel Salek
  Details
Patch to fix crash (727 bytes, patch)
2005-06-07 13:41 UTC, Rodney Dawes
committed Details | Review

Description Pawel Salek 2005-06-07 07:42:14 UTC
Steps to reproduce:
1. Load attached document.
2. ... watch the assertion fail.
HtmlCss-ERROR **: file cssmatcher.c: line 1317 (css_matcher_apply_rule): should
not be reached
aborting...


Stack trace:
  • #2 abort
    from /lib/tls/libc.so.6
  • #3 g_logv
    from /usr/lib/libglib-2.0.so.0
  • #4 g_log
    from /usr/lib/libglib-2.0.so.0
  • #5 css_matcher_apply_rule
    from /usr/lib/libgtkhtml-2.so.0
  • #6 css_matcher_get_style
    from /usr/lib/libgtkhtml-2.so.0
  • #7 css_value_ref
    from /usr/lib/libgtkhtml-2.so.0
  • #8 css_value_ref
    from /usr/lib/libgtkhtml-2.so.0
  • #9 g_cclosure_marshal_VOID__OBJECT
    from /usr/lib/libgobject-2.0.so.0
  • #10 g_closure_invoke
    from /usr/lib/libgobject-2.0.so.0
  • #11 g_signal_has_handler_pending
    from /usr/lib/libgobject-2.0.so.0
  • #12 g_signal_emit_valist
    from /usr/lib/libgobject-2.0.so.0
  • #13 g_signal_emit
    from /usr/lib/libgobject-2.0.so.0
  • #14 html_focus_iterator_next_element
    from /usr/lib/libgtkhtml-2.so.0
  • #15 htmlParseEntityRef
    from /usr/lib/libxml2.so.2
  • #16 htmlParseChunk
    from /usr/lib/libxml2.so.2
  • #17 html_parser_get_type
    from /usr/lib/libgtkhtml-2.so.0
  • #18 html_stream_write
    from /usr/lib/libgtkhtml-2.so.0
  • #19 html_document_write_stream
    from /usr/lib/libgtkhtml-2.so.0

Other information:
Comment 1 Pawel Salek 2005-06-07 07:43:15 UTC
Created attachment 47366 [details]
test case

The gtkhtml test program is expected to crash on this file.
Comment 2 Pawel Salek 2005-06-07 11:56:55 UTC
Created attachment 47377 [details] [review]
Proposed patch

There was an incorrect assumption in the code: unknown attributes do not mean
code is incorrect but may mean that supplied document is non-conforming or just
conforms to a newer version of CSS standard. In any case, the code should just
ignore such cases. FWIW, the simplest test document triggering the assertion
is:
<HTML><BODY style="text-align: auto;"></BODY></HTML>
Comment 3 Rodney Dawes 2005-06-07 12:15:54 UTC
The attached test case does not contain any text-align properties, according to
gedit's find functionality, and does not seem to crash my gtkhtml2.

The proposed patch should not use g_message, and should only change the
g_assert_not_reached to g_warning. Why are you changing the other g_warning to a
g_message as well?
Comment 4 Pawel Salek 2005-06-07 12:36:06 UTC
OK, something went wrong with attaching the message because the version I have
on the disk has - but the one attached to the report does not - I see bugzilla
stripped values of all style attributes and converted tags to lower case!!! Very
nasty. Anyway, you should be able to reproduce the problem with the one-line
document mentioned above.

Re: g_message() and g_warning(). I do not insist on using g_message but I
definetely insist on *not* using g_warning(). My understanding is that
g_warning() should report unexpected conditions in the code itself, not report
incorrect - but correctly handled - input, which can be beyond program's
control. One of the properties of g_warning() is its ability to dump the core
when program is run with --g-fatal-warnings. Dumping the core in case of an
predicted and acceptable event (like an unhandled attribute) is too rough
behavior and just makes difficult to debug real problems with the code.
Comment 5 Pawel Salek 2005-06-07 12:37:41 UTC
Created attachment 47380 [details]
Same test case attached as text/plain this time

Attaching the same HTML document as text/plain so that bugzilla does not mess
with it (hopefully).
Comment 6 Rodney Dawes 2005-06-07 13:41:22 UTC
Created attachment 47382 [details] [review]
Patch to fix crash

Here is a modified version of the proposed patch which just gets rid of the
text-align: bottom; block completely in the switch statement, and uses the same
error message for all unhandled alignment types. I'll be committing this to CVS
momentarily, on HEAD.
Comment 7 Rodney Dawes 2005-06-07 13:42:13 UTC
Committed.
Comment 8 Rodney Dawes 2005-09-07 15:53:44 UTC
*** Bug 315366 has been marked as a duplicate of this bug. ***
Comment 9 Pawel Salek 2005-09-29 08:21:20 UTC
Has this gtkhtml2 version been released? I see Fedora development for example
still carries the buggy gtkhtml2-2.6.3.
Comment 10 Rodney Dawes 2005-09-29 13:32:40 UTC
I just released 2.11.0 this past weekend. It contains this fix among several
others. It also supports <meta http-equiv="refresh"...> and <link
rel="shortcut-icon"...>. I don't plan to make any further 2.6 releases, unless
there is some highly compelling reason to backport some of the fixes and make
one (Sun/Redhat/etc... require a 2.6 release) though.