Fix SkGifCodec bugs around truncated data

Prior to this CL, if a GIF file was truncated before reading the local
color map of a frame, incremental decode would do the wrong thing. In
onStartIncrementalDecode, we would either create a color table based on
the global color map, or we would create a dummy one with only one
color (transparent). The dummy color table is correct if there is
neither a global nor a local color map, and allows us to fill the frame
with transparent. But if more data is provided, and it includes an
actual color map and image data, one of the following can happen:
- If the created color table is smaller than the actual one, the
  decoded data may include indices outside of the range of the created
  color table, resulting in a crash.
- If we get lucky, and the created color table is large enough, it may
  still be the wrong colors (and most likely is).

To solve this, make onStartIncrementalDecode fail if there is a local
color map that has not been read yet. A future call may read more data
and read the correct color map.

This is done by returning kIncompleteInput in
SkGifCodec::prepareToDecode if there is a local color map that has not
yet been read. (It is possible that there is no color map at all, in
which case we still need to support decoding that frame. Skip
attempting to decode in that case.)

In onGetPixels, if prepareToDecode returned kIncompleteInput, return
kInvalidInput. Although the input is technically incomplete, no future
call will provide more data (unlike in incremental decoding), and there
is nothing interesting for the client to draw. This also prevents
SkCodec from attempting to fill the data with an SkSwizzler, which has
not been created. (An alternative solution would be create the dummy
color table and an SkSwizzler, which would keep the current behavior.
But I think the new behavior of returning kInvalidInput makes more
sense.)

Add tests to verify the intended behavior:
- getPixels fails.
- startIncrementalDecode fails, but after providing more data it will
  succeed and incremental decoding matches the image decoded from the
  full stream.
- Both succeed if there is no color table at all.

Change-Id: Ifb52fe7f723673406a28e80c8805a552f0ac33b6
Reviewed-on: https://skia-review.googlesource.com/5758
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>

1 files changed, 38 insertions, 4 deletions

diff --git a/tests/GifTest.cpp b/tests/GifTest.cpp
index 17dce92186..12bfb5348f 100644
--- a/tests/GifTest.cpp
+++ b/tests/GifTest.cpp
@@ -26,9 +26,18 @@ static unsigned char gGIFData[] = {
 };
 
 static unsigned char gGIFDataNoColormap[] = {
-  0x47, 0x49, 0x46, 0x38, 0x39, 0x61, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00,
-  0x21, 0xf9, 0x04, 0x01, 0x0a, 0x00, 0x01, 0x00, 0x2c, 0x00, 0x00, 0x00, 0x00,
-  0x01, 0x00, 0x01, 0x00, 0x00, 0x02, 0x02, 0x4c, 0x01, 0x00, 0x3b
+  // Header
+  0x47, 0x49, 0x46, 0x38, 0x39, 0x61,
+  // Screen descriptor
+  0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00,
+  // Graphics control extension
+  0x21, 0xf9, 0x04, 0x01, 0x0a, 0x00, 0x01, 0x00,
+  // Image descriptor
+  0x2c, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00,
+  // Image data
+  0x02, 0x02, 0x4c, 0x01, 0x00,
+  // Trailer
+  0x3b
 };
 
 static unsigned char gInterlacedGIF[] = {
@@ -177,7 +186,32 @@ DEF_TEST(Gif, reporter) {
 
     test_gif_data_no_colormap(reporter, static_cast<void *>(gGIFDataNoColormap),
                               sizeof(gGIFDataNoColormap));
-    // "libgif warning [missing colormap]"
+
+    // Since there is no color map, we do not even need to parse the image data
+    // to know that we should draw transparent. Truncate the file before the
+    // data. This should still succeed.
+    test_gif_data_no_colormap(reporter, static_cast<void *>(gGIFDataNoColormap), 31);
+
+    // Likewise, incremental decoding should succeed here.
+    {
+        sk_sp<SkData> data = SkData::MakeWithoutCopy(gGIFDataNoColormap, 31);
+        std::unique_ptr<SkCodec> codec(SkCodec::NewFromData(data));
+        REPORTER_ASSERT(reporter, codec);
+        if (codec) {
+            auto info = codec->getInfo().makeColorType(kN32_SkColorType);
+            SkBitmap bm;
+            bm.allocPixels(info);
+            REPORTER_ASSERT(reporter, SkCodec::kSuccess == codec->startIncrementalDecode(
+                    info, bm.getPixels(), bm.rowBytes()));
+            REPORTER_ASSERT(reporter, SkCodec::kSuccess == codec->incrementalDecode());
+            REPORTER_ASSERT(reporter, bm.width() == 1);
+            REPORTER_ASSERT(reporter, bm.height() == 1);
+            REPORTER_ASSERT(reporter, !(bm.empty()));
+            if (!(bm.empty())) {
+                REPORTER_ASSERT(reporter, bm.getColor(0, 0) == 0x00000000);
+            }
+        }
+    }
 
     // test short Gif.  80 is missing a few bytes.
     test_gif_data_short(reporter, static_cast<void *>(gGIFData), 80);