diff options
| -rw-r--r-- | src/codec/SkGifCodec.cpp | 9 | ||||
| -rw-r--r-- | tests/CodecPartialTest.cpp | 6 | ||||
| -rw-r--r-- | tests/GifTest.cpp | 12 | ||||
| -rw-r--r-- | third_party/gif/SkGifImageReader.cpp | 8 |
4 files changed, 26 insertions, 9 deletions
diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp index 7fc3c9805a..d13f97ae72 100644 --- a/src/codec/SkGifCodec.cpp +++ b/src/codec/SkGifCodec.cpp @@ -74,14 +74,19 @@ static SkCodec::Result gif_error(const char* msg, SkCodec::Result result = SkCod SkCodec* SkGifCodec::NewFromStream(SkStream* stream) { std::unique_ptr<SkGifImageReader> reader(new SkGifImageReader(stream)); if (!reader->parse(SkGifImageReader::SkGIFSizeQuery)) { - // Not enough data to determine the size. + // Fatal error occurred. return nullptr; } - if (0 == reader->screenWidth() || 0 == reader->screenHeight()) { + // If no images are in the data, or the first header is not yet defined, we cannot + // create a codec. In either case, the width and height are not yet known. + if (0 == reader->imagesCount() || !reader->frameContext(0)->isHeaderDefined()) { return nullptr; } + // isHeaderDefined() will not return true if the screen size is empty. + SkASSERT(reader->screenHeight() > 0 && reader->screenWidth() > 0); + const auto alpha = reader->firstFrameHasAlpha() ? SkEncodedInfo::kBinary_Alpha : SkEncodedInfo::kOpaque_Alpha; // Use kPalette since Gifs are encoded with a color table. diff --git a/tests/CodecPartialTest.cpp b/tests/CodecPartialTest.cpp index ba1a51f4ef..9611d0982e 100644 --- a/tests/CodecPartialTest.cpp +++ b/tests/CodecPartialTest.cpp @@ -90,7 +90,7 @@ private: SkMemoryStream fStream; }; -static void test_partial(skiatest::Reporter* r, const char* name) { +static void test_partial(skiatest::Reporter* r, const char* name, size_t minBytes = 0) { sk_sp<SkData> file = make_from_resource(name); if (!file) { SkDebugf("missing resource %s\n", name); @@ -104,7 +104,7 @@ static void test_partial(skiatest::Reporter* r, const char* name) { } // Now decode part of the file - HaltingStream* stream = new HaltingStream(file, file->size() / 2); + HaltingStream* stream = new HaltingStream(file, SkTMax(file->size() / 2, minBytes)); // Note that we cheat and hold on to a pointer to stream, though it is owned by // partialCodec. @@ -174,7 +174,7 @@ DEF_TEST(Codec_partial, r) { test_partial(r, "baby_tux.png"); test_partial(r, "box.gif"); - test_partial(r, "randPixels.gif"); + test_partial(r, "randPixels.gif", 215); test_partial(r, "color_wheel.gif"); } diff --git a/tests/GifTest.cpp b/tests/GifTest.cpp index b06d3ead1c..17dce92186 100644 --- a/tests/GifTest.cpp +++ b/tests/GifTest.cpp @@ -227,3 +227,15 @@ DEF_TEST(Gif_Sampled, r) { bm.rowBytes(), &options); REPORTER_ASSERT(r, result == SkCodec::kSuccess); } + +// If a GIF file is truncated before the header for the first image is defined, +// we should not create an SkCodec. +DEF_TEST(Codec_GifTruncated, r) { + SkString path = GetResourcePath("test640x479.gif"); + sk_sp<SkData> data(SkData::MakeFromFileName(path.c_str())); + + // This is right before the header for the first image. + data = SkData::MakeSubset(data.get(), 0, 446); + std::unique_ptr<SkCodec> codec(SkCodec::NewFromData(data)); + REPORTER_ASSERT(r, !codec); +} diff --git a/third_party/gif/SkGifImageReader.cpp b/third_party/gif/SkGifImageReader.cpp index 14aa1f17fa..9a14018734 100644 --- a/third_party/gif/SkGifImageReader.cpp +++ b/third_party/gif/SkGifImageReader.cpp @@ -781,6 +781,10 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query) } } + addFrameIfNecessary(); + SkGIFFrameContext* currentFrame = m_frames.back().get(); + currentFrame->setHeaderDefined(); + if (query == SkGIFSizeQuery) { // The decoder needs to stop, so we return here, before // flushing the buffer. Next time through, we'll be in the same @@ -789,10 +793,6 @@ bool SkGifImageReader::parse(SkGifImageReader::SkGIFParseQuery query) return true; } - addFrameIfNecessary(); - SkGIFFrameContext* currentFrame = m_frames.back().get(); - - currentFrame->setHeaderDefined(); currentFrame->setRect(xOffset, yOffset, width, height); currentFrame->setInterlaced(SkToBool(currentComponent[8] & 0x40)); |