[gdk-pixbuf/wip/run-gif-tests-again: 1/2] gif: Do all of gif_init() with a single read

[Simon McVittie <smcv src gnome org>]

commit bf13fc4c0d4ba9ced6015e825e58cde5370fe323
Author: Simon McVittie <smcv debian org>
Date:   Sat Dec 12 17:32:08 2020 +0000

    gif: Do all of gif_init() with a single read
    
    As documented in the introductory comment, the interface of the various
    functions in the GIF loader is that they read all the bytes they need,
    or return -1 if not enough are available. Since commit
    5212d69f "gif: Replace old buffer management code with GByteArray",
    the incremental loader strictly depends on that assumption.
    
    Unfortunately, gif_init() didn't conform to that interface. If there
    were enough bytes available for the GIF signature (GIF87a or GIF89a)
    but not enough bytes for the screen descriptor, it would return -1
    having already consumed the first 6 bytes of the stream. A subsequent
    retry with more data available would start from the beginning of the
    screen descriptor, and immediately raise an error because the screen
    descriptor is extremely unlikely to start with another copy of the
    "GIF8" magic number.
    
    The regression test tests/pixbuf-short-gif-write.c would have detected
    this, but was accidentally disabled by commit 7f0b214a "tests:
    Conditionally build and run tests".
    
    This seems most easily fixed by reading the whole of the 13-byte
    fixed-length header in one go. Adjust the offsets into the buffer
    used to parse the screen descriptor accordingly.
    
    Signed-off-by: Simon McVittie <smcv debian org>

 gdk-pixbuf/io-gif.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)
---
diff --git a/gdk-pixbuf/io-gif.c b/gdk-pixbuf/io-gif.c
index 57e72d88b..64a492a58 100644
--- a/gdk-pixbuf/io-gif.c
+++ b/gdk-pixbuf/io-gif.c
@@ -513,15 +513,22 @@ gif_prepare_lzw (GifContext *context)
        return 0;
 }
 
-/* needs 13 bytes to proceed. */
+/*
+ * Read the GIF signature and screen descriptor.
+ *
+ * | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9  | 10     | 11 | 12 |
+ * |-----------|-----------|-----------------------------------|
+ * | magic     | version   | screen descriptor                 |
+ * | G | I | F | 8 | 9 | a | width | height | colors | ignored |
+ */
 static gint
 gif_init (GifContext *context)
 {
-       unsigned char buf[16];
+       unsigned char buf[13];
        char version[4];
         gint width, height;
 
-       if (!gif_read (context, buf, 6)) {
+       if (!gif_read (context, buf, 13)) {
                /* Unable to read magic number,
                  * gif_read() should have set error
                  */
@@ -554,22 +561,16 @@ gif_init (GifContext *context)
                return -2;
        }
 
-       /* read the screen descriptor */
-       if (!gif_read (context, buf, 7)) {
-               /* Failed to read screen descriptor, error set */
-               return -1;
-       }
-
-       context->width = LM_to_uint (buf[0], buf[1]);
-       context->height = LM_to_uint (buf[2], buf[3]);
+       context->width = LM_to_uint (buf[6], buf[7]);
+       context->height = LM_to_uint (buf[8], buf[9]);
         /* The 4th byte is
          * high bit: whether to use the background index
          * next 3:   color resolution
          * next:     whether colormap is sorted by priority of allocation
          * last 3:   size of colormap
          */
-       context->global_bit_pixel = 2 << (buf[4] & 0x07);
-        context->has_global_cmap = (buf[4] & 0x80) != 0;
+       context->global_bit_pixel = 2 << (buf[10] & 0x07);
+        context->has_global_cmap = (buf[10] & 0x80) != 0;
 
         context->animation->width = context->width;
         context->animation->height = context->height;