diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp index bceb703cda..0febd2abb0 100644 --- a/dm/DMSrcSink.cpp +++ b/dm/DMSrcSink.cpp @@ -513,6 +513,7 @@ Error CodecSrc::draw(SkCanvas* canvas) const { } switch (result) { case SkCodec::kSuccess: + case SkCodec::kErrorInInput: case SkCodec::kIncompleteInput: { // If the next frame depends on this one, store it in priorFrame. // It is possible that we may discard a frame that future frames depend on, @@ -532,7 +533,7 @@ Error CodecSrc::draw(SkCanvas* canvas) const { canvas->translate(SkIntToScalar(xTranslate), SkIntToScalar(yTranslate)); draw_to_canvas(canvas, bitmapInfo, pixels.get(), rowBytes, colorPtr, colorCount, fDstColorType); - if (result == SkCodec::kIncompleteInput) { + if (result != SkCodec::kSuccess) { return ""; } break; @@ -556,8 +557,9 @@ Error CodecSrc::draw(SkCanvas* canvas) const { switch (codec->getPixels(decodeInfo, pixels.get(), rowBytes, &options, colorPtr, &colorCount)) { case SkCodec::kSuccess: - // We consider incomplete to be valid, since we should still decode what is + // We consider these to be valid, since we should still decode what is // available. + case SkCodec::kErrorInInput: case SkCodec::kIncompleteInput: break; default: @@ -589,7 +591,8 @@ Error CodecSrc::draw(SkCanvas* canvas) const { if (SkCodec::kSuccess == codec->startIncrementalDecode(decodeInfo, dst, rowBytes, &options, colorPtr, &colorCount)) { int rowsDecoded; - if (SkCodec::kIncompleteInput == codec->incrementalDecode(&rowsDecoded)) { + auto result = codec->incrementalDecode(&rowsDecoded); + if (SkCodec::kIncompleteInput == result || SkCodec::kErrorInInput == result) { codec->fillIncompleteImage(decodeInfo, dst, rowBytes, SkCodec::kNo_ZeroInitialized, height, rowsDecoded); @@ -747,6 +750,7 @@ Error CodecSrc::draw(SkCanvas* canvas) const { &options, colorPtr, &colorCount); switch (result) { case SkCodec::kSuccess: + case SkCodec::kErrorInInput: case SkCodec::kIncompleteInput: break; default: @@ -880,6 +884,7 @@ Error AndroidCodecSrc::draw(SkCanvas* canvas) const { switch (codec->getAndroidPixels(decodeInfo, pixels.get(), rowBytes, &options)) { case SkCodec::kSuccess: + case SkCodec::kErrorInInput: case SkCodec::kIncompleteInput: break; default: @@ -1124,8 +1129,13 @@ Error ColorCodecSrc::draw(SkCanvas* canvas) const { size_t rowBytes = bitmap.rowBytes(); SkCodec::Result r = codec->getPixels(decodeInfo, bitmap.getPixels(), rowBytes); - if (SkCodec::kSuccess != r && SkCodec::kIncompleteInput != r) { - return SkStringPrintf("Couldn't getPixels %s. Error code %d", fPath.c_str(), r); + switch (r) { + case SkCodec::kSuccess: + case SkCodec::kErrorInInput: + case SkCodec::kIncompleteInput: + break; + default: + return SkStringPrintf("Couldn't getPixels %s. Error code %d", fPath.c_str(), r); } switch (fMode) { diff --git a/fuzz/fuzz.cpp b/fuzz/fuzz.cpp index 8e2a73376f..d451481049 100644 --- a/fuzz/fuzz.cpp +++ b/fuzz/fuzz.cpp @@ -243,6 +243,9 @@ static void fuzz_img(sk_sp bytes, uint8_t scale, uint8_t mode) { case SkCodec::kIncompleteInput: SkDebugf("[terminated] Partial Success\n"); break; + case SkCodec::kErrorInInput: + SkDebugf("[terminated] Partial Success with error\n"); + break; case SkCodec::kInvalidConversion: SkDebugf("Incompatible colortype conversion\n"); // Crash to allow afl-fuzz to know this was a bug. @@ -376,6 +379,7 @@ static void fuzz_img(sk_sp bytes, uint8_t scale, uint8_t mode) { switch (result) { case SkCodec::kSuccess: case SkCodec::kIncompleteInput: + case SkCodec::kErrorInInput: SkDebugf("okay\n"); break; case SkCodec::kInvalidConversion: @@ -428,7 +432,7 @@ static void fuzz_img(sk_sp bytes, uint8_t scale, uint8_t mode) { } result = codec->incrementalDecode(); - if (result == SkCodec::kIncompleteInput) { + if (result == SkCodec::kIncompleteInput || result == SkCodec::kErrorInInput) { SkDebugf("okay\n"); // Frames beyond this one will not decode. break; diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h index 6b3aa5e020..2043d230b1 100644 --- a/include/codec/SkCodec.h +++ b/include/codec/SkCodec.h @@ -194,6 +194,13 @@ public: * The input is incomplete. A partial image was generated. */ kIncompleteInput, + /** + * Like kIncompleteInput, except the input had an error. + * + * If returned from an incremental decode, decoding cannot continue, + * even with more data. + */ + kErrorInInput, /** * The generator cannot convert to match the request, ignoring * dimensions. diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp index 60d25210c8..26b31ae11f 100644 --- a/src/codec/SkCodec.cpp +++ b/src/codec/SkCodec.cpp @@ -317,8 +317,10 @@ SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t const Result result = this->onGetPixels(info, pixels, rowBytes, *options, ctable, ctableCount, &rowsDecoded); - if ((kIncompleteInput == result || kSuccess == result) && ctableCount) { - SkASSERT(*ctableCount >= 0 && *ctableCount <= 256); + if (ctableCount) { + if (kIncompleteInput == result || kSuccess == result || kErrorInInput == result) { + SkASSERT(*ctableCount >= 0 && *ctableCount <= 256); + } } // A return value of kIncompleteInput indicates a truncated image stream. @@ -326,7 +328,7 @@ SkCodec::Result SkCodec::getPixels(const SkImageInfo& info, void* pixels, size_t // Some subclasses will take care of filling any uninitialized memory on // their own. They indicate that all of the memory has been filled by // setting rowsDecoded equal to the height. - if (kIncompleteInput == result && rowsDecoded != info.height()) { + if ((kIncompleteInput == result || kErrorInInput == result) && rowsDecoded != info.height()) { // FIXME: (skbug.com/5772) fillIncompleteImage will fill using the swizzler's width, unless // there is a subset. In that case, it will use the width of the subset. From here, the // subset will only be non-null in the case of SkWebpCodec, but it treats the subset diff --git a/src/codec/SkCodecImageGenerator.cpp b/src/codec/SkCodecImageGenerator.cpp index bf794d60d3..20d547ad36 100644 --- a/src/codec/SkCodecImageGenerator.cpp +++ b/src/codec/SkCodecImageGenerator.cpp @@ -49,6 +49,7 @@ bool SkCodecImageGenerator::onGetPixels(const SkImageInfo& info, void* pixels, s switch (result) { case SkCodec::kSuccess: case SkCodec::kIncompleteInput: + case SkCodec::kErrorInInput: return true; default: return false; @@ -66,6 +67,7 @@ bool SkCodecImageGenerator::onGetYUV8Planes(const SkYUVSizeInfo& sizeInfo, void* switch (result) { case SkCodec::kSuccess: case SkCodec::kIncompleteInput: + case SkCodec::kErrorInInput: return true; default: return false; diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp index eefe8986b7..89889d2555 100644 --- a/src/codec/SkGifCodec.cpp +++ b/src/codec/SkGifCodec.cpp @@ -407,20 +407,15 @@ SkCodec::Result SkGifCodec::decodeFrame(bool firstAttempt, const Options& opts, return kSuccess; } - // Note: there is a difference between the following call to SkGifImageReader::decode - // returning false and leaving frameDecoded false: - // - If the method returns false, there was an error in the stream. We still treat this as - // incomplete, since we have already decoded some rows. - // - If frameDecoded is false, that just means that we do not have enough data. If more data - // is supplied, we may be able to continue decoding this frame. We also treat this as - // incomplete. - // FIXME: Ensure that we do not attempt to continue decoding if the method returns false and - // more data is supplied. bool frameDecoded = false; - if (!fReader->decode(frameIndex, &frameDecoded) || !frameDecoded) { + const bool fatalError = !fReader->decode(frameIndex, &frameDecoded); + if (fatalError || !frameDecoded) { if (rowsDecoded) { *rowsDecoded = fRowsDecoded; } + if (fatalError) { + return kErrorInInput; + } return kIncompleteInput; } diff --git a/src/codec/SkSampledCodec.cpp b/src/codec/SkSampledCodec.cpp index 2b6483b058..045f184f29 100644 --- a/src/codec/SkSampledCodec.cpp +++ b/src/codec/SkSampledCodec.cpp @@ -251,12 +251,12 @@ SkCodec::Result SkSampledCodec::sampledDecode(const SkImageInfo& info, void* pix if (incResult == SkCodec::kSuccess) { return SkCodec::kSuccess; } - SkASSERT(incResult == SkCodec::kIncompleteInput); + SkASSERT(incResult == SkCodec::kIncompleteInput || incResult == SkCodec::kErrorInInput); SkASSERT(rowsDecoded <= info.height()); this->codec()->fillIncompleteImage(info, pixels, rowBytes, options.fZeroInitialized, info.height(), rowsDecoded); - return SkCodec::kIncompleteInput; + return incResult; } else if (startResult != SkCodec::kUnimplemented) { return startResult; } // kUnimplemented means use the old method. diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp index 04c5a088bd..7ec903728e 100644 --- a/tests/CodecTest.cpp +++ b/tests/CodecTest.cpp @@ -1450,7 +1450,7 @@ static void test_invalid_images(skiatest::Reporter* r, const char* path, DEF_TEST(Codec_InvalidImages, r) { // ASAN will complain if there is an issue. - test_invalid_images(r, "invalid_images/skbug5887.gif", SkCodec::kIncompleteInput); + test_invalid_images(r, "invalid_images/skbug5887.gif", SkCodec::kErrorInInput); test_invalid_images(r, "invalid_images/many-progressive-scans.jpg", SkCodec::kInvalidInput); test_invalid_images(r, "invalid_images/b33251605.bmp", SkCodec::kIncompleteInput); test_invalid_images(r, "invalid_images/bad_palette.png", SkCodec::kInvalidInput);