aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLeon Scroggins III <scroggo@google.com>2017-09-27 16:31:08 -0400
committerLeon Scroggins <scroggo@google.com>2017-10-02 15:25:42 +0000
commit7a3ba537f7456b4870a983cd9e0a09bb3d478efc (patch)
treeaefe29bb7085d4c4baf080aee0cd0b41e638dae7
parente14a0aa7ea6a4494bbfae16864fab44c69b1f8d8 (diff)
downloadskia-7a3ba537f7456b4870a983cd9e0a09bb3d478efc.tar.gz
Fix truncated webp images
Bug: b/65290323 Test: ag/2974111 Merged-In: Ib6f385766d6d46ed7fe56188cae5a71b100102bd Change-Id: I0cba5ab639f1e66b7c493a9f63735a0f5edbcfbf Original message description: ======================================================================== If a webp file is truncated such that no rows can be decoded, WebPIDecGetRGB does not initialize its "last_y" parameter. We use rowsDecoded (passed as last_y) to determine which remaining rows to fill. Check the return value of WebPIDecGetRGB. If it fails (returns null), or rowsDecoded is <= 0 (matching Chromium's check), return kInvalidInput, since there is nothing to draw. Note that this is a change in behavior for Android. Previously we would decode an empty webp to just a transparent/black rectangle, whereas now we simply fail. I think this is a change for the better. Add a test which truncates a file to have 0 rows available and attempts to decode it. msan verifies that we no longer depend on the uninitialized value. Stop attempting to test decoding subsets from an incomplete webp (in CodecTest.cpp). Unless we have decoded the portion covered by the subset, this will fail. Remove test images inc0.webp (from both dm/ and colorspace/) and inc1.webp. These just decode to transparent rectangles. Replace them with inc2.webp and inc3.webp, which decode part of the image and then have to fill with transparent. Change-Id: I64d40be91c574b45963f9a43d8dd8f4929dd2939 Reviewed-on: https://skia-review.googlesource.com/50303 Commit-Queue: Leon Scroggins <scroggo@google.com> Reviewed-by: James Zern <jzern@google.com> ======================================================================== For simplicity, this does not update VERSION or tasks.json, as does the original CL.
-rw-r--r--src/codec/SkWebpCodec.cpp7
-rw-r--r--tests/CodecTest.cpp25
2 files changed, 26 insertions, 6 deletions
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index ab0b91b112..d23d00656b 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -323,7 +323,7 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
return kInvalidInput;
}
- int rowsDecoded;
+ int rowsDecoded = 0;
SkCodec::Result result;
switch (WebPIUpdate(idec, frame.fragment.bytes, frame.fragment.size)) {
case VP8_STATUS_OK:
@@ -331,7 +331,10 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
result = kSuccess;
break;
case VP8_STATUS_SUSPENDED:
- WebPIDecGetRGB(idec, &rowsDecoded, nullptr, nullptr, nullptr);
+ if (!WebPIDecGetRGB(idec, &rowsDecoded, nullptr, nullptr, nullptr)
+ || rowsDecoded <= 0) {
+ return kInvalidInput;
+ }
*rowsDecodedPtr = rowsDecoded + dstY;
result = kIncompleteInput;
break;
diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp
index 4a9c4a6515..6bb0f1b2cb 100644
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -393,10 +393,6 @@ static void check(skiatest::Reporter* r,
if (supportsSubsetDecoding) {
if (expectedResult == SkCodec::kSuccess) {
REPORTER_ASSERT(r, result == expectedResult);
- } else {
- SkASSERT(expectedResult == SkCodec::kIncompleteInput);
- REPORTER_ASSERT(r, result == SkCodec::kIncompleteInput
- || result == SkCodec::kSuccess);
}
// Webp is the only codec that supports subsets, and it will have modified the subset
// to have even left/top.
@@ -1596,3 +1592,24 @@ DEF_TEST(Codec_EncodeICC, r) {
test_encode_icc(r, SkEncodedImageFormat::kJPEG, SkTransferFunctionBehavior::kIgnore);
test_encode_icc(r, SkEncodedImageFormat::kWEBP, SkTransferFunctionBehavior::kIgnore);
}
+
+DEF_TEST(Codec_webp_rowsDecoded, r) {
+ const char* path = "baby_tux.webp";
+ sk_sp<SkData> data(GetResourceAsData(path));
+ if (!data) {
+ return;
+ }
+
+ // Truncate this file so that the header is available but no rows can be
+ // decoded. This should create a codec but fail to decode.
+ size_t truncatedSize = 5000;
+ sk_sp<SkData> subset = SkData::MakeSubset(data.get(), 0, truncatedSize);
+ std::unique_ptr<SkCodec> codec(SkCodec::NewFromData(std::move(subset)));
+ if (!codec) {
+ ERRORF(r, "Failed to create a codec for %s truncated to only %lu bytes",
+ path, truncatedSize);
+ return;
+ }
+
+ test_info(r, codec.get(), codec->getInfo(), SkCodec::kInvalidInput, nullptr);
+}