GNOME Bugzilla – Bug 556066
Last byte of FLAC image buffer chopped off
Last modified: 2008-10-14 15:16:37 UTC
``gst_tag_image_data_to_image_buffer()`` tries to do some sort of magic with NULL termination bytes and data lengths, but it's reporting the image's buffer to be one byte too small. This is because when the buffer is allocated with image_data_len + 1 bytes, its size is reported as image_data_len. When it's resized later on line 437, it's resized to image_data_len - 1 bytes.
Created attachment 120479 [details] [review] Set GST_BUFFER_SIZE(image) to image_data_len In this patch, instead of trying to re-calculate the size, simply set it to the original size.
You're right of course... I've committed a slightly different fix to CVS now. GST_BUFFER_SIZE (image) should be set to "image_data_len + 1" in line 413, otherwise the NULL termination doesn't make sense anyway. 2008-10-13 Sebastian Dröge <sebastian.droege@collabora.co.uk> * gst-libs/gst/tag/tags.c: (gst_tag_image_data_to_image_buffer): Don't drop the last byte of image tags if they're not an URI list. Fixes bug #556066.
Sebastian: I don't think that patch is correct. The size of the buffer is already set to image_data_len + 1 when the buffer is allocated. Keeping it at image_data_len + 1 bytes will cause errors because a trailing NULL byte will be appended to the image data and cause image parsers to fail. My patch preserves the NULL byte, **and** returns the correct data length of binary data.
Hm, I don't get it, sorry :) First we allocate the buffer with image_data_len + 1, set the last, additional byte to \0 and then typefind. Afterwards we look at the typefinding result and unless it's an URI list we make the buffer size one smaller (resulting in the \0 being outside of the buffer). Before the problem was, that we allocate the buffer with image_data_len + 1 bytes, set the buffer size immediately to image_data_len and pass this to typefinding and later make the buffer size one less if we don't have an URI list. This then results in image_data_len as size of URI lists and image_data_len-1 for others, i.e. one byte less than it should be for everything. Am I missing something?
Apologies, I misread your comment and thought you had set the buffer size to "image_data_len + 1" if it was not a URI list. Your patch is correct. That should teach me to make bug comments before updating my checkout.