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>

5 files changed, 93 insertions, 78 deletions

diff --git a/src/core/SkFontDescriptor.cpp b/src/core/SkFontDescriptor.cpp
index 14b0318def..ccc19eba2d 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 @@ enum {
 
     // 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 @@ bool SkFontDescriptor::Deserialize(SkStream* stream, SkFontDescriptor* result) {
     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) {
+                    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) {
-                    axis[i] = read_uint(stream);
+                    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 @@ bool SkFontDescriptor::Deserialize(SkStream* stream, SkFontDescriptor* result) {
         }
     }
 
-    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 @@ void SkFontDescriptor::serialize(SkWStream* stream) {
         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 85b22ac20f..5993e49972 100644
--- a/src/core/SkPicture.cpp
+++ b/src/core/SkPicture.cpp
@@ -80,24 +80,25 @@ bool SkPicture::StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) {
 
     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 @@ sk_sp<SkPicture> SkPicture::MakeFromStream(SkStream* stream, const SkDeserialPro
         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 efd41837d9..6be36dd1d2 100644
--- a/src/core/SkPictureData.cpp
+++ b/src/core/SkPictureData.cpp
@@ -276,11 +276,12 @@ bool SkPictureData::parseStreamTag(SkStream* stream,
             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 @@ bool SkPictureData::parseStream(SkStream* stream,
                                 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 dfdc257ff6..57fd7f5d02 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 b09db06f11..493e6d4cfc 100644
--- a/src/utils/SkMultiPictureDocument.cpp
+++ b/src/utils/SkMultiPictureDocument.cpp
@@ -110,16 +110,16 @@ int SkMultiPictureDocumentReadPageCount(SkStreamSeekable* stream) {
         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,