Make SkPngCodec only read as much of the stream as necessary
Previously, SkPngCodec assumed that the stream only contained one
image, which ended at the end of the stream. It read the stream in
arbitrarily-sized chunks, and then passed that data to libpng for
processing.
If a stream contains more than one image, this may result in reading
beyond the end of the image, making future reads read the wrong data.
Now, SkPngCodec starts by reading 8 bytes at a time. After the
signature, 8 bytes is enough to know which chunk is next and how many
bytes are in the chunk.
When decoding the size, we stop when we reach IDAT, and when decoding
the image, we stop when we reach IEND.
This manual parsing is necessary to support APNG, which is planned in
the future. It also allows us to remove the SK_GOOGLE3_PNG_HACK, which
was a workaround for reading more than necessary at the beginning of
the image.
Add a test that simulates the issue, by decoding a special stream that
reports an error if the codec attempts to read beyond the end.
Temporarily disable the partial decoding tests for png. A larger change
will be necessary to get those working again, and no clients are
currently relying on incrementally decoding PNGs (i.e. decode part of
an image, then decode further with more data).
Bug: skia:5368
BUG:34073812
Change-Id: If832f7b20565411226fb5be3c305a4d16bf9269d
Reviewed-on: https://skia-review.googlesource.com/13900
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
diff --git a/gn/tests.gni b/gn/tests.gni
index 5955a32..ffd8b41 100644
--- a/gn/tests.gni
+++ b/gn/tests.gni
@@ -34,6 +34,7 @@
"$_tests/ClipperTest.cpp",
"$_tests/ClipStackTest.cpp",
"$_tests/CodecAnimTest.cpp",
+ "$_tests/CodecExactReadTest.cpp",
"$_tests/CodecPartialTest.cpp",
"$_tests/CodecTest.cpp",
"$_tests/ColorFilterTest.cpp",
diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp
index 8bab368..5e05606 100644
--- a/src/codec/SkPngCodec.cpp
+++ b/src/codec/SkPngCodec.cpp
@@ -22,17 +22,13 @@
#include "SkUtils.h"
#include "png.h"
+#include <algorithm>
// This warning triggers false postives way too often in here.
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic ignored "-Wclobbered"
#endif
-#if PNG_LIBPNG_VER_MAJOR > 1 || (PNG_LIBPNG_VER_MAJOR == 1 && PNG_LIBPNG_VER_MINOR >= 5)
- // This is not needed with version 1.5
- #undef SK_GOOGLE3_PNG_HACK
-#endif
-
// FIXME (scroggo): We can use png_jumpbuf directly once Google3 is on 1.6
#define PNG_JMPBUF(x) png_jmpbuf((png_structp) x)
@@ -80,8 +76,6 @@
SkCodec** codecPtr)
: fPng_ptr(png_ptr)
, fInfo_ptr(nullptr)
- , fDecodedBounds(false)
- , fReadHeader(false)
, fStream(stream)
, fChunkReader(reader)
, fOutCodec(codecPtr)
@@ -117,28 +111,12 @@
private:
png_structp fPng_ptr;
png_infop fInfo_ptr;
- bool fDecodedBounds;
- bool fReadHeader;
SkStream* fStream;
SkPngChunkReader* fChunkReader;
SkCodec** fOutCodec;
- /**
- * Supplied to libpng to call when it has read enough data to determine
- * bounds.
- */
- static void InfoCallback(png_structp png_ptr, png_infop) {
- // png_get_progressive_ptr returns the pointer we set on the png_ptr with
- // png_set_progressive_read_fn
- static_cast<AutoCleanPng*>(png_get_progressive_ptr(png_ptr))->infoCallback();
- }
+ void infoCallback(size_t idatLength);
- void infoCallback();
-
-#ifdef SK_GOOGLE3_PNG_HACK
-// public so it can be called by SkPngCodec::rereadHeaderIfNecessary().
-public:
-#endif
void releasePngPtrs() {
fPng_ptr = nullptr;
fInfo_ptr = nullptr;
@@ -146,12 +124,29 @@
};
#define AutoCleanPng(...) SK_REQUIRE_LOCAL_VAR(AutoCleanPng)
+static inline bool is_chunk(const png_byte* chunk, const char* tag) {
+ return memcmp(chunk + 4, tag, 4) == 0;
+}
+
+static inline bool process_data(png_structp png_ptr, png_infop info_ptr,
+ SkStream* stream, void* buffer, size_t bufferSize, size_t length) {
+ while (length > 0) {
+ const size_t bytesToProcess = std::min(bufferSize, length);
+ if (stream->read(buffer, bytesToProcess) < bytesToProcess) {
+ return false;
+ }
+ png_process_data(png_ptr, info_ptr, (png_bytep) buffer, bytesToProcess);
+ length -= bytesToProcess;
+ }
+ return true;
+}
+
bool AutoCleanPng::decodeBounds() {
if (setjmp(PNG_JMPBUF(fPng_ptr))) {
return false;
}
- png_set_progressive_read_fn(fPng_ptr, this, InfoCallback, nullptr, nullptr);
+ png_set_progressive_read_fn(fPng_ptr, nullptr, nullptr, nullptr, nullptr);
// Arbitrary buffer size, though note that it matches (below)
// SkPngCodec::processData(). FIXME: Can we better suit this to the size of
@@ -159,22 +154,39 @@
constexpr size_t kBufferSize = 4096;
char buffer[kBufferSize];
+ {
+ // Parse the signature.
+ if (fStream->read(buffer, 8) < 8) {
+ return false;
+ }
+
+ png_process_data(fPng_ptr, fInfo_ptr, (png_bytep) buffer, 8);
+ }
+
while (true) {
- const size_t bytesRead = fStream->read(buffer, kBufferSize);
- if (!bytesRead) {
+ // Parse chunk length and type.
+ if (fStream->read(buffer, 8) < 8) {
// We have read to the end of the input without decoding bounds.
break;
}
- png_process_data(fPng_ptr, fInfo_ptr, (png_bytep) buffer, bytesRead);
- if (fReadHeader) {
- break;
+ png_byte* chunk = reinterpret_cast<png_byte*>(buffer);
+ png_process_data(fPng_ptr, fInfo_ptr, chunk, 8);
+
+ // Include the full chunk + CRC.
+ const size_t length = png_get_uint_32(chunk) + 4;
+
+ if (is_chunk(chunk, "IDAT")) {
+ this->infoCallback(length);
+ return true;
+ }
+
+ if (!process_data(fPng_ptr, fInfo_ptr, fStream, buffer, kBufferSize, length)) {
+ return false;
}
}
- // For safety, clear the pointer to this object.
- png_set_progressive_read_fn(fPng_ptr, nullptr, nullptr, nullptr, nullptr);
- return fDecodedBounds;
+ return false;
}
void SkPngCodec::processData() {
@@ -198,16 +210,30 @@
constexpr size_t kBufferSize = 4096;
char buffer[kBufferSize];
+ bool iend = false;
while (true) {
- const size_t bytesRead = this->stream()->read(buffer, kBufferSize);
- png_process_data(fPng_ptr, fInfo_ptr, (png_bytep) buffer, bytesRead);
+ size_t length;
+ if (fDecodedIdat) {
+ // Parse chunk length and type.
+ if (this->stream()->read(buffer, 8) < 8) {
+ break;
+ }
- if (!bytesRead) {
- // We have read to the end of the input. Note that we quit *after*
- // calling png_process_data, because decodeBounds may have told
- // libpng to save the remainder of the buffer, in which case
- // png_process_data will process the saved buffer, though the
- // stream has no more to read.
+ png_byte* chunk = reinterpret_cast<png_byte*>(buffer);
+ png_process_data(fPng_ptr, fInfo_ptr, chunk, 8);
+ if (is_chunk(chunk, "IEND")) {
+ iend = true;
+ }
+
+ // Include the full chunk + CRC.
+ length = png_get_uint_32(chunk) + 4;
+ } else {
+ length = fIdatLength;
+ fDecodedIdat = true;
+ }
+
+ if (!process_data(fPng_ptr, fInfo_ptr, this->stream(), buffer, kBufferSize, length)
+ || iend) {
break;
}
}
@@ -485,12 +511,6 @@
GetDecoder(png_ptr)->rowCallback(row, rowNum);
}
-#ifdef SK_GOOGLE3_PNG_HACK
- static void RereadInfoCallback(png_structp png_ptr, png_infop) {
- GetDecoder(png_ptr)->rereadInfoCallback();
- }
-#endif
-
private:
int fRowsWrittenToOutput;
void* fDst;
@@ -509,11 +529,7 @@
Result decodeAllRows(void* dst, size_t rowBytes, int* rowsDecoded) override {
const int height = this->getInfo().height();
- png_progressive_info_ptr callback = nullptr;
-#ifdef SK_GOOGLE3_PNG_HACK
- callback = RereadInfoCallback;
-#endif
- png_set_progressive_read_fn(this->png_ptr(), this, callback, AllRowsCallback, nullptr);
+ png_set_progressive_read_fn(this->png_ptr(), this, nullptr, AllRowsCallback, nullptr);
fDst = dst;
fRowBytes = rowBytes;
@@ -542,11 +558,7 @@
}
void setRange(int firstRow, int lastRow, void* dst, size_t rowBytes) override {
- png_progressive_info_ptr callback = nullptr;
-#ifdef SK_GOOGLE3_PNG_HACK
- callback = RereadInfoCallback;
-#endif
- png_set_progressive_read_fn(this->png_ptr(), this, callback, RowCallback, nullptr);
+ png_set_progressive_read_fn(this->png_ptr(), this, nullptr, RowCallback, nullptr);
fFirstRow = firstRow;
fLastRow = lastRow;
fDst = dst;
@@ -615,12 +627,6 @@
decoder->interlacedRowCallback(row, rowNum, pass);
}
-#ifdef SK_GOOGLE3_PNG_HACK
- static void RereadInfoInterlacedCallback(png_structp png_ptr, png_infop) {
- static_cast<SkPngInterlacedDecoder*>(png_get_progressive_ptr(png_ptr))->rereadInfoInterlaced();
- }
-#endif
-
private:
const int fNumberPasses;
int fFirstRow;
@@ -634,14 +640,6 @@
typedef SkPngCodec INHERITED;
-#ifdef SK_GOOGLE3_PNG_HACK
- void rereadInfoInterlaced() {
- this->rereadInfoCallback();
- // Note: This allocates more memory than necessary, if we are sampling/subset.
- this->setUpInterlaceBuffer(this->getInfo().height());
- }
-#endif
-
// FIXME: Currently sharing interlaced callback for all rows and subset. It's not
// as expensive as the subset version of non-interlaced, but it still does extra
// work.
@@ -675,11 +673,7 @@
SkCodec::Result decodeAllRows(void* dst, size_t rowBytes, int* rowsDecoded) override {
const int height = this->getInfo().height();
this->setUpInterlaceBuffer(height);
- png_progressive_info_ptr callback = nullptr;
-#ifdef SK_GOOGLE3_PNG_HACK
- callback = RereadInfoInterlacedCallback;
-#endif
- png_set_progressive_read_fn(this->png_ptr(), this, callback, InterlacedRowCallback,
+ png_set_progressive_read_fn(this->png_ptr(), this, nullptr, InterlacedRowCallback,
nullptr);
fFirstRow = 0;
@@ -709,11 +703,7 @@
void setRange(int firstRow, int lastRow, void* dst, size_t rowBytes) override {
// FIXME: We could skip rows in the interlace buffer that we won't put in the output.
this->setUpInterlaceBuffer(lastRow - firstRow + 1);
- png_progressive_info_ptr callback = nullptr;
-#ifdef SK_GOOGLE3_PNG_HACK
- callback = RereadInfoInterlacedCallback;
-#endif
- png_set_progressive_read_fn(this->png_ptr(), this, callback, InterlacedRowCallback, nullptr);
+ png_set_progressive_read_fn(this->png_ptr(), this, nullptr, InterlacedRowCallback, nullptr);
fFirstRow = firstRow;
fLastRow = lastRow;
fDst = dst;
@@ -767,54 +757,6 @@
}
};
-#ifdef SK_GOOGLE3_PNG_HACK
-bool SkPngCodec::rereadHeaderIfNecessary() {
- if (!fNeedsToRereadHeader) {
- return true;
- }
-
- // On the first call, we'll need to rewind ourselves. Future calls will
- // have already rewound in rewindIfNecessary.
- if (this->stream()->getPosition() > 0) {
- this->stream()->rewind();
- }
-
- this->destroyReadStruct();
- png_structp png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, nullptr,
- sk_error_fn, sk_warning_fn);
- if (!png_ptr) {
- return false;
- }
-
- // Only use the AutoCleanPng to delete png_ptr as necessary.
- // (i.e. not for reading bounds etc.)
- AutoCleanPng autoClean(png_ptr, nullptr, nullptr, nullptr);
-
- png_infop info_ptr = png_create_info_struct(png_ptr);
- if (info_ptr == nullptr) {
- return false;
- }
-
- autoClean.setInfoPtr(info_ptr);
-
-#ifdef PNG_READ_UNKNOWN_CHUNKS_SUPPORTED
- // Hookup our chunkReader so we can see any user-chunks the caller may be interested in.
- // This needs to be installed before we read the png header. Android may store ninepatch
- // chunks in the header.
- if (fPngChunkReader.get()) {
- png_set_keep_unknown_chunks(png_ptr, PNG_HANDLE_CHUNK_ALWAYS, (png_byte*)"", 0);
- png_set_read_user_chunk_fn(png_ptr, (png_voidp) fPngChunkReader.get(), sk_read_user_chunk);
- }
-#endif
-
- fPng_ptr = png_ptr;
- fInfo_ptr = info_ptr;
- autoClean.releasePngPtrs();
- fNeedsToRereadHeader = false;
- return true;
-}
-#endif // SK_GOOGLE3_PNG_HACK
-
// Reads the header and initializes the output fields, if not NULL.
//
// @param stream Input data. Will be read to get enough information to properly
@@ -886,21 +828,17 @@
return true;
}
-// FIXME (scroggo): Once SK_GOOGLE3_PNG_HACK is no more, this method can be inline in
-// AutoCleanPng::infoCallback
-static void general_info_callback(png_structp png_ptr, png_infop info_ptr,
- SkEncodedInfo::Color* outColor, SkEncodedInfo::Alpha* outAlpha,
- int* outBitDepth) {
+void AutoCleanPng::infoCallback(size_t idatLength) {
png_uint_32 origWidth, origHeight;
int bitDepth, encodedColorType;
- png_get_IHDR(png_ptr, info_ptr, &origWidth, &origHeight, &bitDepth,
+ png_get_IHDR(fPng_ptr, fInfo_ptr, &origWidth, &origHeight, &bitDepth,
&encodedColorType, nullptr, nullptr, nullptr);
// TODO: Should we support 16-bits of precision for gray images?
if (bitDepth == 16 && (PNG_COLOR_TYPE_GRAY == encodedColorType ||
PNG_COLOR_TYPE_GRAY_ALPHA == encodedColorType)) {
bitDepth = 8;
- png_set_strip_16(png_ptr);
+ png_set_strip_16(fPng_ptr);
}
// Now determine the default colorType and alphaType and set the required transforms.
@@ -915,18 +853,18 @@
if (bitDepth < 8) {
// TODO: Should we use SkSwizzler here?
bitDepth = 8;
- png_set_packing(png_ptr);
+ png_set_packing(fPng_ptr);
}
color = SkEncodedInfo::kPalette_Color;
// Set the alpha depending on if a transparency chunk exists.
- alpha = png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS) ?
+ alpha = png_get_valid(fPng_ptr, fInfo_ptr, PNG_INFO_tRNS) ?
SkEncodedInfo::kUnpremul_Alpha : SkEncodedInfo::kOpaque_Alpha;
break;
case PNG_COLOR_TYPE_RGB:
- if (png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS)) {
+ if (png_get_valid(fPng_ptr, fInfo_ptr, PNG_INFO_tRNS)) {
// Convert to RGBA if transparency chunk exists.
- png_set_tRNS_to_alpha(png_ptr);
+ png_set_tRNS_to_alpha(fPng_ptr);
color = SkEncodedInfo::kRGBA_Color;
alpha = SkEncodedInfo::kBinary_Alpha;
} else {
@@ -939,11 +877,11 @@
if (bitDepth < 8) {
// TODO: Should we use SkSwizzler here?
bitDepth = 8;
- png_set_expand_gray_1_2_4_to_8(png_ptr);
+ png_set_expand_gray_1_2_4_to_8(fPng_ptr);
}
- if (png_get_valid(png_ptr, info_ptr, PNG_INFO_tRNS)) {
- png_set_tRNS_to_alpha(png_ptr);
+ if (png_get_valid(fPng_ptr, fInfo_ptr, PNG_INFO_tRNS)) {
+ png_set_tRNS_to_alpha(fPng_ptr);
color = SkEncodedInfo::kGrayAlpha_Color;
alpha = SkEncodedInfo::kBinary_Alpha;
} else {
@@ -965,43 +903,9 @@
color = SkEncodedInfo::kRGBA_Color;
alpha = SkEncodedInfo::kUnpremul_Alpha;
}
- if (outColor) {
- *outColor = color;
- }
- if (outAlpha) {
- *outAlpha = alpha;
- }
- if (outBitDepth) {
- *outBitDepth = bitDepth;
- }
-}
-
-#ifdef SK_GOOGLE3_PNG_HACK
-void SkPngCodec::rereadInfoCallback() {
- general_info_callback(fPng_ptr, fInfo_ptr, nullptr, nullptr, nullptr);
- png_set_interlace_handling(fPng_ptr);
- png_read_update_info(fPng_ptr, fInfo_ptr);
-}
-#endif
-
-void AutoCleanPng::infoCallback() {
- SkEncodedInfo::Color color;
- SkEncodedInfo::Alpha alpha;
- int bitDepth;
- general_info_callback(fPng_ptr, fInfo_ptr, &color, &alpha, &bitDepth);
const int numberPasses = png_set_interlace_handling(fPng_ptr);
- fReadHeader = true;
- fDecodedBounds = true;
-#ifndef SK_GOOGLE3_PNG_HACK
- // 1 tells libpng to save any extra data. We may be able to be more efficient by saving
- // it ourselves.
- png_process_data_pause(fPng_ptr, 1);
-#else
- // Hack to make png_process_data stop.
- fPng_ptr->buffer_size = 0;
-#endif
if (fOutCodec) {
SkASSERT(nullptr == *fOutCodec);
SkColorSpace_Base::ICCTypeFlag iccType = SkColorSpace_Base::kRGB_ICCTypeFlag;
@@ -1016,11 +920,6 @@
}
SkEncodedInfo encodedInfo = SkEncodedInfo::Make(color, alpha, bitDepth);
- // FIXME (scroggo): Once we get rid of SK_GOOGLE3_PNG_HACK, general_info_callback can
- // be inlined, so these values will already be set.
- png_uint_32 origWidth = png_get_image_width(fPng_ptr, fInfo_ptr);
- png_uint_32 origHeight = png_get_image_height(fPng_ptr, fInfo_ptr);
- png_byte bitDepth = png_get_bit_depth(fPng_ptr, fInfo_ptr);
SkImageInfo imageInfo = encodedInfo.makeImageInfo(origWidth, origHeight, colorSpace);
if (SkEncodedInfo::kOpaque_Alpha == alpha) {
@@ -1041,9 +940,9 @@
fChunkReader, fPng_ptr, fInfo_ptr, bitDepth, numberPasses);
}
(*fOutCodec)->setUnsupportedICC(unsupportedICC);
+ static_cast<SkPngCodec*>(*fOutCodec)->setIdatLength(idatLength);
}
-
// Release the pointers, which are now owned by the codec or the caller is expected to
// take ownership.
this->releasePngPtrs();
@@ -1058,9 +957,8 @@
, fInfo_ptr(info_ptr)
, fColorXformSrcRow(nullptr)
, fBitDepth(bitDepth)
-#ifdef SK_GOOGLE3_PNG_HACK
- , fNeedsToRereadHeader(true)
-#endif
+ , fIdatLength(0)
+ , fDecodedIdat(false)
{}
SkPngCodec::~SkPngCodec() {
@@ -1187,10 +1085,6 @@
}
bool SkPngCodec::onRewind() {
-#ifdef SK_GOOGLE3_PNG_HACK
- fNeedsToRereadHeader = true;
- return true;
-#else
// This sets fPng_ptr and fInfo_ptr to nullptr. If read_header
// succeeds, they will be repopulated, and if it fails, they will
// remain nullptr. Any future accesses to fPng_ptr and fInfo_ptr will
@@ -1206,8 +1100,8 @@
fPng_ptr = png_ptr;
fInfo_ptr = info_ptr;
+ fDecodedIdat = false;
return true;
-#endif
}
SkCodec::Result SkPngCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
@@ -1219,13 +1113,6 @@
{
return kInvalidConversion;
}
-#ifdef SK_GOOGLE3_PNG_HACK
- // Note that this is done after initializeXforms. Otherwise that method
- // would not have png_ptr to use.
- if (!this->rereadHeaderIfNecessary()) {
- return kCouldNotRewind;
- }
-#endif
if (options.fSubset) {
return kUnimplemented;
@@ -1244,12 +1131,6 @@
{
return kInvalidConversion;
}
-#ifdef SK_GOOGLE3_PNG_HACK
- // See note in onGetPixels.
- if (!this->rereadHeaderIfNecessary()) {
- return kCouldNotRewind;
- }
-#endif
this->allocateStorage(dstInfo);
diff --git a/src/codec/SkPngCodec.h b/src/codec/SkPngCodec.h
index 09231f1..4809723 100644
--- a/src/codec/SkPngCodec.h
+++ b/src/codec/SkPngCodec.h
@@ -16,13 +16,6 @@
#include "SkRefCnt.h"
#include "SkSwizzler.h"
-// FIXME (scroggo): GOOGLE3 is currently using an outdated version of libpng,
-// so we need to work around the lack of the method png_process_data_pause.
-// This code will be unnecessary once we update GOOGLE3. It would make more
-// sense to condition this on the version of libpng being used, but we do not
-// know that here because png.h is only included by the cpp file.
-#define SK_GOOGLE3_PNG_HACK
-
class SkStream;
class SkPngCodec : public SkCodec {
@@ -32,6 +25,9 @@
// Assume IsPng was called and returned true.
static SkCodec* NewFromStream(SkStream*, SkPngChunkReader* = NULL);
+ // FIXME (scroggo): Temporarily needed by AutoCleanPng.
+ void setIdatLength(size_t len) { fIdatLength = len; }
+
~SkPngCodec() override;
protected:
@@ -76,18 +72,6 @@
*/
void processData();
-#ifdef SK_GOOGLE3_PNG_HACK
- // In libpng 1.2.56, png_process_data_pause does not exist, so when we wanted to
- // read the header, we may have read too far. In that case, we need to delete the
- // png_ptr and info_ptr and recreate them. This method does that (and attaches the
- // chunk reader.
- bool rereadHeaderIfNecessary();
-
- // This method sets up the new png_ptr/info_ptr (created in rereadHeaderIfNecessary)
- // the way we set up the old one the first time in AutoCleanPng.decodeBounds's callback.
- void rereadInfoCallback();
-#endif
-
Result onStartIncrementalDecode(const SkImageInfo& dstInfo, void* pixels, size_t rowBytes,
const SkCodec::Options&,
SkPMColor* ctable, int* ctableCount) override;
@@ -134,9 +118,8 @@
SkAlphaType fXformAlphaType;
int fXformWidth;
-#ifdef SK_GOOGLE3_PNG_HACK
- bool fNeedsToRereadHeader;
-#endif
+ size_t fIdatLength;
+ bool fDecodedIdat;
typedef SkCodec INHERITED;
};
diff --git a/tests/CodecExactReadTest.cpp b/tests/CodecExactReadTest.cpp
new file mode 100644
index 0000000..7e0d8ea
--- /dev/null
+++ b/tests/CodecExactReadTest.cpp
@@ -0,0 +1,102 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "Resources.h"
+#include "Test.h"
+
+#include "SkBitmap.h"
+#include "SkCodec.h"
+#include "SkData.h"
+#include "SkStream.h"
+
+namespace {
+// This class emits a skiatest failure if a client attempts to read beyond its
+// end. Since it is used with complete, valid images, and contains nothing
+// after the encoded image data, it will emit a failure if the client attempts
+// to read beyond the logical end of the data.
+class MyStream : public SkStream {
+public:
+ static MyStream* Make(const char* path, skiatest::Reporter* r) {
+ SkASSERT(path);
+ sk_sp<SkData> data(GetResourceAsData(path));
+ if (!data) {
+ return nullptr;
+ }
+
+ return new MyStream(path, std::move(data), r);
+ }
+
+ size_t read(void* buf, size_t bytes) override {
+ const size_t remaining = fStream.getLength() - fStream.getPosition();
+ if (bytes > remaining) {
+ ERRORF(fReporter, "Tried to read %lu bytes (only %lu remaining) from %s",
+ bytes, remaining, fPath);
+ }
+ return fStream.read(buf, bytes);
+ }
+
+ bool rewind() override {
+ return fStream.rewind();
+ }
+
+ bool isAtEnd() const override {
+ return fStream.isAtEnd();
+ }
+private:
+ const char* fPath;
+ SkMemoryStream fStream;
+ skiatest::Reporter* fReporter; // Unowned
+
+ MyStream(const char* path, sk_sp<SkData> data, skiatest::Reporter* r)
+ : fPath(path)
+ , fStream(std::move(data))
+ , fReporter(r)
+ {}
+};
+} // namespace
+
+// Test that SkPngCodec does not attempt to read its input beyond the logical
+// end of its data. Some other SkCodecs do, but some Android apps rely on not
+// doing so for PNGs.
+DEF_TEST(Codec_end, r) {
+ for (const char* path : { "plane.png",
+ "yellow_rose.png",
+ "plane_interlaced.png" }) {
+ std::unique_ptr<MyStream> stream(MyStream::Make(path, r));
+ if (!stream) {
+ continue;
+ }
+
+ std::unique_ptr<SkCodec> codec(SkCodec::NewFromStream(stream.release()));
+ if (!codec) {
+ ERRORF(r, "Failed to create a codec from %s\n", path);
+ continue;
+ }
+
+ auto info = codec->getInfo().makeColorType(kN32_SkColorType);
+ SkBitmap bm;
+ bm.allocPixels(info);
+
+ auto result = codec->getPixels(bm.info(), bm.getPixels(), bm.rowBytes());
+ if (result != SkCodec::kSuccess) {
+ ERRORF(r, "Failed to getPixels from %s. error %i", path, result);
+ continue;
+ }
+
+ // Rewind and do an incremental decode.
+ result = codec->startIncrementalDecode(bm.info(), bm.getPixels(), bm.rowBytes());
+ if (result != SkCodec::kSuccess) {
+ ERRORF(r, "Failed to startIncrementalDecode from %s. error %i", path, result);
+ continue;
+ }
+
+ result = codec->incrementalDecode();
+ if (result != SkCodec::kSuccess) {
+ ERRORF(r, "Failed to incrementalDecode from %s. error %i", path, result);
+ }
+ }
+}
diff --git a/tests/CodecPartialTest.cpp b/tests/CodecPartialTest.cpp
index 20cd1d1..c029922 100644
--- a/tests/CodecPartialTest.cpp
+++ b/tests/CodecPartialTest.cpp
@@ -118,6 +118,9 @@
}
DEF_TEST(Codec_partial, r) {
+#if 0
+ // FIXME (scroggo): SkPngCodec needs to use SkStreamBuffer in order to
+ // support incremental decoding.
test_partial(r, "plane.png");
test_partial(r, "plane_interlaced.png");
test_partial(r, "yellow_rose.png");
@@ -128,7 +131,7 @@
test_partial(r, "arrow.png");
test_partial(r, "randPixels.png");
test_partial(r, "baby_tux.png");
-
+#endif
test_partial(r, "box.gif");
test_partial(r, "randPixels.gif", 215);
test_partial(r, "color_wheel.gif");
