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 131218 - g_logv crashes if argument is non-utf8
g_logv crashes if argument is non-utf8
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2004-01-12 12:03 UTC by Sarfraaz Ahmed
Modified: 2011-02-18 16:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
a patch (1.22 KB, patch)
2004-01-31 01:58 UTC, Matthias Clasen
none Details | Review
a new patch (1.94 KB, patch)
2004-03-04 21:36 UTC, Matthias Clasen
none Details | Review

Description Sarfraaz Ahmed 2004-01-12 12:03:57 UTC
While using evolution 1.5.2 on Gnome 2.4, i am getting a crash as shown in
the trace below ...

(gdb) thread 9
[Switching to thread 9 (Thread 1116949808 (LWP 576))]#0  0xffffe002 in ?? ()
(gdb) bt
  • #0 ??
  • #1 libgnomeui_segv_handle
    at gnome-ui-init.c line 738
  • #2 <signal handler called>
  • #3 ??
  • #4 abort
    from /lib/tls/libc.so.6
  • #5 g_logv
  • #6 g_log
  • #7 g_string_erase
    at gstring.c line 681
  • #8 escape_string
    at gmessages.c line 804
  • #9 g_log_default_handler
    at gmessages.c line 880
  • #10 g_logv
    at gmessages.c line 503
  • #11 g_log
    at gmessages.c line 541
  • #12 camel_mime_filter_charset_new_convert
    at camel-mime-filter-charset.c line 268
  • #13 em_format_format_text
    at em-format.c line 864
  • #14 efh_write_text_html
    at em-format-html.c line 780
  • #15 emfh_getpuri
    at em-format-html.c line 422
  • #16 efh_format_do
    at em-format-html.c line 1205
  • #17 mail_msg_received
    at mail-mt.c line 506
  • #18 thread_received_msg
    at e-msgport.c line 617
  • #19 thread_dispatch
    at e-msgport.c line 698
  • #20 start_thread
    from /lib/tls/libpthread.so.0


I did some analysis on this and found that this occuring because the value
of the variable "length" in g_string_erase is getting miscalulated. But,
this is getting jumbled up since, we give a non-utf-8 character as an input
to g_strdup_vprintf in g_logv.

I think, we need to check and convert the arguments passed to utf-8 before
sending them to g_strdup_vprintf.
Comment 1 Sarfraaz Ahmed 2004-01-12 12:08:00 UTC
What i meant in the last statement above was that ... "we need to
check if the argument is utf-8 and convert it to utf-8 if it is not,
before sending the argument to g_strdup_vprintf"
Comment 2 Matthias Clasen 2004-01-31 01:24:59 UTC
You cannot convert an arbitrary byte string to UTF-8. At best you
could detect if its not valid UTF-8 and display it as hex or octal
escapes.
I guess this is what escape_string should do. 

I think we should do this before 2.4, since it is bad when logging
crashes, and this is new behaviour which got introduced with
escape_string() after 2.2.
Comment 3 Matthias Clasen 2004-01-31 01:58:29 UTC
Created attachment 23915 [details] [review]
a patch
Comment 4 Owen Taylor 2004-01-31 14:35:42 UTC
I don't think the patch is quite right. 

From g_log_default_handler()

      if (g_get_charset (&charset))
        g_string_append (gstring, message);     /* charset is UTF-8
already */
      else
        {
          string = strdup_convert (message, charset);
          g_string_append (gstring, string);
          g_free (string);
        }
 
      escape_string (gstring);

So, we're actually calling escape_string, which expects
UTF-8 *after* we convert to the locale charset!

So, that needs to be cleared up. 

Note also that in the non-UTF-8 case where we
call strdup_convert, we're already checking for
valid UTF-8, and there is code to handle invalid
UTF-8 as escape sequences.
checking for invalid 
Comment 5 Matthias Clasen 2004-03-04 21:36:50 UTC
Created attachment 25175 [details] [review]
a new patch
Comment 6 Matthias Clasen 2004-03-04 21:40:10 UTC
The new patch moves escape_string to before the conversion, so that
the string is still (possibly invalid) UTF-8.
Comment 7 Owen Taylor 2004-03-14 18:59:47 UTC
Applied with a couple of changes:

+ g_utf8_get_char_validated (p, 3);
- g_utf8_get_char_validated (p, -1);

(UTF-8 sequences can be longer than 3, and -1 does the 
right thing for a null-terminated string)

-          tmp = g_strdup_printf ("\\x%02hhx", (guint)*p);
+          tmp = g_strdup_printf ("\\x%02x", (guint)*p);

hh is C99. 

I also changed strdup_convert() to use hex rather than
octal to match.

Sun Mar 14 13:56:48 2004  Owen Taylor  <otaylor@redhat.com>
 
        * glib/gmessages.c (escape_string): Handle invalid
        UTF-8. (#131218, patch from Matthias Clasen)
Comment 8 Mariano Suárez-Alvarez 2004-04-08 08:24:06 UTC
*** Bug 139441 has been marked as a duplicate of this bug. ***
Comment 9 Mariano Suárez-Alvarez 2004-04-08 08:30:12 UTC
Reopening, since bug 139441 has a systematic crash in escape_string when using
the zh_CN.GBK locale.
Comment 10 Owen Taylor 2004-04-08 11:41:36 UTC
Reclosing. Bug 139030 covers the new crash.