More tolerant about malformed GIF files

Source/WebKit/chromium/tests/GIFImageDecoderTest.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 168 matching lines @@
 }
 
 TEST(GIFImageDecoderTest, brokenSecondFrame)
 {
     OwnPtr<GIFImageDecoder> decoder(createDecoder());
 
     RefPtr<SharedBuffer> data = readFile("/Source/WebKit/chromium/tests/data/broken.gif");
     ASSERT_TRUE(data.get());
     decoder->setData(data.get(), true);
 
-    EXPECT_EQ(0u, decoder->frameCount());
-    ImageFrame* frame = decoder->frameBufferAtIndex(0);
+    // One frame is detected but cannot be decoded.
+    EXPECT_EQ(1u, decoder->frameCount());
+    ImageFrame* frame = decoder->frameBufferAtIndex(1);
     EXPECT_FALSE(frame);
-    EXPECT_EQ(cAnimationLoopOnce, decoder->repetitionCount());
 }
 
 TEST(GIFImageDecoderTest, progressiveDecode)
 {
     RefPtr<SharedBuffer> fullData = readFile("/Source/WebKit/chromium/tests/data/radient.gif");
     ASSERT_TRUE(fullData.get());
     const size_t fullLength = fullData->size();
 
     OwnPtr<GIFImageDecoder> decoder;
     ImageFrame* frame;
@@ skipping 87 matching lines @@
     EXPECT_FALSE(decoder->failed());
     EXPECT_TRUE(decoder->frameIsCompleteAtIndex(0));
     EXPECT_FALSE(decoder->frameIsCompleteAtIndex(1));
 
     decoder->setData(data.get(), true);
     EXPECT_EQ(2u, decoder->frameCount());
     EXPECT_TRUE(decoder->frameIsCompleteAtIndex(0));
     EXPECT_TRUE(decoder->frameIsCompleteAtIndex(1));
 }
 
+TEST(GIFImageDecoderTest, badImageSeparator)
+{
+    RefPtr<SharedBuffer> fullData = readFile("/LayoutTests/fast/images/resources/animated.gif");
+    ASSERT_TRUE(fullData.get());
+    const size_t fullLength = fullData->size();
+
+    OwnPtr<GIFImageDecoder> decoder(createDecoder());
+    decoder->setData(fullData.get(), true);
+    EXPECT_EQ(2u, decoder->frameCount());
+    ImageFrame* frame = decoder->frameBufferAtIndex(0);
+    ASSERT_TRUE(frame);
+    EXPECT_EQ(ImageFrame::FrameComplete, frame->status());
+    const unsigned referenceHash = hashSkBitmap(frame->getSkBitmap());
+
+    // Replace the second image separator with an invalid character.
+    RefPtr<SharedBuffer> badData = SharedBuffer::create();
+    int count = 0;
+    for (size_t i = 0; i < fullLength; ++i) {
+        if (fullData->data()[i] == ',') {
+            if (count++ == 1) {

[Peter Kasting, 2013/05/29 04:30:17]

Nit: I think "if (++count == 2)" is slightly clearer, and "++count;  if (count
== 2)" is even better; but,

Better than all this would be if we could do the equivalent of find() within the
SharedBuffer to locate the commas and then append blocks of data at once instead
of going character-by-character; but,

Even better would be to just check a known-bad image into the tree and load that
here, and scrap the algorithm entirely.

[Alpha Left Google, 2013/05/29 19:38:58]

Done. SharedBuffer return const char* so a copy is required. I also try to avoid
adding more data files and reuse existing ones.

[Peter Kasting, 2013/05/29 20:02:53]

Why?  A file to test this could be a few hundred bytes.  Seems much clearer and
more maintainable than code.

After all, we aadd test files to test other bugs all the time.

+                badData->append("=", 1);
+                continue;
+            }
+        }
+        badData->append(fullData->data() + i, 1);
+    }
+
+    decoder = createDecoder();
+    decoder->setData(badData.get(), true);
+    EXPECT_EQ(1u, decoder->frameCount());
+    frame = decoder->frameBufferAtIndex(0);
+    ASSERT_TRUE(frame);
+    EXPECT_EQ(ImageFrame::FrameComplete, frame->status());
+    const unsigned hash = hashSkBitmap(frame->getSkBitmap());
+
+    EXPECT_EQ(referenceHash, hash);
+}
+
 #endif
 
 } // namespace