Report error on failure to create SkCodec
Update NewFromStream to report an error on failure to create an
SkCodec, so that a client can distinguish between
- not enough data
- invalid data
In Chromium, this will allow blink::ImageDecoder to call SetFailed if
the stream is invalid early and we never create an SkCodec. Without
this, ImageDecoder will keep trying to create an SkCodec when it
receives more data.
Change-Id: I4f505c56d91c982be36a828fd0f7db17b1596588
Reviewed-on: https://skia-review.googlesource.com/22642
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Chris Blume <cblume@chromium.org>
diff --git a/src/codec/SkAndroidCodec.cpp b/src/codec/SkAndroidCodec.cpp
index 046f0ae..97be146 100644
--- a/src/codec/SkAndroidCodec.cpp
+++ b/src/codec/SkAndroidCodec.cpp
@@ -66,7 +66,7 @@
{}
SkAndroidCodec* SkAndroidCodec::NewFromStream(SkStream* stream, SkPngChunkReader* chunkReader) {
- std::unique_ptr<SkCodec> codec(SkCodec::NewFromStream(stream, chunkReader));
+ std::unique_ptr<SkCodec> codec(SkCodec::NewFromStream(stream, nullptr, chunkReader));
if (nullptr == codec) {
return nullptr;
}
diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp
index 3e224c1..70397f6 100644
--- a/src/codec/SkBmpCodec.cpp
+++ b/src/codec/SkBmpCodec.cpp
@@ -67,16 +67,16 @@
* Creates a bmp decoder
* Reads enough of the stream to determine the image format
*/
-SkCodec* SkBmpCodec::NewFromStream(SkStream* stream) {
- return SkBmpCodec::NewFromStream(stream, false);
+SkCodec* SkBmpCodec::NewFromStream(SkStream* stream, Result* result) {
+ return SkBmpCodec::NewFromStream(stream, result, false);
}
/*
* Creates a bmp decoder for a bmp embedded in ico
* Reads enough of the stream to determine the image format
*/
-SkCodec* SkBmpCodec::NewFromIco(SkStream* stream) {
- return SkBmpCodec::NewFromStream(stream, true);
+SkCodec* SkBmpCodec::NewFromIco(SkStream* stream, Result* result) {
+ return SkBmpCodec::NewFromStream(stream, result, true);
}
// Header size constants
@@ -132,14 +132,7 @@
}
}
-/*
- * Read enough of the stream to initialize the SkBmpCodec. Returns a bool
- * representing success or failure. If it returned true, and codecOut was
- * not nullptr, it will be set to a new SkBmpCodec.
- * If codecOut is set to a new SkCodec, it will take ownership of the stream.
- * Otherwise, the stream will not be deleted.
- */
-bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
+SkCodec::Result SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
std::unique_ptr<SkCodec>* codecOut) {
// The total bytes in the bmp file
// We only need to use this value for RLE decoding, so we will only
@@ -157,14 +150,14 @@
if (stream->read(hBuffer, kBmpHeaderBytesPlusFour) !=
kBmpHeaderBytesPlusFour) {
SkCodecPrintf("Error: unable to read first bitmap header.\n");
- return false;
+ return kIncompleteInput;
}
totalBytes = get_int(hBuffer, 2);
offset = get_int(hBuffer, 10);
if (offset < kBmpHeaderBytes + kBmpOS2V1Bytes) {
SkCodecPrintf("Error: invalid starting location for pixel data\n");
- return false;
+ return kInvalidInput;
}
// The size of the second (info) header in bytes
@@ -173,7 +166,7 @@
infoBytes = get_int(hBuffer, 14);
if (infoBytes < kBmpOS2V1Bytes) {
SkCodecPrintf("Error: invalid second header size.\n");
- return false;
+ return kInvalidInput;
}
} else {
// This value is only used by RLE compression. Bmp in Ico files do not
@@ -190,19 +183,19 @@
uint8_t hBuffer[4];
if (stream->read(hBuffer, 4) != 4) {
SkCodecPrintf("Error: unable to read size of second bitmap header.\n");
- return false;
+ return kIncompleteInput;
}
infoBytes = get_int(hBuffer, 0);
if (infoBytes < kBmpOS2V1Bytes) {
SkCodecPrintf("Error: invalid second header size.\n");
- return false;
+ return kInvalidInput;
}
}
// Determine image information depending on second header format
const BmpHeaderType headerType = get_header_type(infoBytes);
if (kUnknown_BmpHeaderType == headerType) {
- return false;
+ return kInvalidInput;
}
// We already read the first four bytes of the info header to get the size
@@ -212,7 +205,7 @@
std::unique_ptr<uint8_t[]> iBuffer(new uint8_t[infoBytesRemaining]);
if (stream->read(iBuffer.get(), infoBytesRemaining) != infoBytesRemaining) {
SkCodecPrintf("Error: unable to read second bitmap header.\n");
- return false;
+ return kIncompleteInput;
}
// The number of bits used per pixel in the pixel data
@@ -268,7 +261,7 @@
case kUnknown_BmpHeaderType:
// We'll exit above in this case.
SkASSERT(false);
- return false;
+ return kInvalidInput;
}
// Check for valid dimensions from header
@@ -287,7 +280,7 @@
constexpr int kMaxDim = 1 << 16;
if (width <= 0 || height <= 0 || width >= kMaxDim || height >= kMaxDim) {
SkCodecPrintf("Error: invalid bitmap dimensions.\n");
- return false;
+ return kInvalidInput;
}
// Create mask struct
@@ -337,7 +330,7 @@
uint8_t buffer[kBmpMaskBytes];
if (stream->read(buffer, kBmpMaskBytes) != kBmpMaskBytes) {
SkCodecPrintf("Error: unable to read bit inputMasks.\n");
- return false;
+ return kIncompleteInput;
}
maskBytes = kBmpMaskBytes;
inputMasks.red = get_int(buffer, 0);
@@ -384,10 +377,10 @@
// in chromium. I have not come across a test case
// that uses this format.
SkCodecPrintf("Error: huffman format unsupported.\n");
- return false;
+ return kUnimplemented;
default:
SkCodecPrintf("Error: invalid bmp bit masks header.\n");
- return false;
+ return kInvalidInput;
}
break;
case kJpeg_BmpCompressionMethod:
@@ -401,16 +394,16 @@
// It is unsupported in the previous version and
// in chromium. I think it is used mostly for printers.
SkCodecPrintf("Error: compression format not supported.\n");
- return false;
+ return kUnimplemented;
case kCMYK_BmpCompressionMethod:
case kCMYK8BitRLE_BmpCompressionMethod:
case kCMYK4BitRLE_BmpCompressionMethod:
// TODO: Same as above.
SkCodecPrintf("Error: CMYK not supported for bitmap decoding.\n");
- return false;
+ return kUnimplemented;
default:
SkCodecPrintf("Error: invalid format for bitmap decoding.\n");
- return false;
+ return kInvalidInput;
}
iBuffer.reset();
@@ -421,7 +414,7 @@
// Seems like we can just assume that the offset is zero and try to decode?
// Maybe we don't want to try to decode corrupt images?
SkCodecPrintf("Error: pixel data offset less than header size.\n");
- return false;
+ return kInvalidInput;
}
@@ -474,7 +467,7 @@
break;
default:
SkCodecPrintf("Error: invalid input value for bits per pixel.\n");
- return false;
+ return kInvalidInput;
}
if (codecOut) {
@@ -486,16 +479,17 @@
codecOut->reset(new SkBmpStandardCodec(width, height, info, stream, bitsPerPixel,
numColors, bytesPerColor, offset - bytesRead,
rowOrder, isOpaque, inIco));
- return static_cast<SkBmpStandardCodec*>(codecOut->get())->didCreateSrcBuffer();
+ return static_cast<SkBmpStandardCodec*>(codecOut->get())->didCreateSrcBuffer()
+ ? kSuccess : kInvalidInput;
}
- return true;
+ return kSuccess;
}
case kBitMask_BmpInputFormat: {
// Bmp-in-Ico must be standard mode
if (inIco) {
SkCodecPrintf("Error: Icos may not use bit mask format.\n");
- return false;
+ return kInvalidInput;
}
switch (bitsPerPixel) {
@@ -505,7 +499,7 @@
break;
default:
SkCodecPrintf("Error: invalid input value for bits per pixel.\n");
- return false;
+ return kInvalidInput;
}
// Skip to the start of the pixel array.
@@ -513,7 +507,7 @@
// in bit mask mode.
if (stream->skip(offset - bytesRead) != offset - bytesRead) {
SkCodecPrintf("Error: unable to skip to image data.\n");
- return false;
+ return kIncompleteInput;
}
if (codecOut) {
@@ -521,7 +515,7 @@
std::unique_ptr<SkMasks> masks(SkMasks::CreateMasks(inputMasks, bitsPerPixel));
if (nullptr == masks) {
SkCodecPrintf("Error: invalid input masks.\n");
- return false;
+ return kInvalidInput;
}
// Masked bmps are not a great fit for SkEncodedInfo, since they have
@@ -540,9 +534,10 @@
const SkEncodedInfo info = SkEncodedInfo::Make(color, alpha, 8);
codecOut->reset(new SkBmpMaskCodec(width, height, info, stream, bitsPerPixel,
masks.release(), rowOrder));
- return static_cast<SkBmpMaskCodec*>(codecOut->get())->didCreateSrcBuffer();
+ return static_cast<SkBmpMaskCodec*>(codecOut->get())->didCreateSrcBuffer()
+ ? kSuccess : kInvalidInput;
}
- return true;
+ return kSuccess;
}
case kRLE_BmpInputFormat: {
@@ -552,7 +547,7 @@
// Check for a valid number of total bytes when in RLE mode
if (totalBytes <= offset) {
SkCodecPrintf("Error: RLE requires valid input size.\n");
- return false;
+ return kInvalidInput;
}
// Bmp-in-Ico must be standard mode
@@ -572,11 +567,11 @@
numColors, bytesPerColor, offset - bytesRead,
rowOrder));
}
- return true;
+ return kSuccess;
}
default:
SkASSERT(false);
- return false;
+ return kInvalidInput;
}
}
@@ -584,16 +579,16 @@
* Creates a bmp decoder
* Reads enough of the stream to determine the image format
*/
-SkCodec* SkBmpCodec::NewFromStream(SkStream* stream, bool inIco) {
+SkCodec* SkBmpCodec::NewFromStream(SkStream* stream, Result* result, bool inIco) {
std::unique_ptr<SkStream> streamDeleter(stream);
std::unique_ptr<SkCodec> codec;
- bool success = ReadHeader(stream, inIco, &codec);
+ *result = ReadHeader(stream, inIco, &codec);
if (codec) {
// codec has taken ownership of stream, so we do not need to
// delete it.
streamDeleter.release();
}
- return success ? codec.release() : nullptr;
+ return kSuccess == *result ? codec.release() : nullptr;
}
SkBmpCodec::SkBmpCodec(int width, int height, const SkEncodedInfo& info, SkStream* stream,
@@ -606,7 +601,7 @@
{}
bool SkBmpCodec::onRewind() {
- return SkBmpCodec::ReadHeader(this->stream(), this->inIco(), nullptr);
+ return SkBmpCodec::ReadHeader(this->stream(), this->inIco(), nullptr) == kSuccess;
}
int32_t SkBmpCodec::getDstRow(int32_t y, int32_t height) const {
diff --git a/src/codec/SkBmpCodec.h b/src/codec/SkBmpCodec.h
index bf9727f..d5f5a32 100644
--- a/src/codec/SkBmpCodec.h
+++ b/src/codec/SkBmpCodec.h
@@ -28,13 +28,13 @@
* Creates a bmp decoder
* Reads enough of the stream to determine the image format
*/
- static SkCodec* NewFromStream(SkStream*);
+ static SkCodec* NewFromStream(SkStream*, Result*);
/*
* Creates a bmp decoder for a bmp embedded in ico
* Reads enough of the stream to determine the image format
*/
- static SkCodec* NewFromIco(SkStream*);
+ static SkCodec* NewFromIco(SkStream*, Result*);
protected:
@@ -44,12 +44,11 @@
SkEncodedImageFormat onGetEncodedFormat() const override { return SkEncodedImageFormat::kBMP; }
/*
- * Read enough of the stream to initialize the SkBmpCodec. Returns a bool
- * representing success or failure. If it returned true, and codecOut was
- * not nullptr, it will be set to a new SkBmpCodec.
+ * Read enough of the stream to initialize the SkBmpCodec.
+ * On kSuccess, if codecOut is not nullptr, it will be set to a new SkBmpCodec.
* If an SkCodec is created, it will take ownership of the SkStream.
*/
- static bool ReadHeader(SkStream*, bool inIco, std::unique_ptr<SkCodec>* codecOut);
+ static Result ReadHeader(SkStream*, bool inIco, std::unique_ptr<SkCodec>* codecOut);
bool onRewind() override;
@@ -113,7 +112,7 @@
* Creates a bmp decoder
* Reads enough of the stream to determine the image format
*/
- static SkCodec* NewFromStream(SkStream*, bool inIco);
+ static SkCodec* NewFromStream(SkStream*, Result*, bool inIco);
/*
* Decodes the next dstInfo.height() lines.
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index 4bd3917..872fe2b 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -26,7 +26,7 @@
struct DecoderProc {
bool (*IsFormat)(const void*, size_t);
- SkCodec* (*NewFromStream)(SkStream*);
+ SkCodec* (*NewFromStream)(SkStream*, SkCodec::Result*);
};
static const DecoderProc gDecoderProcs[] = {
@@ -49,8 +49,14 @@
}
SkCodec* SkCodec::NewFromStream(SkStream* stream,
- SkPngChunkReader* chunkReader) {
+ Result* outResult, SkPngChunkReader* chunkReader) {
+ Result resultStorage;
+ if (!outResult) {
+ outResult = &resultStorage;
+ }
+
if (!stream) {
+ *outResult = kInvalidInput;
return nullptr;
}
@@ -81,6 +87,7 @@
bytesRead = stream->read(buffer, bytesToRead);
if (!stream->rewind()) {
SkCodecPrintf("Encoded image data could not peek or rewind to determine format!\n");
+ *outResult = kCouldNotRewind;
return nullptr;
}
}
@@ -89,22 +96,28 @@
// But this code follows the same pattern as the loop.
#ifdef SK_HAS_PNG_LIBRARY
if (SkPngCodec::IsPng(buffer, bytesRead)) {
- return SkPngCodec::NewFromStream(streamDeleter.release(), chunkReader);
+ return SkPngCodec::NewFromStream(streamDeleter.release(), outResult, chunkReader);
} else
#endif
{
for (DecoderProc proc : gDecoderProcs) {
if (proc.IsFormat(buffer, bytesRead)) {
- return proc.NewFromStream(streamDeleter.release());
+ return proc.NewFromStream(streamDeleter.release(), outResult);
}
}
#ifdef SK_CODEC_DECODES_RAW
// Try to treat the input as RAW if all the other checks failed.
- return SkRawCodec::NewFromStream(streamDeleter.release());
+ return SkRawCodec::NewFromStream(streamDeleter.release(), outResult);
#endif
}
+ if (bytesRead < bytesToRead) {
+ *outResult = kIncompleteInput;
+ } else {
+ *outResult = kUnimplemented;
+ }
+
return nullptr;
}
@@ -112,7 +125,7 @@
if (!data) {
return nullptr;
}
- return NewFromStream(new SkMemoryStream(data), reader);
+ return NewFromStream(new SkMemoryStream(data), nullptr, reader);
}
SkCodec::SkCodec(int width, int height, const SkEncodedInfo& info,
diff --git a/src/codec/SkGifCodec.cpp b/src/codec/SkGifCodec.cpp
index 081f237..879297d 100644
--- a/src/codec/SkGifCodec.cpp
+++ b/src/codec/SkGifCodec.cpp
@@ -66,21 +66,17 @@
return result;
}
-/*
- * Assumes IsGif was called and returned true
- * Creates a gif decoder
- * Reads enough of the stream to determine the image format
- */
-SkCodec* SkGifCodec::NewFromStream(SkStream* stream) {
+SkCodec* SkGifCodec::NewFromStream(SkStream* stream, Result* result) {
std::unique_ptr<SkGifImageReader> reader(new SkGifImageReader(stream));
- if (!reader->parse(SkGifImageReader::SkGIFSizeQuery)) {
- // Fatal error occurred.
+ *result = reader->parse(SkGifImageReader::SkGIFSizeQuery);
+ if (*result != kSuccess) {
return nullptr;
}
// If no images are in the data, or the first header is not yet defined, we cannot
// create a codec. In either case, the width and height are not yet known.
if (0 == reader->imagesCount() || !reader->frameContext(0)->isHeaderDefined()) {
+ *result = kInvalidInput;
return nullptr;
}
diff --git a/src/codec/SkGifCodec.h b/src/codec/SkGifCodec.h
index 7bb86f8..6ecd419 100644
--- a/src/codec/SkGifCodec.h
+++ b/src/codec/SkGifCodec.h
@@ -27,10 +27,9 @@
/*
* Assumes IsGif was called and returned true
- * Creates a gif decoder
* Reads enough of the stream to determine the image format
*/
- static SkCodec* NewFromStream(SkStream*);
+ static SkCodec* NewFromStream(SkStream*, Result*);
// Callback for SkGifImageReader when a row is available.
bool haveDecodedRow(int frameIndex, const unsigned char* rowBegin,
diff --git a/src/codec/SkIcoCodec.cpp b/src/codec/SkIcoCodec.cpp
index d1cbed0..2e61bfe 100644
--- a/src/codec/SkIcoCodec.cpp
+++ b/src/codec/SkIcoCodec.cpp
@@ -26,12 +26,7 @@
!memcmp(buffer, curSig, sizeof(curSig)));
}
-/*
- * Assumes IsIco was called and returned true
- * Creates an Ico decoder
- * Reads enough of the stream to determine the image format
- */
-SkCodec* SkIcoCodec::NewFromStream(SkStream* stream) {
+SkCodec* SkIcoCodec::NewFromStream(SkStream* stream, Result* result) {
// Ensure that we do not leak the input stream
std::unique_ptr<SkStream> inputStream(stream);
@@ -44,6 +39,7 @@
if (inputStream.get()->read(dirBuffer.get(), kIcoDirectoryBytes) !=
kIcoDirectoryBytes) {
SkCodecPrintf("Error: unable to read ico directory header.\n");
+ *result = kIncompleteInput;
return nullptr;
}
@@ -51,6 +47,7 @@
const uint16_t numImages = get_short(dirBuffer.get(), 4);
if (0 == numImages) {
SkCodecPrintf("Error: No images embedded in ico.\n");
+ *result = kInvalidInput;
return nullptr;
}
@@ -66,6 +63,7 @@
if (!dirEntryBuffer) {
SkCodecPrintf("Error: OOM allocating ICO directory for %i images.\n",
numImages);
+ *result = kInternalError;
return nullptr;
}
auto* directoryEntries = reinterpret_cast<Entry*>(dirEntryBuffer.get());
@@ -76,6 +74,7 @@
if (inputStream->read(entryBuffer, kIcoDirEntryBytes) !=
kIcoDirEntryBytes) {
SkCodecPrintf("Error: Dir entries truncated in ico.\n");
+ *result = kIncompleteInput;
return nullptr;
}
@@ -98,6 +97,9 @@
directoryEntries[i].size = size;
}
+ // Default Result, if no valid embedded codecs are found.
+ *result = kInvalidInput;
+
// It is "customary" that the embedded images will be stored in order of
// increasing offset. However, the specification does not indicate that
// they must be stored in this order, so we will not trust that this is the
@@ -141,6 +143,7 @@
if (inputStream->read(buffer.get(), size) != size) {
SkCodecPrintf("Warning: could not create embedded stream.\n");
+ *result = kIncompleteInput;
break;
}
@@ -150,10 +153,11 @@
// Check if the embedded codec is bmp or png and create the codec
SkCodec* codec = nullptr;
+ Result dummyResult;
if (SkPngCodec::IsPng((const char*) data->bytes(), data->size())) {
- codec = SkPngCodec::NewFromStream(embeddedStream.release());
+ codec = SkPngCodec::NewFromStream(embeddedStream.release(), &dummyResult);
} else {
- codec = SkBmpCodec::NewFromIco(embeddedStream.release());
+ codec = SkBmpCodec::NewFromIco(embeddedStream.release(), &dummyResult);
}
// Save a valid codec
@@ -185,8 +189,9 @@
SkEncodedInfo info = codecs->operator[](maxIndex)->getEncodedInfo();
SkColorSpace* colorSpace = codecs->operator[](maxIndex)->getInfo().colorSpace();
- // Note that stream is owned by the embedded codec, the ico does not need
- // direct access to the stream.
+ *result = kSuccess;
+ // The original stream is no longer needed, because the embedded codecs own their
+ // own streams.
return new SkIcoCodec(width, height, info, codecs.release(), sk_ref_sp(colorSpace));
}
diff --git a/src/codec/SkIcoCodec.h b/src/codec/SkIcoCodec.h
index 9f2457a..0c27934 100644
--- a/src/codec/SkIcoCodec.h
+++ b/src/codec/SkIcoCodec.h
@@ -25,7 +25,7 @@
* Creates an Ico decoder
* Reads enough of the stream to determine the image format
*/
- static SkCodec* NewFromStream(SkStream*);
+ static SkCodec* NewFromStream(SkStream*, Result*);
protected:
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 7282989..f3dbdf1 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -185,15 +185,15 @@
return iccData;
}
-bool SkJpegCodec::ReadHeader(SkStream* stream, SkCodec** codecOut, JpegDecoderMgr** decoderMgrOut,
- sk_sp<SkColorSpace> defaultColorSpace) {
+SkCodec::Result SkJpegCodec::ReadHeader(SkStream* stream, SkCodec** codecOut,
+ JpegDecoderMgr** decoderMgrOut, sk_sp<SkColorSpace> defaultColorSpace) {
// Create a JpegDecoderMgr to own all of the decompress information
std::unique_ptr<JpegDecoderMgr> decoderMgr(new JpegDecoderMgr(stream));
// libjpeg errors will be caught and reported here
if (setjmp(decoderMgr->getJmpBuf())) {
- return decoderMgr->returnFalse("ReadHeader");
+ return decoderMgr->returnFailure("ReadHeader", kInvalidInput);
}
// Initialize the decompress info and the source manager
@@ -208,15 +208,20 @@
}
// Read the jpeg header
- if (JPEG_HEADER_OK != jpeg_read_header(decoderMgr->dinfo(), true)) {
- return decoderMgr->returnFalse("ReadHeader");
+ switch (jpeg_read_header(decoderMgr->dinfo(), true)) {
+ case JPEG_HEADER_OK:
+ break;
+ case JPEG_SUSPENDED:
+ return decoderMgr->returnFailure("ReadHeader", kIncompleteInput);
+ default:
+ return decoderMgr->returnFailure("ReadHeader", kInvalidInput);
}
if (codecOut) {
// Get the encoded color type
SkEncodedInfo::Color color;
if (!decoderMgr->getEncodedColor(&color)) {
- return false;
+ return kInvalidInput;
}
// Create image info object and the codec
@@ -254,17 +259,19 @@
SkASSERT(nullptr != decoderMgrOut);
*decoderMgrOut = decoderMgr.release();
}
- return true;
+ return kSuccess;
}
-SkCodec* SkJpegCodec::NewFromStream(SkStream* stream) {
- return SkJpegCodec::NewFromStream(stream, SkColorSpace::MakeSRGB());
+SkCodec* SkJpegCodec::NewFromStream(SkStream* stream, Result* result) {
+ return SkJpegCodec::NewFromStream(stream, result, SkColorSpace::MakeSRGB());
}
-SkCodec* SkJpegCodec::NewFromStream(SkStream* stream, sk_sp<SkColorSpace> defaultColorSpace) {
+SkCodec* SkJpegCodec::NewFromStream(SkStream* stream, Result* result,
+ sk_sp<SkColorSpace> defaultColorSpace) {
std::unique_ptr<SkStream> streamDeleter(stream);
SkCodec* codec = nullptr;
- if (ReadHeader(stream, &codec, nullptr, std::move(defaultColorSpace))) {
+ *result = ReadHeader(stream, &codec, nullptr, std::move(defaultColorSpace));
+ if (kSuccess == *result) {
// Codec has taken ownership of the stream, we do not need to delete it
SkASSERT(codec);
streamDeleter.release();
@@ -347,7 +354,7 @@
bool SkJpegCodec::onRewind() {
JpegDecoderMgr* decoderMgr = nullptr;
- if (!ReadHeader(this->stream(), nullptr, &decoderMgr, nullptr)) {
+ if (kSuccess != ReadHeader(this->stream(), nullptr, &decoderMgr, nullptr)) {
return fDecoderMgr->returnFalse("onRewind");
}
SkASSERT(nullptr != decoderMgr);
diff --git a/src/codec/SkJpegCodec.h b/src/codec/SkJpegCodec.h
index 2d6822e..96228f3 100644
--- a/src/codec/SkJpegCodec.h
+++ b/src/codec/SkJpegCodec.h
@@ -29,10 +29,9 @@
/*
* Assumes IsJpeg was called and returned true
- * Creates a jpeg decoder
* Takes ownership of the stream
*/
- static SkCodec* NewFromStream(SkStream*);
+ static SkCodec* NewFromStream(SkStream*, Result*);
protected:
@@ -64,7 +63,7 @@
/*
* Allows SkRawCodec to communicate the color space from the exif data.
*/
- static SkCodec* NewFromStream(SkStream*, sk_sp<SkColorSpace> defaultColorSpace);
+ static SkCodec* NewFromStream(SkStream*, Result*, sk_sp<SkColorSpace> defaultColorSpace);
/*
* Read enough of the stream to initialize the SkJpegCodec.
@@ -88,7 +87,7 @@
* If the jpeg does not have an embedded color space, the image data should
* be tagged with this color space.
*/
- static bool ReadHeader(SkStream* stream, SkCodec** codecOut,
+ static Result ReadHeader(SkStream* stream, SkCodec** codecOut,
JpegDecoderMgr** decoderMgrOut, sk_sp<SkColorSpace> defaultColorSpace);
/*
diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp
index ea832ff..560b5bf 100644
--- a/src/codec/SkPngCodec.cpp
+++ b/src/codec/SkPngCodec.cpp
@@ -761,31 +761,30 @@
// png_structp on success.
// @param info_ptrp Optional output variable. If non-NULL, will be set to a new
// png_infop on success;
-// @return true on success, in which case the caller is responsible for calling
+// @return if kSuccess, the caller is responsible for calling
// png_destroy_read_struct(png_ptrp, info_ptrp).
-// If it returns false, the passed in fields (except stream) are unchanged.
-static bool read_header(SkStream* stream, SkPngChunkReader* chunkReader, SkCodec** outCodec,
- png_structp* png_ptrp, png_infop* info_ptrp) {
+// Otherwise, the passed in fields (except stream) are unchanged.
+static SkCodec::Result read_header(SkStream* stream, SkPngChunkReader* chunkReader,
+ SkCodec** outCodec,
+ png_structp* png_ptrp, png_infop* info_ptrp) {
// The image is known to be a PNG. Decode enough to know the SkImageInfo.
png_structp png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, nullptr,
sk_error_fn, sk_warning_fn);
if (!png_ptr) {
- return false;
+ return SkCodec::kInternalError;
}
AutoCleanPng autoClean(png_ptr, stream, chunkReader, outCodec);
png_infop info_ptr = png_create_info_struct(png_ptr);
if (info_ptr == nullptr) {
- return false;
+ return SkCodec::kInternalError;
}
autoClean.setInfoPtr(info_ptr);
- // FIXME: Could we use the return value of setjmp to specify the type of
- // error?
if (setjmp(PNG_JMPBUF(png_ptr))) {
- return false;
+ return SkCodec::kInvalidInput;
}
#ifdef PNG_READ_UNKNOWN_CHUNKS_SUPPORTED
@@ -801,7 +800,7 @@
const bool decodedBounds = autoClean.decodeBounds();
if (!decodedBounds) {
- return false;
+ return SkCodec::kIncompleteInput;
}
// On success, decodeBounds releases ownership of png_ptr and info_ptr.
@@ -816,7 +815,7 @@
if (outCodec) {
SkASSERT(*outCodec);
}
- return true;
+ return SkCodec::kSuccess;
}
void AutoCleanPng::infoCallback(size_t idatLength) {
@@ -1071,7 +1070,8 @@
png_structp png_ptr;
png_infop info_ptr;
- if (!read_header(this->stream(), fPngChunkReader.get(), nullptr, &png_ptr, &info_ptr)) {
+ if (kSuccess != read_header(this->stream(), fPngChunkReader.get(), nullptr,
+ &png_ptr, &info_ptr)) {
return false;
}
@@ -1145,16 +1145,15 @@
return INHERITED::onGetFillValue(dstInfo);
}
-SkCodec* SkPngCodec::NewFromStream(SkStream* stream, SkPngChunkReader* chunkReader) {
+SkCodec* SkPngCodec::NewFromStream(SkStream* stream, Result* result, SkPngChunkReader* chunkReader) {
std::unique_ptr<SkStream> streamDeleter(stream);
SkCodec* outCodec = nullptr;
- if (read_header(streamDeleter.get(), chunkReader, &outCodec, nullptr, nullptr)) {
+ *result = read_header(stream, chunkReader, &outCodec, nullptr, nullptr);
+ if (kSuccess == *result) {
// Codec has taken ownership of the stream.
SkASSERT(outCodec);
streamDeleter.release();
- return outCodec;
}
-
- return nullptr;
+ return outCodec;
}
diff --git a/src/codec/SkPngCodec.h b/src/codec/SkPngCodec.h
index 25a5f07..b5f9347 100644
--- a/src/codec/SkPngCodec.h
+++ b/src/codec/SkPngCodec.h
@@ -23,7 +23,8 @@
static bool IsPng(const char*, size_t);
// Assume IsPng was called and returned true.
- static SkCodec* NewFromStream(SkStream*, SkPngChunkReader* = NULL);
+ static SkCodec* NewFromStream(SkStream*, Result*,
+ SkPngChunkReader* = nullptr);
// FIXME (scroggo): Temporarily needed by AutoCleanPng.
void setIdatLength(size_t len) { fIdatLength = len; }
diff --git a/src/codec/SkRawCodec.cpp b/src/codec/SkRawCodec.cpp
index 83b7947..d23eec0 100644
--- a/src/codec/SkRawCodec.cpp
+++ b/src/codec/SkRawCodec.cpp
@@ -447,16 +447,10 @@
*/
static SkDngImage* NewFromStream(SkRawStream* stream) {
std::unique_ptr<SkDngImage> dngImage(new SkDngImage(stream));
- if (!dngImage->isTiffHeaderValid()) {
+ if (!dngImage->initFromPiex() && !dngImage->readDng()) {
return nullptr;
}
- if (!dngImage->initFromPiex()) {
- if (!dngImage->readDng()) {
- return nullptr;
- }
- }
-
return dngImage.release();
}
@@ -535,12 +529,12 @@
return fIsXtransImage;
}
-private:
// Quick check if the image contains a valid TIFF header as requested by DNG format.
- bool isTiffHeaderValid() const {
+ // Does not affect ownership of stream.
+ static bool IsTiffHeaderValid(SkRawStream* stream) {
const size_t kHeaderSize = 4;
- SkAutoSTMalloc<kHeaderSize, unsigned char> header(kHeaderSize);
- if (!fStream->read(header.get(), 0 /* offset */, kHeaderSize)) {
+ unsigned char header[kHeaderSize];
+ if (!stream->read(header, 0 /* offset */, kHeaderSize)) {
return false;
}
@@ -553,6 +547,7 @@
return 0x2A == get_endian_short(header + 2, littleEndian);
}
+private:
bool init(int width, int height, const dng_point& cfaPatternSize) {
fWidth = width;
fHeight = height;
@@ -635,7 +630,7 @@
* SkJpegCodec. If PIEX returns kFail, then the file is invalid, return nullptr. In other cases,
* fallback to create SkRawCodec for DNG images.
*/
-SkCodec* SkRawCodec::NewFromStream(SkStream* stream) {
+SkCodec* SkRawCodec::NewFromStream(SkStream* stream, Result* result) {
std::unique_ptr<SkRawStream> rawStream;
if (is_asset_stream(*stream)) {
rawStream.reset(new SkRawAssetStream(stream));
@@ -649,6 +644,7 @@
if (::piex::IsRaw(&piexStream)) {
::piex::Error error = ::piex::GetPreviewImageData(&piexStream, &imageData);
if (error == ::piex::Error::kFail) {
+ *result = kInvalidInput;
return nullptr;
}
@@ -672,17 +668,27 @@
// FIXME: one may avoid the copy of memoryStream and use the buffered rawStream.
SkMemoryStream* memoryStream =
rawStream->transferBuffer(imageData.preview.offset, imageData.preview.length);
- return memoryStream ? SkJpegCodec::NewFromStream(memoryStream, std::move(colorSpace))
- : nullptr;
+ if (!memoryStream) {
+ *result = kInvalidInput;
+ return nullptr;
+ }
+ return SkJpegCodec::NewFromStream(memoryStream, result, std::move(colorSpace));
}
}
+ if (!SkDngImage::IsTiffHeaderValid(rawStream.get())) {
+ *result = kUnimplemented;
+ return nullptr;
+ }
+
// Takes the ownership of the rawStream.
std::unique_ptr<SkDngImage> dngImage(SkDngImage::NewFromStream(rawStream.release()));
if (!dngImage) {
+ *result = kInvalidInput;
return nullptr;
}
+ *result = kSuccess;
return new SkRawCodec(dngImage.release());
}
diff --git a/src/codec/SkRawCodec.h b/src/codec/SkRawCodec.h
index d2ef921..5ed7211 100644
--- a/src/codec/SkRawCodec.h
+++ b/src/codec/SkRawCodec.h
@@ -28,7 +28,7 @@
* Creates a RAW decoder
* Takes ownership of the stream
*/
- static SkCodec* NewFromStream(SkStream*);
+ static SkCodec* NewFromStream(SkStream*, Result*);
~SkRawCodec() override;
diff --git a/src/codec/SkWbmpCodec.cpp b/src/codec/SkWbmpCodec.cpp
index aab0a41..9a0ed6e 100644
--- a/src/codec/SkWbmpCodec.cpp
+++ b/src/codec/SkWbmpCodec.cpp
@@ -148,12 +148,16 @@
return read_header(&stream, nullptr);
}
-SkCodec* SkWbmpCodec::NewFromStream(SkStream* stream) {
+SkCodec* SkWbmpCodec::NewFromStream(SkStream* stream, Result* result) {
std::unique_ptr<SkStream> streamDeleter(stream);
SkISize size;
if (!read_header(stream, &size)) {
+ // This already succeeded in IsWbmp, so this stream was corrupted in/
+ // after rewind.
+ *result = kCouldNotRewind;
return nullptr;
}
+ *result = kSuccess;
SkEncodedInfo info = SkEncodedInfo::Make(SkEncodedInfo::kGray_Color,
SkEncodedInfo::kOpaque_Alpha, 1);
return new SkWbmpCodec(size.width(), size.height(), info, streamDeleter.release());
diff --git a/src/codec/SkWbmpCodec.h b/src/codec/SkWbmpCodec.h
index e8a6e40..f81b428 100644
--- a/src/codec/SkWbmpCodec.h
+++ b/src/codec/SkWbmpCodec.h
@@ -21,7 +21,7 @@
* Creates a wbmp codec
* Takes ownership of the stream
*/
- static SkCodec* NewFromStream(SkStream*);
+ static SkCodec* NewFromStream(SkStream*, Result*);
protected:
SkEncodedImageFormat onGetEncodedFormat() const override;
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index 096d505..8d3d8cc 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -43,7 +43,7 @@
// Parse headers of RIFF container, and check for valid Webp (VP8) content.
// Returns an SkWebpCodec on success
-SkCodec* SkWebpCodec::NewFromStream(SkStream* stream) {
+SkCodec* SkWebpCodec::NewFromStream(SkStream* stream, Result* result) {
std::unique_ptr<SkStream> streamDeleter(stream);
// Webp demux needs a contiguous data buffer.
@@ -62,9 +62,19 @@
// pointer in |webpData| to remain valid. This works because the pointer remains valid
// until the SkData is freed.
WebPData webpData = { data->bytes(), data->size() };
- SkAutoTCallVProc<WebPDemuxer, WebPDemuxDelete> demux(WebPDemuxPartial(&webpData, nullptr));
- if (nullptr == demux) {
- return nullptr;
+ WebPDemuxState state;
+ SkAutoTCallVProc<WebPDemuxer, WebPDemuxDelete> demux(WebPDemuxPartial(&webpData, &state));
+ switch (state) {
+ case WEBP_DEMUX_PARSE_ERROR:
+ *result = kInvalidInput;
+ return nullptr;
+ case WEBP_DEMUX_PARSING_HEADER:
+ *result = kIncompleteInput;
+ return nullptr;
+ case WEBP_DEMUX_PARSED_HEADER:
+ case WEBP_DEMUX_DONE:
+ SkASSERT(demux);
+ break;
}
const int width = WebPDemuxGetI(demux, WEBP_FF_CANVAS_WIDTH);
@@ -73,11 +83,9 @@
// Sanity check for image size that's about to be decoded.
{
const int64_t size = sk_64_mul(width, height);
- if (!sk_64_isS32(size)) {
- return nullptr;
- }
// now check that if we are 4-bytes per pixel, we also don't overflow
- if (sk_64_asS32(size) > (0x7FFFFFFF >> 2)) {
+ if (!sk_64_isS32(size) || sk_64_asS32(size) > (0x7FFFFFFF >> 2)) {
+ *result = kInvalidInput;
return nullptr;
}
}
@@ -96,13 +104,21 @@
WebPIterator frame;
SkAutoTCallVProc<WebPIterator, WebPDemuxReleaseIterator> autoFrame(&frame);
if (!WebPDemuxGetFrame(demux, 1, &frame)) {
+ *result = kIncompleteInput;
return nullptr;
}
WebPBitstreamFeatures features;
- VP8StatusCode status = WebPGetFeatures(frame.fragment.bytes, frame.fragment.size, &features);
- if (VP8_STATUS_OK != status) {
- return nullptr;
+ switch (WebPGetFeatures(frame.fragment.bytes, frame.fragment.size, &features)) {
+ case VP8_STATUS_OK:
+ break;
+ case VP8_STATUS_SUSPENDED:
+ case VP8_STATUS_NOT_ENOUGH_DATA:
+ *result = kIncompleteInput;
+ return nullptr;
+ default:
+ *result = kInvalidInput;
+ return nullptr;
}
const bool hasAlpha = SkToBool(frame.has_alpha)
@@ -139,14 +155,15 @@
}
break;
default:
+ *result = kInvalidInput;
return nullptr;
}
+ *result = kSuccess;
SkEncodedInfo info = SkEncodedInfo::Make(color, alpha, 8);
- SkWebpCodec* codecOut = new SkWebpCodec(width, height, info, std::move(colorSpace),
- streamDeleter.release(), demux.release(),
- std::move(data));
- return codecOut;
+ return new SkWebpCodec(width, height, info, std::move(colorSpace),
+ streamDeleter.release(), demux.release(),
+ std::move(data));
}
SkISize SkWebpCodec::onGetScaledDimensions(float desiredScale) const {
diff --git a/src/codec/SkWebpCodec.h b/src/codec/SkWebpCodec.h
index c911ebd..e06d097 100644
--- a/src/codec/SkWebpCodec.h
+++ b/src/codec/SkWebpCodec.h
@@ -28,7 +28,7 @@
class SkWebpCodec final : public SkCodec {
public:
// Assumes IsWebp was called and returned true.
- static SkCodec* NewFromStream(SkStream*);
+ static SkCodec* NewFromStream(SkStream*, Result*);
static bool IsWebp(const void*, size_t);
protected:
Result onGetPixels(const SkImageInfo&, void*, size_t, const Options&, int*) override;