RESTRICT AUTOMERGE: Fix uninitialized errors in SkPngCodec
Bug: 117838472
Test: Iae4d7f393c892111b12282c5eae31d79912721f9
- Initialize rowsDecoded in SkSampledCodec. Otherwise,
fillIncompleteImage may be called with an uninitialized
value. This change was originally uploaded to AOSP as
https://android-review.googlesource.com/c/platform/external/skia/+/785816
- If SkPngCodec hits an error, still transform from the
interlace buffer (if needed) and set rowsDecoded properly.
- Do not copy uninitialized memory from the interlace buffer.
- Make BRD treat kErrorInInput like kIncompleteInput. The two errors
are different for the purposes of incremental decode. For a direct
decode, they're essentially the same - part was decoded, but then
the decode was interrupted. This allows testing images with
errors on the bots without reporting a failure.
Originally uploaded as https://skia-review.googlesource.com/c/skia/+/161822
- Also includes SkPngCodec SafetyNet logging from
https://skia-review.googlesource.com/c/skia/+/170354
Change-Id: Ie170abf65393feb4edba60aa941f2783fe18cd8b
(cherry picked from commit fc165d5cccf133574058cd0c5ba3a95ea6ad2bb8)
diff --git a/src/android/SkBitmapRegionCodec.cpp b/src/android/SkBitmapRegionCodec.cpp
index 6b17a90..682c705 100644
--- a/src/android/SkBitmapRegionCodec.cpp
+++ b/src/android/SkBitmapRegionCodec.cpp
@@ -106,10 +106,14 @@
SkCodec::Result result = fCodec->getAndroidPixels(decodeInfo, dst, bitmap->rowBytes(),
&options);
- if (SkCodec::kSuccess != result && SkCodec::kIncompleteInput != result) {
- SkCodecPrintf("Error: Could not get pixels.\n");
- return false;
+ switch (result) {
+ case SkCodec::kSuccess:
+ case SkCodec::kIncompleteInput:
+ case SkCodec::kErrorInInput:
+ return true;
+ default:
+ SkCodecPrintf("Error: Could not get pixels with message \"%s\".\n",
+ SkCodec::ResultToString(result));
+ return false;
}
-
- return true;
}
diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp
index 31a407c..aa62138 100644
--- a/src/codec/SkPngCodec.cpp
+++ b/src/codec/SkPngCodec.cpp
@@ -25,6 +25,10 @@
#include "png.h"
#include <algorithm>
+#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
+ #include "SkAndroidFrameworkUtils.h"
+#endif
+
// This warning triggers false postives way too often in here.
#if defined(__GNUC__) && !defined(__clang__)
#pragma GCC diagnostic ignored "-Wclobbered"
@@ -481,6 +485,14 @@
}
}
+static SkCodec::Result log_and_return_error(bool success) {
+ if (success) return SkCodec::kIncompleteInput;
+#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
+ SkAndroidFrameworkUtils::SafetyNetLog("117838472");
+#endif
+ return SkCodec::kErrorInInput;
+}
+
class SkPngNormalDecoder : public SkPngCodec {
public:
SkPngNormalDecoder(const SkEncodedInfo& info, const SkImageInfo& imageInfo,
@@ -528,19 +540,16 @@
fFirstRow = 0;
fLastRow = height - 1;
- if (!this->processData()) {
- return kErrorInInput;
- }
-
- if (fRowsWrittenToOutput == height) {
- return SkCodec::kSuccess;
+ const bool success = this->processData();
+ if (success && fRowsWrittenToOutput == height) {
+ return kSuccess;
}
if (rowsDecoded) {
*rowsDecoded = fRowsWrittenToOutput;
}
- return SkCodec::kIncompleteInput;
+ return log_and_return_error(success);
}
void allRowsCallback(png_bytep row, int rowNum) {
@@ -560,25 +569,22 @@
fRowsNeeded = fLastRow - fFirstRow + 1;
}
- SkCodec::Result decode(int* rowsDecoded) override {
+ Result decode(int* rowsDecoded) override {
if (this->swizzler()) {
const int sampleY = this->swizzler()->sampleY();
fRowsNeeded = get_scaled_dimension(fLastRow - fFirstRow + 1, sampleY);
}
- if (!this->processData()) {
- return kErrorInInput;
- }
-
- if (fRowsWrittenToOutput == fRowsNeeded) {
- return SkCodec::kSuccess;
+ const bool success = this->processData();
+ if (success && fRowsWrittenToOutput == fRowsNeeded) {
+ return kSuccess;
}
if (rowsDecoded) {
*rowsDecoded = fRowsWrittenToOutput;
}
- return SkCodec::kIncompleteInput;
+ return log_and_return_error(success);
}
void rowCallback(png_bytep row, int rowNum) {
@@ -670,7 +676,7 @@
}
}
- SkCodec::Result decodeAllRows(void* dst, size_t rowBytes, int* rowsDecoded) override {
+ Result decodeAllRows(void* dst, size_t rowBytes, int* rowsDecoded) override {
const int height = this->getInfo().height();
this->setUpInterlaceBuffer(height);
png_set_progressive_read_fn(this->png_ptr(), this, nullptr, InterlacedRowCallback,
@@ -680,10 +686,7 @@
fLastRow = height - 1;
fLinesDecoded = 0;
- if (!this->processData()) {
- return kErrorInInput;
- }
-
+ const bool success = this->processData();
png_bytep srcRow = fInterlaceBuffer.get();
// FIXME: When resuming, this may rewrite rows that did not change.
for (int rowNum = 0; rowNum < fLinesDecoded; rowNum++) {
@@ -691,15 +694,15 @@
dst = SkTAddOffset<void>(dst, rowBytes);
srcRow = SkTAddOffset<png_byte>(srcRow, fPng_rowbytes);
}
- if (fInterlacedComplete) {
- return SkCodec::kSuccess;
+ if (success && fInterlacedComplete) {
+ return kSuccess;
}
if (rowsDecoded) {
*rowsDecoded = fLinesDecoded;
}
- return SkCodec::kIncompleteInput;
+ return log_and_return_error(success);
}
void setRange(int firstRow, int lastRow, void* dst, size_t rowBytes) override {
@@ -713,45 +716,45 @@
fLinesDecoded = 0;
}
- SkCodec::Result decode(int* rowsDecoded) override {
- if (this->processData() == false) {
- return kErrorInInput;
- }
+ Result decode(int* rowsDecoded) override {
+ const bool success = this->processData();
// Now apply Xforms on all the rows that were decoded.
if (!fLinesDecoded) {
if (rowsDecoded) {
*rowsDecoded = 0;
}
- return SkCodec::kIncompleteInput;
+ return log_and_return_error(success);
}
const int sampleY = this->swizzler() ? this->swizzler()->sampleY() : 1;
const int rowsNeeded = get_scaled_dimension(fLastRow - fFirstRow + 1, sampleY);
- int rowsWrittenToOutput = 0;
// FIXME: For resuming interlace, we may swizzle a row that hasn't changed. But it
// may be too tricky/expensive to handle that correctly.
// Offset srcRow by get_start_coord rows. We do not need to account for fFirstRow,
// since the first row in fInterlaceBuffer corresponds to fFirstRow.
- png_bytep srcRow = SkTAddOffset<png_byte>(fInterlaceBuffer.get(),
- fPng_rowbytes * get_start_coord(sampleY));
+ int srcRow = get_start_coord(sampleY);
void* dst = fDst;
- for (; rowsWrittenToOutput < rowsNeeded; rowsWrittenToOutput++) {
- this->applyXformRow(dst, srcRow);
+ int rowsWrittenToOutput = 0;
+ while (rowsWrittenToOutput < rowsNeeded && srcRow < fLinesDecoded) {
+ png_bytep src = SkTAddOffset<png_byte>(fInterlaceBuffer.get(), fPng_rowbytes * srcRow);
+ this->applyXformRow(dst, src);
dst = SkTAddOffset<void>(dst, fRowBytes);
- srcRow = SkTAddOffset<png_byte>(srcRow, fPng_rowbytes * sampleY);
+
+ rowsWrittenToOutput++;
+ srcRow += sampleY;
}
- if (fInterlacedComplete) {
- return SkCodec::kSuccess;
+ if (success && fInterlacedComplete) {
+ return kSuccess;
}
if (rowsDecoded) {
*rowsDecoded = rowsWrittenToOutput;
}
- return SkCodec::kIncompleteInput;
+ return log_and_return_error(success);
}
void setUpInterlaceBuffer(int height) {
diff --git a/src/codec/SkSampledCodec.cpp b/src/codec/SkSampledCodec.cpp
index ac0539f..2282d6b 100644
--- a/src/codec/SkSampledCodec.cpp
+++ b/src/codec/SkSampledCodec.cpp
@@ -113,17 +113,17 @@
const SkCodec::Result startResult = this->codec()->startIncrementalDecode(
scaledInfo, pixels, rowBytes, &codecOptions);
if (SkCodec::kSuccess == startResult) {
- int rowsDecoded;
+ int rowsDecoded = 0;
const SkCodec::Result incResult = this->codec()->incrementalDecode(&rowsDecoded);
if (incResult == SkCodec::kSuccess) {
return SkCodec::kSuccess;
}
- SkASSERT(SkCodec::kIncompleteInput == incResult);
+ SkASSERT(incResult == SkCodec::kIncompleteInput || incResult == SkCodec::kErrorInInput);
// FIXME: Can zero initialized be read from SkCodec::fOptions?
this->codec()->fillIncompleteImage(scaledInfo, pixels, rowBytes,
options.fZeroInitialized, scaledSubsetHeight, rowsDecoded);
- return SkCodec::kIncompleteInput;
+ return incResult;
} else if (startResult != SkCodec::kUnimplemented) {
return startResult;
}
@@ -244,7 +244,7 @@
sampler->setSampleY(sampleY);
- int rowsDecoded;
+ int rowsDecoded = 0;
const SkCodec::Result incResult = this->codec()->incrementalDecode(&rowsDecoded);
if (incResult == SkCodec::kSuccess) {
return SkCodec::kSuccess;
