GNOME Bugzilla – Bug 619945
GConverterOutputStream triggers assertion and corrupts data
Last modified: 2010-08-20 18:51:50 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.
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.
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().
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.
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.
Created attachment 162549 [details] [review] Regression test for corruption in GConverterOutputStream
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.
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.
*** Bug 625974 has been marked as a duplicate of this bug. ***