Make SkStream readers report failure.
This also fixes an issue noticed while making this change where
SkFontDescriptor improperly round trips negative axis values.
Change-Id: Iacc5929a185659dcacc18c802c4908e4f34c6899
Reviewed-on: https://skia-review.googlesource.com/128341
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
diff --git a/src/core/SkFontDescriptor.cpp b/src/core/SkFontDescriptor.cpp
index 14b0318..ccc19eb 100644
--- a/src/core/SkFontDescriptor.cpp
+++ b/src/core/SkFontDescriptor.cpp
@@ -11,6 +11,8 @@
#include "SkData.h"
enum {
+ kInvalid = 0x00,
+
// these must match the sfnt 'name' enums
kFontFamilyName = 0x01,
kFullName = 0x04,
@@ -18,40 +20,45 @@
// These count backwards from 0xFF, so as not to collide with the SFNT
// defines for names in its 'name' table.
- kFontAxes = 0xFC,
+ kFontAxes = 0xFB,
+ kFontAxes_bad = 0xFC, // Broken negative axes, remove when MIN_PICTURE_VERSION > 62.
kFontIndex = 0xFD,
kSentinel = 0xFF,
};
SkFontDescriptor::SkFontDescriptor() { }
-static void read_string(SkStream* stream, SkString* string) {
- const uint32_t length = SkToU32(stream->readPackedUInt());
+static bool SK_WARN_UNUSED_RESULT read_string(SkStream* stream, SkString* string) {
+ size_t length;
+ if (!stream->readPackedUInt(&length)) { return false; }
if (length > 0) {
string->resize(length);
- stream->read(string->writable_str(), length);
+ if (stream->read(string->writable_str(), length) != length) { return false; }
}
+ return true;
}
-static void write_string(SkWStream* stream, const SkString& string, uint32_t id) {
- if (!string.isEmpty()) {
- stream->writePackedUInt(id);
- stream->writePackedUInt(string.size());
- stream->write(string.c_str(), string.size());
- }
+static bool write_string(SkWStream* stream, const SkString& string, uint32_t id) {
+ if (string.isEmpty()) { return true; }
+ return stream->writePackedUInt(id) &&
+ stream->writePackedUInt(string.size()) &&
+ stream->write(string.c_str(), string.size());
}
-static size_t read_uint(SkStream* stream) {
- return stream->readPackedUInt();
+static bool write_uint(SkWStream* stream, size_t n, uint32_t id) {
+ return stream->writePackedUInt(id) &&
+ stream->writePackedUInt(n);
}
-static void write_uint(SkWStream* stream, size_t n, uint32_t id) {
- stream->writePackedUInt(id);
- stream->writePackedUInt(n);
+static size_t SK_WARN_UNUSED_RESULT read_id(SkStream* stream) {
+ size_t i;
+ if (!stream->readPackedUInt(&i)) { return kInvalid; }
+ return i;
}
bool SkFontDescriptor::Deserialize(SkStream* stream, SkFontDescriptor* result) {
- size_t styleBits = stream->readPackedUInt();
+ size_t styleBits;
+ if (!stream->readPackedUInt(&styleBits)) { return false; }
result->fStyle = SkFontStyle((styleBits >> 16) & 0xFFFF,
(styleBits >> 8 ) & 0xFF,
static_cast<SkFontStyle::Slant>(styleBits & 0xFF));
@@ -59,26 +66,35 @@
SkAutoSTMalloc<4, SkFixed> axis;
size_t axisCount = 0;
size_t index = 0;
- for (size_t id; (id = stream->readPackedUInt()) != kSentinel;) {
+ for (size_t id; (id = read_id(stream)) != kSentinel;) {
switch (id) {
case kFontFamilyName:
- read_string(stream, &result->fFamilyName);
+ if (!read_string(stream, &result->fFamilyName)) { return false; }
break;
case kFullName:
- read_string(stream, &result->fFullName);
+ if (!read_string(stream, &result->fFullName)) { return false; }
break;
case kPostscriptName:
- read_string(stream, &result->fPostscriptName);
+ if (!read_string(stream, &result->fPostscriptName)) { return false; }
break;
case kFontAxes:
- axisCount = read_uint(stream);
+ if (!stream->readPackedUInt(&axisCount)) { return false; }
axis.reset(axisCount);
for (size_t i = 0; i < axisCount; ++i) {
- axis[i] = read_uint(stream);
+ if (!stream->readS32(&axis[i])) { return false; }
+ }
+ break;
+ case kFontAxes_bad:
+ if (!stream->readPackedUInt(&axisCount)) { return false; }
+ axis.reset(axisCount);
+ for (size_t i = 0; i < axisCount; ++i) {
+ size_t packedAxis;
+ if (!stream->readPackedUInt(&packedAxis)) { return false; }
+ axis[i] = packedAxis;
}
break;
case kFontIndex:
- index = read_uint(stream);
+ if (!stream->readPackedUInt(&index)) { return false; }
break;
default:
SkDEBUGFAIL("Unknown id used by a font descriptor");
@@ -86,16 +102,16 @@
}
}
- size_t length = stream->readPackedUInt();
+ size_t length;
+ if (!stream->readPackedUInt(&length)) { return false; }
if (length > 0) {
sk_sp<SkData> data(SkData::MakeUninitialized(length));
- if (stream->read(data->writable_data(), length) == length) {
- result->fFontData = skstd::make_unique<SkFontData>(
- SkMemoryStream::Make(std::move(data)), index, axis, axisCount);
- } else {
+ if (stream->read(data->writable_data(), length) != length) {
SkDEBUGFAIL("Could not read font data");
return false;
}
+ result->fFontData = skstd::make_unique<SkFontData>(
+ SkMemoryStream::Make(std::move(data)), index, axis, axisCount);
}
return true;
}
@@ -114,7 +130,7 @@
if (fFontData->getAxisCount()) {
write_uint(stream, fFontData->getAxisCount(), kFontAxes);
for (int i = 0; i < fFontData->getAxisCount(); ++i) {
- stream->writePackedUInt(fFontData->getAxis()[i]);
+ stream->write32(fFontData->getAxis()[i]);
}
}
}
diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp
index 85b22ac..5993e49 100644
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -80,24 +80,25 @@
SkPictInfo info;
SkASSERT(sizeof(kMagic) == sizeof(info.fMagic));
- if (!stream->read(&info.fMagic, sizeof(kMagic))) {
+ if (stream->read(&info.fMagic, sizeof(kMagic)) != sizeof(kMagic)) {
return false;
}
- info.setVersion( stream->readU32());
- info.fCullRect.fLeft = stream->readScalar();
- info.fCullRect.fTop = stream->readScalar();
- info.fCullRect.fRight = stream->readScalar();
- info.fCullRect.fBottom = stream->readScalar();
+ uint32_t version;
+ if (!stream->readU32(&version)) { return false; }
+ info.setVersion(version);
+ if (!stream->readScalar(&info.fCullRect.fLeft )) { return false; }
+ if (!stream->readScalar(&info.fCullRect.fTop )) { return false; }
+ if (!stream->readScalar(&info.fCullRect.fRight )) { return false; }
+ if (!stream->readScalar(&info.fCullRect.fBottom)) { return false; }
if (info.getVersion() < SkReadBuffer::kRemoveHeaderFlags_Version) {
- (void)stream->readU32(); // used to be flags
+ if (!stream->readU32(nullptr)) { return false; }
}
- if (IsValidPictInfo(info)) {
- if (pInfo) { *pInfo = info; }
- return true;
- }
- return false;
+ if (!IsValidPictInfo(info)) { return false; }
+
+ if (pInfo) { *pInfo = info; }
+ return true;
}
bool SkPicture_StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) {
return SkPicture::StreamIsSKP(stream, pInfo);
@@ -171,15 +172,17 @@
procs = *procsPtr;
}
- switch (stream->readU8()) {
+ uint8_t trailingStreamByteAfterPictInfo;
+ if (!stream->readU8(&trailingStreamByteAfterPictInfo)) { return nullptr; }
+ switch (trailingStreamByteAfterPictInfo) {
case kPictureData_TrailingStreamByteAfterPictInfo: {
std::unique_ptr<SkPictureData> data(
SkPictureData::CreateFromStream(stream, info, procs, typefaces));
return Forwardport(info, data.get(), nullptr);
}
case kCustom_TrailingStreamByteAfterPictInfo: {
- int32_t ssize = stream->readS32();
- if (ssize >= 0 || !procs.fPictureProc) {
+ int32_t ssize;
+ if (!stream->readS32(&ssize) || ssize >= 0 || !procs.fPictureProc) {
return nullptr;
}
size_t size = sk_negate_to_size_t(ssize);
diff --git a/src/core/SkPictureData.cpp b/src/core/SkPictureData.cpp
index efd4183..6be36dd 100644
--- a/src/core/SkPictureData.cpp
+++ b/src/core/SkPictureData.cpp
@@ -276,11 +276,12 @@
break;
case SK_PICT_FACTORY_TAG: {
SkASSERT(!haveBuffer);
- size = stream->readU32();
+ if (!stream->readU32(&size)) { return false; }
fFactoryPlayback = skstd::make_unique<SkFactoryPlayback>(size);
for (size_t i = 0; i < size; i++) {
SkString str;
- const size_t len = stream->readPackedUInt();
+ size_t len;
+ if (!stream->readPackedUInt(&len)) { return false; }
str.resize(len);
if (stream->read(str.writable_str(), len) != len) {
return false;
@@ -480,12 +481,14 @@
const SkDeserialProcs& procs,
SkTypefacePlayback* topLevelTFPlayback) {
for (;;) {
- uint32_t tag = stream->readU32();
+ uint32_t tag;
+ if (!stream->readU32(&tag)) { return false; }
if (SK_PICT_EOF_TAG == tag) {
break;
}
- uint32_t size = stream->readU32();
+ uint32_t size;
+ if (!stream->readU32(&size)) { return false; }
if (!this->parseStreamTag(stream, tag, size, procs, topLevelTFPlayback)) {
return false; // we're invalid
}
diff --git a/src/core/SkStream.cpp b/src/core/SkStream.cpp
index dfdc257..57fd7f5 100644
--- a/src/core/SkStream.cpp
+++ b/src/core/SkStream.cpp
@@ -20,50 +20,43 @@
///////////////////////////////////////////////////////////////////////////////
-int8_t SkStream::readS8() {
- int8_t value;
- SkDEBUGCODE(size_t len =) this->read(&value, 1);
- SkASSERT(1 == len);
- return value;
+bool SkStream::readS8(int8_t* i) {
+ return this->read(i, sizeof(*i)) == sizeof(*i);
}
-int16_t SkStream::readS16() {
- int16_t value;
- SkDEBUGCODE(size_t len =) this->read(&value, 2);
- SkASSERT(2 == len);
- return value;
+bool SkStream::readS16(int16_t* i) {
+ return this->read(i, sizeof(*i)) == sizeof(*i);
}
-int32_t SkStream::readS32() {
- int32_t value;
- SkDEBUGCODE(size_t len =) this->read(&value, 4);
- SkASSERT(4 == len);
- return value;
+bool SkStream::readS32(int32_t* i) {
+ return this->read(i, sizeof(*i)) == sizeof(*i);
}
-SkScalar SkStream::readScalar() {
- SkScalar value;
- SkDEBUGCODE(size_t len =) this->read(&value, sizeof(SkScalar));
- SkASSERT(sizeof(SkScalar) == len);
- return value;
+bool SkStream::readScalar(SkScalar* i) {
+ return this->read(i, sizeof(*i)) == sizeof(*i);
}
#define SK_MAX_BYTE_FOR_U8 0xFD
#define SK_BYTE_SENTINEL_FOR_U16 0xFE
#define SK_BYTE_SENTINEL_FOR_U32 0xFF
-size_t SkStream::readPackedUInt() {
+bool SkStream::readPackedUInt(size_t* i) {
uint8_t byte;
if (!this->read(&byte, 1)) {
- return 0;
+ return false;
}
if (SK_BYTE_SENTINEL_FOR_U16 == byte) {
- return this->readU16();
+ uint16_t i16;
+ if (!this->readU16(&i16)) { return false; }
+ *i = i16;
} else if (SK_BYTE_SENTINEL_FOR_U32 == byte) {
- return this->readU32();
+ uint32_t i32;
+ if (!this->readU32(&i32)) { return false; }
+ *i = i32;
} else {
- return byte;
+ *i = byte;
}
+ return true;
}
//////////////////////////////////////////////////////////////////////////////////////
diff --git a/src/utils/SkMultiPictureDocument.cpp b/src/utils/SkMultiPictureDocument.cpp
index b09db06..493e6d4 100644
--- a/src/utils/SkMultiPictureDocument.cpp
+++ b/src/utils/SkMultiPictureDocument.cpp
@@ -110,16 +110,16 @@
stream = nullptr;
return 0;
}
- uint32_t versionNumber = stream->readU32();
- if (versionNumber != kVersion) {
+ uint32_t versionNumber;
+ if (!stream->readU32(&versionNumber) || versionNumber != kVersion) {
return 0;
}
- uint32_t pageCount = stream->readU32();
- if (pageCount > INT_MAX) {
+ uint32_t pageCount;
+ if (!stream->readU32(&pageCount) || pageCount > INT_MAX) {
return 0;
}
// leave stream position right here.
- return (int)pageCount;
+ return SkTo<int>(pageCount);
}
bool SkMultiPictureDocumentReadPageSizes(SkStreamSeekable* stream,