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/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 {