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 619945 - GConverterOutputStream triggers assertion and corrupts data
GConverterOutputStream triggers assertion and corrupts data
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
unspecified
Other Linux
: Normal major
: ---
Assigned To: gtkdev
gtkdev
: 625974 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2010-05-28 14:09 UTC by Jürg Billeter
Modified: 2010-08-20 18:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test case (1.80 KB, text/plain)
2010-05-28 14:09 UTC, Jürg Billeter
  Details
Proposed patch for first issue (1.54 KB, patch)
2010-05-28 14:10 UTC, Jürg Billeter
none Details | Review
Proposed patch for second issue (1.54 KB, patch)
2010-05-28 14:11 UTC, Jürg Billeter
none Details | Review
Regression test for corruption in GConverterOutputStream (2.64 KB, patch)
2010-06-02 15:15 UTC, Christian Dywan
none Details | Review

Description Jürg Billeter 2010-05-28 14:09:07 UTC
Created attachment 162222 [details]
Test case

The loop in g_converter_output_stream_write and g_converter_output_stream_flush triggers an assertion in GConverter and outputs corrupted data. The attached test case demonstrates the issue.
Comment 1 Jürg Billeter 2010-05-28 14:10:21 UTC
Created attachment 162223 [details] [review]
Proposed patch for first issue

Ensure we always have some target space in GConverterOutputStream

When the converter fills the whole buffer without reading all input, we need to enlarge the buffer. Otherwise we get an assertion failure for `outbuf_size > 0' in g_converter_convert.
Comment 2 Jürg Billeter 2010-05-28 14:11:55 UTC
Created attachment 162224 [details] [review]
Proposed patch for second issue

Use correct offset for buffer data in GConverterOutputStream

Otherwise we overwrite already converted data in the case that more than one call to g_converter_convert is necessary in write() or flush().
Comment 3 Christian Dywan 2010-06-02 12:52:45 UTC
The patches look reasonable to me. I also tried the test case and can confirm that the patches work as expected, with glib master.

Probably a good condidate for 2.24 as well.
Comment 4 Matthias Clasen 2010-06-02 13:13:05 UTC
Patches look reasonable to me as well, though I am not that familiar with the converter streams. It would be good to convert the testcase into a regression test in gio/tests/converter-stream.c.
Comment 5 Christian Dywan 2010-06-02 15:15:50 UTC
Created attachment 162549 [details] [review]
Regression test for corruption in GConverterOutputStream
Comment 6 Alexander Larsson 2010-06-10 11:59:22 UTC
I don't have time to look at this right now, but it seems like the 1st patch is not quite right. Rather it seems the problem is a confusion about what buffer_available() means as it is used in buffer_ensure_space(). We already call buffer_ensure_space() to allocate space, but its not quite working right.
Comment 7 Alexander Larsson 2010-06-11 08:07:11 UTC
No, it was I that was confused.
Pushed all three patches to master and cherry picked to glib-2-24.
I also did some renaming and added some comments on master to make this code a bit easier to read.
Comment 8 Jürg Billeter 2010-08-20 18:51:50 UTC
*** Bug 625974 has been marked as a duplicate of this bug. ***