aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
-rw-r--r--src/codec/SkCodec.cpp11
-rw-r--r--src/codec/SkCodec_libbmp.cpp6
-rw-r--r--src/codec/SkCodec_libbmp.h1
-rw-r--r--src/codec/SkCodec_libgif.cpp3
-rw-r--r--src/codec/SkCodec_libpng.cpp3
-rw-r--r--src/codec/SkCodec_wbmp.cpp3
-rw-r--r--tests/CodexTest.cpp29
7 files changed, 48 insertions, 8 deletions
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index 5604af8d3d..7bc9146041 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -32,8 +32,10 @@ SkCodec* SkCodec::NewFromStream(SkStream* stream) {
if (!stream) {
return NULL;
}
+
+ SkAutoTDelete<SkStream> streamDeleter(stream);
- SkCodec* codec = NULL;
+ SkAutoTDelete<SkCodec> codec(NULL);
for (uint32_t i = 0; i < SK_ARRAY_COUNT(gDecoderProcs); i++) {
DecoderProc proc = gDecoderProcs[i];
const bool correctFormat = proc.IsFormat(stream);
@@ -41,7 +43,7 @@ SkCodec* SkCodec::NewFromStream(SkStream* stream) {
return NULL;
}
if (correctFormat) {
- codec = proc.NewFromStream(stream);
+ codec.reset(proc.NewFromStream(streamDeleter.detach()));
break;
}
}
@@ -50,12 +52,11 @@ SkCodec* SkCodec::NewFromStream(SkStream* stream) {
// This is about 4x smaller than a test image that takes a few minutes for
// dm to decode and draw.
const int32_t maxSize = 1 << 27;
- if (codec != NULL &&
- codec->getInfo().width() * codec->getInfo().height() > maxSize) {
+ if (codec && codec->getInfo().width() * codec->getInfo().height() > maxSize) {
SkCodecPrintf("Error: Image size too large, cannot decode.\n");
return NULL;
} else {
- return codec;
+ return codec.detach();
}
}
diff --git a/src/codec/SkCodec_libbmp.cpp b/src/codec/SkCodec_libbmp.cpp
index 75c5715bda..2b20faebad 100644
--- a/src/codec/SkCodec_libbmp.cpp
+++ b/src/codec/SkCodec_libbmp.cpp
@@ -108,6 +108,7 @@ SkCodec* SkBmpCodec::NewFromIco(SkStream* stream) {
* Read enough of the stream to initialize the SkBmpCodec. Returns a bool
* representing success or failure. If it returned true, and codecOut was
* not NULL, it will be set to a new SkBmpCodec.
+ * Does *not* take ownership of the passed in SkStream.
*
*/
bool SkBmpCodec::ReadHeader(SkStream* stream, bool isIco, SkCodec** codecOut) {
@@ -496,8 +497,13 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool isIco, SkCodec** codecOut) {
*
*/
SkCodec* SkBmpCodec::NewFromStream(SkStream* stream, bool isIco) {
+ SkAutoTDelete<SkStream> streamDeleter(stream);
SkCodec* codec = NULL;
if (ReadHeader(stream, isIco, &codec)) {
+ // codec has taken ownership of stream, so we do not need to
+ // delete it.
+ SkASSERT(codec);
+ streamDeleter.detach();
return codec;
}
return NULL;
diff --git a/src/codec/SkCodec_libbmp.h b/src/codec/SkCodec_libbmp.h
index 650259ad16..81b514e8f5 100644
--- a/src/codec/SkCodec_libbmp.h
+++ b/src/codec/SkCodec_libbmp.h
@@ -103,6 +103,7 @@ private:
* Read enough of the stream to initialize the SkBmpCodec. Returns a bool
* representing success or failure. If it returned true, and codecOut was
* not NULL, it will be set to a new SkBmpCodec.
+ * Does *not* take ownership of the passed in SkStream.
*
*/
static bool ReadHeader(SkStream*, bool isIco, SkCodec** codecOut);
diff --git a/src/codec/SkCodec_libgif.cpp b/src/codec/SkCodec_libgif.cpp
index 2a1d81fc00..9356a6973b 100644
--- a/src/codec/SkCodec_libgif.cpp
+++ b/src/codec/SkCodec_libgif.cpp
@@ -136,6 +136,7 @@ static uint32_t find_trans_index(const SavedImage& image) {
* Reads enough of the stream to determine the image format
*/
SkCodec* SkGifCodec::NewFromStream(SkStream* stream) {
+ SkAutoTDelete<SkStream> streamDeleter(stream);
// Read gif header, logical screen descriptor, and global color table
SkAutoTCallIProc<GifFileType, CloseGif> gif(open_gif(stream));
@@ -165,7 +166,7 @@ SkCodec* SkGifCodec::NewFromStream(SkStream* stream) {
// use kPremul directly even when kUnpremul is supported.
const SkImageInfo& imageInfo = SkImageInfo::Make(width, height,
kIndex_8_SkColorType, kPremul_SkAlphaType);
- return SkNEW_ARGS(SkGifCodec, (imageInfo, stream, gif.detach()));
+ return SkNEW_ARGS(SkGifCodec, (imageInfo, streamDeleter.detach(), gif.detach()));
}
SkGifCodec::SkGifCodec(const SkImageInfo& srcInfo, SkStream* stream,
diff --git a/src/codec/SkCodec_libpng.cpp b/src/codec/SkCodec_libpng.cpp
index 33111cee67..121b74ff0b 100644
--- a/src/codec/SkCodec_libpng.cpp
+++ b/src/codec/SkCodec_libpng.cpp
@@ -346,11 +346,12 @@ static bool read_header(SkStream* stream, png_structp* png_ptrp,
}
SkCodec* SkPngCodec::NewFromStream(SkStream* stream) {
+ SkAutoTDelete<SkStream> streamDeleter(stream);
png_structp png_ptr;
png_infop info_ptr;
SkImageInfo imageInfo;
if (read_header(stream, &png_ptr, &info_ptr, &imageInfo)) {
- return SkNEW_ARGS(SkPngCodec, (imageInfo, stream, png_ptr, info_ptr));
+ return SkNEW_ARGS(SkPngCodec, (imageInfo, streamDeleter.detach(), png_ptr, info_ptr));
}
return NULL;
}
diff --git a/src/codec/SkCodec_wbmp.cpp b/src/codec/SkCodec_wbmp.cpp
index 465c76d4dc..073165d2ca 100644
--- a/src/codec/SkCodec_wbmp.cpp
+++ b/src/codec/SkCodec_wbmp.cpp
@@ -154,6 +154,7 @@ bool SkWbmpCodec::IsWbmp(SkStream* stream) {
}
SkCodec* SkWbmpCodec::NewFromStream(SkStream* stream) {
+ SkAutoTDelete<SkStream> streamDeleter(stream);
SkISize size;
if (!read_header(stream, &size)) {
return NULL;
@@ -161,5 +162,5 @@ SkCodec* SkWbmpCodec::NewFromStream(SkStream* stream) {
SkImageInfo info =
SkImageInfo::Make(size.width(), size.height(), kGray_8_SkColorType,
kOpaque_SkAlphaType);
- return SkNEW_ARGS(SkWbmpCodec, (info, stream));
+ return SkNEW_ARGS(SkWbmpCodec, (info, streamDeleter.detach()));
}
diff --git a/tests/CodexTest.cpp b/tests/CodexTest.cpp
index 825251854d..26846a4550 100644
--- a/tests/CodexTest.cpp
+++ b/tests/CodexTest.cpp
@@ -109,3 +109,32 @@ DEF_TEST(Codec, r) {
check(r, "randPixels.png", SkISize::Make(8, 8), true);
check(r, "yellow_rose.png", SkISize::Make(400, 301), true);
}
+
+static void test_invalid_stream(skiatest::Reporter* r, const void* stream, size_t len) {
+ SkCodec* codec = SkCodec::NewFromStream(new SkMemoryStream(stream, len, false));
+ // We should not have gotten a codec. Bots should catch us if we leaked anything.
+ REPORTER_ASSERT(r, !codec);
+}
+
+// Ensure that SkCodec::NewFromStream handles freeing the passed in SkStream,
+// even on failure. Test some bad streams.
+DEF_TEST(Codec_leaks, r) {
+ // No codec should claim this as their format, so this tests SkCodec::NewFromStream.
+ const char nonSupportedStream[] = "hello world";
+ // The other strings should look like the beginning of a file type, so we'll call some
+ // internal version of NewFromStream, which must also delete the stream on failure.
+ const unsigned char emptyPng[] = { 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a };
+ const unsigned char emptyJpeg[] = { 0xFF, 0xD8, 0xFF };
+ const char emptyWebp[] = "RIFF1234WEBPVP";
+ const char emptyBmp[] = { 'B', 'M' };
+ const char emptyIco[] = { '\x00', '\x00', '\x01', '\x00' };
+ const char emptyGif[] = "GIFVER";
+
+ test_invalid_stream(r, nonSupportedStream, sizeof(nonSupportedStream));
+ test_invalid_stream(r, emptyPng, sizeof(emptyPng));
+ test_invalid_stream(r, emptyJpeg, sizeof(emptyJpeg));
+ test_invalid_stream(r, emptyWebp, sizeof(emptyWebp));
+ test_invalid_stream(r, emptyBmp, sizeof(emptyBmp));
+ test_invalid_stream(r, emptyIco, sizeof(emptyIco));
+ test_invalid_stream(r, emptyGif, sizeof(emptyGif));
+}