upgrade SkReadBuffer to always validate

Bug: skia:
Change-Id: I054560b66c6cde346d939015326d8547879d2c4b
Reviewed-on: https://skia-review.googlesource.com/81160
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>

16 files changed, 238 insertions, 482 deletions

diff --git a/fuzz/FuzzCanvas.cpp b/fuzz/FuzzCanvas.cpp
index a3737dc332..618f7ac934 100644
--- a/fuzz/FuzzCanvas.cpp
+++ b/fuzz/FuzzCanvas.cpp
@@ -52,6 +52,7 @@
 #include "SkPaintImageFilter.h"
 #include "SkPerlinNoiseShader.h"
 #include "SkPictureImageFilter.h"
+#include "SkReadBuffer.h"
 #include "SkRRectsGaussianEdgeMaskFilter.h"
 #include "SkTableColorFilter.h"
 #include "SkTextBlob.h"
@@ -60,7 +61,6 @@
 
 // SRC
 #include "SkUtils.h"
-#include "SkValidatingReadBuffer.h"
 
 #if SK_SUPPORT_GPU
 #include "GrContextFactory.h"
@@ -1771,8 +1771,8 @@ DEF_FUZZ(RasterN32CanvasViaSerialization, fuzz) {
     if (!pic) { fuzz->signalBug(); }
     sk_sp<SkData> data = pic->serialize();
     if (!data) { fuzz->signalBug(); }
-    SkValidatingReadBuffer vrb(data->data(), data->size());
-    auto deserialized = SkPicture::MakeFromBuffer(vrb);
+    SkReadBuffer rb(data->data(), data->size());
+    auto deserialized = SkPicture::MakeFromBuffer(rb);
     if (!deserialized) { fuzz->signalBug(); }
     auto surface = SkSurface::MakeRasterN32Premul(kCanvasSize.width(), kCanvasSize.height());
     SkASSERT(surface && surface->getCanvas());
diff --git a/fuzz/fuzz.cpp b/fuzz/fuzz.cpp
index ae9f0a4b01..f2f483ac72 100644
--- a/fuzz/fuzz.cpp
+++ b/fuzz/fuzz.cpp
@@ -20,11 +20,11 @@
 #include "SkPaint.h"
 #include "SkPath.h"
 #include "SkPicture.h"
+#include "SkReadBuffer.h"
 #include "SkRegion.h"
 #include "SkStream.h"
 #include "SkSurface.h"
 #include "SkTextBlob.h"
-#include "SkValidatingReadBuffer.h"
 
 #if SK_SUPPORT_GPU
 #include "SkSLCompiler.h"
@@ -479,7 +479,7 @@ static void fuzz_color_deserialize(sk_sp<SkData> bytes) {
 
 static void fuzz_path_deserialize(sk_sp<SkData> bytes) {
     SkPath path;
-    SkValidatingReadBuffer buf(bytes->data(), bytes->size());
+    SkReadBuffer buf(bytes->data(), bytes->size());
     buf.readPath(&path);
     if (!buf.isValid()) {
         SkDebugf("[terminated] Couldn't deserialize SkPath.\n");
@@ -512,7 +512,7 @@ static void fuzz_region_deserialize(sk_sp<SkData> bytes) {
 }
 
 static void fuzz_textblob_deserialize(sk_sp<SkData> bytes) {
-    SkValidatingReadBuffer buf(bytes->data(), bytes->size());
+    SkReadBuffer buf(bytes->data(), bytes->size());
     auto tb = SkTextBlob::MakeFromBuffer(buf);
     if (!buf.isValid()) {
         SkDebugf("[terminated] Couldn't deserialize SkTextBlob.\n");
diff --git a/gn/core.gni b/gn/core.gni
index 5ef7da60d0..a13b4d7109 100644
--- a/gn/core.gni
+++ b/gn/core.gni
@@ -327,8 +327,6 @@ skia_core_sources = [
   "$_src/core/SkUnPreMultiply.cpp",
   "$_src/core/SkUtils.cpp",
   "$_src/core/SkUtils.h",
-  "$_src/core/SkValidatingReadBuffer.cpp",
-  "$_src/core/SkValidatingReadBuffer.h",
   "$_src/core/SkValidationUtils.h",
   "$_src/core/SkVertices.cpp",
   "$_src/core/SkVertState.cpp",
diff --git a/include/core/SkFlattenable.h b/include/core/SkFlattenable.h
index 40cbaa5d0d..b0bfbe23e7 100644
--- a/include/core/SkFlattenable.h
+++ b/include/core/SkFlattenable.h
@@ -10,10 +10,12 @@
 
 #include "SkRefCnt.h"
 
+class SkData;
 class SkReadBuffer;
 class SkWriteBuffer;
 
-class SkPrivateEffectInitializer;
+struct SkSerialProcs;
+struct SkDeserialProcs;
 
 /*
  *  Flattening is straight-forward:
@@ -113,9 +115,18 @@ public:
     /**
      *  Override this if your subclass needs to record data that it will need to recreate itself
      *  from its CreateProc (returned by getFactory()).
+     *
+     *  DEPRECATED public : will move to protected ... use serialize() instead
      */
     virtual void flatten(SkWriteBuffer&) const {}
 
+    //
+    // public ways to serialize / deserialize
+    //
+    sk_sp<SkData> serialize(const SkSerialProcs* = nullptr) const;
+    static sk_sp<SkFlattenable> Deserialize(Type, const void* data, size_t length,
+                                            const SkDeserialProcs* procs = nullptr);
+
 protected:
     class PrivateInitializer {
     public:
diff --git a/include/core/SkFlattenableSerialization.h b/include/core/SkFlattenableSerialization.h
index c6fd53dbe3..f6525cb217 100644
--- a/include/core/SkFlattenableSerialization.h
+++ b/include/core/SkFlattenableSerialization.h
@@ -13,6 +13,8 @@
 class SkData;
 class SkImageFilter;
 
+// DEPRECATED -- these can all be called directly using SkFlattenable.h
+
 SK_API SkData* SkValidatingSerializeFlattenable(SkFlattenable*);
 SK_API SkFlattenable* SkValidatingDeserializeFlattenable(const void* data, size_t size,
                                                          SkFlattenable::Type type);
diff --git a/src/core/SkFlattenable.cpp b/src/core/SkFlattenable.cpp
index e89dae5c76..c975023367 100644
--- a/src/core/SkFlattenable.cpp
+++ b/src/core/SkFlattenable.cpp
@@ -121,3 +121,26 @@ const char* SkFlattenable::FactoryToName(Factory fact) {
     }
     return nullptr;
 }
+
+///////////////////////////////////////////////////////////////////////////////////////////////////
+
+sk_sp<SkData> SkFlattenable::serialize(const SkSerialProcs* procs) const {
+    SkBinaryWriteBuffer writer;
+    if (procs) {
+        writer.setSerialProcs(*procs);
+    }
+    writer.writeFlattenable(this);
+    size_t size = writer.bytesWritten();
+    auto data = SkData::MakeUninitialized(size);
+    writer.writeToMemory(data->writable_data());
+    return data;
+}
+
+sk_sp<SkFlattenable> SkFlattenable::Deserialize(SkFlattenable::Type type, const void* data,
+                                                size_t size, const SkDeserialProcs* procs) {
+    SkReadBuffer buffer(data, size);
+    if (procs) {
+        buffer.setDeserialProcs(*procs);
+    }
+    return sk_sp<SkFlattenable>(buffer.readFlattenable(type));
+}
diff --git a/src/core/SkFlattenableSerialization.cpp b/src/core/SkFlattenableSerialization.cpp
index e72b4c5348..3976010fb2 100644
--- a/src/core/SkFlattenableSerialization.cpp
+++ b/src/core/SkFlattenableSerialization.cpp
@@ -6,27 +6,19 @@
  */
 
 #include "SkFlattenableSerialization.h"
-
 #include "SkData.h"
-#include "SkValidatingReadBuffer.h"
-#include "SkWriteBuffer.h"
+#include "SkImageFilter.h"
 
 SkData* SkValidatingSerializeFlattenable(SkFlattenable* flattenable) {
-    SkBinaryWriteBuffer writer;
-    writer.writeFlattenable(flattenable);
-    size_t size = writer.bytesWritten();
-    auto data = SkData::MakeUninitialized(size);
-    writer.writeToMemory(data->writable_data());
-    return data.release();
+    return flattenable->serialize().release();
 }
 
 SkFlattenable* SkValidatingDeserializeFlattenable(const void* data, size_t size,
                                                   SkFlattenable::Type type) {
-    SkValidatingReadBuffer buffer(data, size);
-    return buffer.readFlattenable(type);
+    return SkFlattenable::Deserialize(type, data, size).release();
 }
 
 sk_sp<SkImageFilter> SkValidatingDeserializeImageFilter(const void* data, size_t size) {
-    return sk_sp<SkImageFilter>((SkImageFilter*)SkValidatingDeserializeFlattenable(
-                                data, size, SkImageFilter::GetFlattenableType()));
+    auto flat = SkFlattenable::Deserialize(SkFlattenable::kSkImageFilter_Type, data, size);
+    return sk_sp<SkImageFilter>(static_cast<SkImageFilter*>(flat.release()));
 }
diff --git a/src/core/SkPictureData.cpp b/src/core/SkPictureData.cpp
index 24e161f191..a86aee2fa0 100644
--- a/src/core/SkPictureData.cpp
+++ b/src/core/SkPictureData.cpp
@@ -448,7 +448,6 @@ bool SkPictureData::parseStreamTag(SkStream* stream,
                 return false;
             }
 
-            /* Should we use SkValidatingReadBuffer instead? */
             SkReadBuffer buffer(storage.get(), size);
             buffer.setFlags(pictInfoFlagsToReadBufferFlags(fInfo.fFlags));
             buffer.setVersion(fInfo.getVersion());
diff --git a/src/core/SkReadBuffer.cpp b/src/core/SkReadBuffer.cpp
index b1d7808dcb..b6d0d14f48 100644
--- a/src/core/SkReadBuffer.cpp
+++ b/src/core/SkReadBuffer.cpp
@@ -11,6 +11,7 @@
 #include "SkImageDeserializer.h"
 #include "SkImageGenerator.h"
 #include "SkMakeUnique.h"
+#include "SkMatrixPriv.h"
 #include "SkReadBuffer.h"
 #include "SkStream.h"
 #include "SkTypeface.h"
@@ -97,55 +98,101 @@ SkReadBuffer::~SkReadBuffer() {
     sk_free(fMemoryPtr);
 }
 
+void SkReadBuffer::setInvalid() {
+    if (!fError) {
+        // When an error is found, send the read cursor to the end of the stream
+        fReader.skip(fReader.available());
+        fError = true;
+    }
+}
+
+const void* SkReadBuffer::skip(size_t size) {
+    size_t inc = SkAlign4(size);
+    this->validate(inc >= size);
+    const void* addr = fReader.peek();
+    this->validate(IsPtrAlign4(addr) && fReader.isAvailable(inc));
+    if (fError) {
+        return nullptr;
+    }
+
+    fReader.skip(size);
+    return addr;
+}
+
 void SkReadBuffer::setDeserialProcs(const SkDeserialProcs& procs) {
     fProcs = procs;
 }
 
 bool SkReadBuffer::readBool() {
-    return fReader.readBool();
+    uint32_t value = this->readInt();
+    // Boolean value should be either 0 or 1
+    this->validate(!(value & ~1));
+    return value != 0;
 }
 
 SkColor SkReadBuffer::readColor() {
-    return fReader.readInt();
+    return this->readInt();
 }
 
 int32_t SkReadBuffer::readInt() {
-    return fReader.readInt();
+    const size_t inc = sizeof(int32_t);
+    this->validate(IsPtrAlign4(fReader.peek()) && fReader.isAvailable(inc));
+    return fError ? 0 : fReader.readInt();
 }
 
 SkScalar SkReadBuffer::readScalar() {
-    return fReader.readScalar();
+    const size_t inc = sizeof(SkScalar);
+    this->validate(IsPtrAlign4(fReader.peek()) && fReader.isAvailable(inc));
+    return fError ? 0 : fReader.readScalar();
 }
 
 uint32_t SkReadBuffer::readUInt() {
-    return fReader.readU32();
+    return this->readInt();
 }
 
 int32_t SkReadBuffer::read32() {
-    return fReader.readInt();
+    return this->readInt();
+}
+
+uint8_t SkReadBuffer::peekByte() {
+    if (fReader.available() <= 0) {
+        fError = true;
+        return 0;
+    }
+    return *((uint8_t*) fReader.peek());
 }
 
 bool SkReadBuffer::readPad32(void* buffer, size_t bytes) {
-    if (!fReader.isAvailable(bytes)) {
+    if (!this->validate(fReader.isAvailable(bytes))) {
         return false;
     }
     fReader.read(buffer, bytes);
     return true;
 }
 
-uint8_t SkReadBuffer::peekByte() {
-    SkASSERT(fReader.available() > 0);
-    return *((uint8_t*) fReader.peek());
-}
-
 void SkReadBuffer::readString(SkString* string) {
-    size_t len;
-    const char* strContents = fReader.readString(&len);
-    string->set(strContents, len);
+    const size_t len = this->readUInt();
+    const void* ptr = fReader.peek();
+    const char* cptr = (const char*)ptr;
+
+    // skip over the string + '\0' and then pad to a multiple of 4
+    const size_t alignedSize = SkAlign4(len + 1);
+    this->skip(alignedSize);
+    if (!fError) {
+        this->validate(cptr[len] == '\0');
+    }
+    if (!fError) {
+        string->set(cptr, len);
+    }
 }
 
 void SkReadBuffer::readColor4f(SkColor4f* color) {
-    memcpy(color, fReader.skip(sizeof(SkColor4f)), sizeof(SkColor4f));
+    const void* ptr = this->skip(sizeof(SkColor4f));
+    if (!fError) {
+        memcpy(color, ptr, sizeof(SkColor4f));
+    } else {
+        *color = {0, 0, 0, 0};
+    }
 }
 
 void SkReadBuffer::readPoint(SkPoint* point) {
@@ -160,39 +207,74 @@ void SkReadBuffer::readPoint3(SkPoint3* point) {
 }
 
 void SkReadBuffer::readMatrix(SkMatrix* matrix) {
-    fReader.readMatrix(matrix);
+    size_t size = 0;
+    if (!fError) {
+        size = SkMatrixPriv::ReadFromMemory(matrix, fReader.peek(), fReader.available());
+        this->validate((SkAlign4(size) == size) && (0 != size));
+    }
+    if (!fError) {
+        (void)this->skip(size);
+    }
 }
 
 void SkReadBuffer::readIRect(SkIRect* rect) {
-    memcpy(rect, fReader.skip(sizeof(SkIRect)), sizeof(SkIRect));
+    const void* ptr = this->skip(sizeof(SkIRect));
+    if (!fError) {
+        memcpy(rect, ptr, sizeof(SkIRect));
+    } else {
+        rect->setEmpty();
+    }
 }
 
 void SkReadBuffer::readRect(SkRect* rect) {
-    memcpy(rect, fReader.skip(sizeof(SkRect)), sizeof(SkRect));
+    const void* ptr = this->skip(sizeof(SkRect));
+    if (!fError) {
+        memcpy(rect, ptr, sizeof(SkRect));
+    } else {
+        rect->setEmpty();
+    }
 }
 
 void SkReadBuffer::readRRect(SkRRect* rrect) {
-    fReader.readRRect(rrect);
+    if (!this->validate(fReader.readRRect(rrect))) {
+        rrect->setEmpty();
+    }
 }
 
 void SkReadBuffer::readRegion(SkRegion* region) {
-    fReader.readRegion(region);
+    size_t size = 0;
+    if (!fError) {
+        size = region->readFromMemory(fReader.peek(), fReader.available());
+        this->validate((SkAlign4(size) == size) && (0 != size));
+    }
+    if (!fError) {
+        (void)this->skip(size);
+    }
 }
 
 void SkReadBuffer::readPath(SkPath* path) {
-    fReader.readPath(path);
+    size_t size = 0;
+    if (!fError) {
+        size = path->readFromMemory(fReader.peek(), fReader.available());
+        this->validate((SkAlign4(size) == size) && (0 != size));
+    }
+    if (!fError) {
+        (void)this->skip(size);
+    }
 }
 
 bool SkReadBuffer::readArray(void* value, size_t size, size_t elementSize) {
-    const size_t count = this->getArrayCount();
-    if (count == size) {
-        (void)fReader.skip(sizeof(uint32_t)); // Skip array count
-        const size_t byteLength = count * elementSize;
-        memcpy(value, fReader.skip(SkAlign4(byteLength)), byteLength);
+    const uint32_t count = this->getArrayCount();
+    this->validate(size == count);
+    (void)this->skip(sizeof(uint32_t)); // Skip array count
+    const uint64_t byteLength64 = sk_64_mul(count, elementSize);
+    const size_t byteLength = count * elementSize;
+    this->validate(byteLength == byteLength64);
+    const void* ptr = this->skip(SkAlign4(byteLength));
+    if (!fError) {
+        memcpy(value, ptr, byteLength);
         return true;
     }
-    SkASSERT(false);
-    fReader.skip(fReader.available());
     return false;
 }
 
@@ -221,7 +303,9 @@ bool SkReadBuffer::readScalarArray(SkScalar* values, size_t size) {
 }
 
 uint32_t SkReadBuffer::getArrayCount() {
-    return *(uint32_t*)fReader.peek();
+    const size_t inc = sizeof(uint32_t);
+    fError = fError || !IsPtrAlign4(fReader.peek()) || !fReader.isAvailable(inc);
+    return fError ? 0 : *(uint32_t*)fReader.peek();
 }
 
 sk_sp<SkImage> SkReadBuffer::readImage() {
@@ -312,8 +396,8 @@ SkFlattenable* SkReadBuffer::readFlattenable(SkFlattenable::Type ft) {
             return nullptr;
         }
     } else if (fFactoryCount > 0) {
-        int32_t index = fReader.readU32();
-        if (0 == index) {
+        int32_t index = this->read32();
+        if (0 == index || !this->isValid()) {
             return nullptr; // writer failed to give us the flattenable
         }
         index -= 1;     // we stored the index-base-1
diff --git a/src/core/SkReadBuffer.h b/src/core/SkReadBuffer.h
index ae3fde6818..78038aa7af 100644
--- a/src/core/SkReadBuffer.h
+++ b/src/core/SkReadBuffer.h
@@ -115,37 +115,37 @@ public:
     size_t size() { return fReader.size(); }
     size_t offset() { return fReader.offset(); }
     bool eof() { return fReader.eof(); }
-    virtual const void* skip(size_t size) { return fReader.skip(size); }
+    const void* skip(size_t size);
 
     // primitives
-    virtual bool readBool();
-    virtual SkColor readColor();
-    virtual int32_t readInt();
-    virtual SkScalar readScalar();
-    virtual uint32_t readUInt();
-    virtual int32_t read32();
+    bool readBool();
+    SkColor readColor();
+    int32_t readInt();
+    SkScalar readScalar();
+    uint32_t readUInt();
+    int32_t read32();
 
     // peek
-    virtual uint8_t peekByte();
+    uint8_t peekByte();
 
     // strings -- the caller is responsible for freeing the string contents
-    virtual void readString(SkString* string);
+    void readString(SkString* string);
 
     // common data structures
-    virtual void readColor4f(SkColor4f* color);
-    virtual void readPoint(SkPoint* point);
+    void readColor4f(SkColor4f* color);
+    void readPoint(SkPoint* point);
     SkPoint readPoint() { SkPoint p; this->readPoint(&p); return p; }
-    virtual void readPoint3(SkPoint3* point);
-    virtual void readMatrix(SkMatrix* matrix);
-    virtual void readIRect(SkIRect* rect);
-    virtual void readRect(SkRect* rect);
-    virtual void readRRect(SkRRect* rrect);
-    virtual void readRegion(SkRegion* region);
-
-    virtual void readPath(SkPath* path);
+    void readPoint3(SkPoint3* point);
+    void readMatrix(SkMatrix* matrix);
+    void readIRect(SkIRect* rect);
+    void readRect(SkRect* rect);
+    void readRRect(SkRRect* rrect);
+    void readRegion(SkRegion* region);
+
+    void readPath(SkPath* path);
     virtual void readPaint(SkPaint* paint) { paint->unflatten(*this); }
 
-    virtual SkFlattenable* readFlattenable(SkFlattenable::Type);
+    SkFlattenable* readFlattenable(SkFlattenable::Type);
     template <typename T> sk_sp<T> readFlattenable() {
         return sk_sp<T>((T*)this->readFlattenable(T::GetFlattenableType()));
     }
@@ -158,15 +158,15 @@ public:
     sk_sp<SkShader> readShader() { return this->readFlattenable<SkShaderBase>(); }
 
     // Reads SkAlign4(bytes), but will only copy bytes into the buffer.
-    virtual bool readPad32(void* buffer, size_t bytes);
+    bool readPad32(void* buffer, size_t bytes);
 
     // binary data and arrays
-    virtual bool readByteArray(void* value, size_t size);
-    virtual bool readColorArray(SkColor* colors, size_t size);
-    virtual bool readColor4fArray(SkColor4f* colors, size_t size);
-    virtual bool readIntArray(int32_t* values, size_t size);
-    virtual bool readPointArray(SkPoint* points, size_t size);
-    virtual bool readScalarArray(SkScalar* values, size_t size);
+    bool readByteArray(void* value, size_t size);
+    bool readColorArray(SkColor* colors, size_t size);
+    bool readColor4fArray(SkColor4f* colors, size_t size);
+    bool readIntArray(int32_t* values, size_t size);
+    bool readPointArray(SkPoint* points, size_t size);
+    bool readScalarArray(SkScalar* values, size_t size);
 
     sk_sp<SkData> readByteArrayAsData() {
         size_t len = this->getArrayCount();
@@ -179,7 +179,7 @@ public:
     }
 
     // helpers to get info about arrays and binary data
-    virtual uint32_t getArrayCount();
+    uint32_t getArrayCount();
 
     // If there is a real error (e.g. data is corrupted) this returns null. If the image cannot
     // be created (e.g. it was not originally encoded) then this returns an image that doesn't
@@ -221,9 +221,17 @@ public:
     void setImageDeserializer(SkImageDeserializer* factory);
 #endif
 
-    // Default impelementations don't check anything.
-    virtual bool validate(bool isValid) { return isValid; }
-    virtual bool isValid() const { return true; }
+    /**
+     *  If isValid is false, sets the buffer to be "invalid". Returns true if the buffer
+     *  is still valid.
+     */
+    bool validate(bool isValid) {
+        if (!isValid) {
+            this->setInvalid();
+        }
+        return !fError;
+    }
+    bool isValid() const { return !fError; }
     bool validateIndex(int index, int count) {
         return this->validate(index >= 0 && index < count);
     }
@@ -266,6 +274,7 @@ protected:
     SkTHashMap<uint32_t, SkString> fFlattenableDict;
 
 private:
+    void setInvalid();
     bool readArray(void* value, size_t size, size_t elementSize);
 
     uint32_t fFlags;
@@ -291,6 +300,12 @@ private:
 #endif // DEBUG_NON_DETERMINISTIC_ASSERT
 
     SkInflator* fInflator = nullptr;
+
+    static bool IsPtrAlign4(const void* ptr) {
+        return SkIsAlign4((uintptr_t)ptr);
+    }
+
+    bool fError = false;
 };
 
 #endif // SkReadBuffer_DEFINED
diff --git a/src/core/SkTextBlob.cpp b/src/core/SkTextBlob.cpp
index 3879c1e78e..193c82de66 100644
--- a/src/core/SkTextBlob.cpp
+++ b/src/core/SkTextBlob.cpp
@@ -7,9 +7,9 @@
 
 #include "SkTextBlobRunIterator.h"
 
+#include "SkReadBuffer.h"
 #include "SkSafeMath.h"
 #include "SkTypeface.h"
-#include "SkValidatingReadBuffer.h"
 #include "SkWriteBuffer.h"
 
 #include <limits>
@@ -895,11 +895,11 @@ sk_sp<SkData> SkTextBlob::serialize(SkTypefaceCatalogerProc proc, void* ctx) con
     return data;
 }
 
-class SkTypefaceResolverReadBuffer : public SkValidatingReadBuffer {
+class SkTypefaceResolverReadBuffer : public SkReadBuffer {
 public:
     SkTypefaceResolverReadBuffer(const void* data, size_t size, SkTypefaceResolverProc proc,
                                  void* ctx)
-        : SkValidatingReadBuffer(data, size)
+        : SkReadBuffer(data, size)
         , fResolverProc(proc)
         , fResolverCtx(ctx)
     {}
diff --git a/src/core/SkValidatingReadBuffer.cpp b/src/core/SkValidatingReadBuffer.cpp
deleted file mode 100644
index e13cde5da6..0000000000
--- a/src/core/SkValidatingReadBuffer.cpp
+++ /dev/null
@@ -1,306 +0,0 @@
-/*
- * Copyright 2013 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "SkBitmap.h"
-#include "SkMatrixPriv.h"
-#include "SkValidatingReadBuffer.h"
-#include "SkStream.h"
-#include "SkTypeface.h"
-
-SkValidatingReadBuffer::SkValidatingReadBuffer(const void* data, size_t size) :
-    fError(false) {
-    this->setMemory(data, size);
-    this->setFlags(SkReadBuffer::kValidation_Flag);
-}
-
-SkValidatingReadBuffer::~SkValidatingReadBuffer() {
-}
-
-bool SkValidatingReadBuffer::validate(bool isValid) {
-    if (!fError && !isValid) {
-        // When an error is found, send the read cursor to the end of the stream
-        fReader.skip(fReader.available());
-        fError = true;
-    }
-    return !fError;
-}
-
-bool SkValidatingReadBuffer::isValid() const {
-    return !fError;
-}
-
-void SkValidatingReadBuffer::setMemory(const void* data, size_t size) {
-    this->validate(IsPtrAlign4(data) && (SkAlign4(size) == size));
-    if (!fError) {
-        fReader.setMemory(data, size);
-    }
-}
-
-const void* SkValidatingReadBuffer::skip(size_t size) {
-    size_t inc = SkAlign4(size);
-    this->validate(inc >= size);
-    const void* addr = fReader.peek();
-    this->validate(IsPtrAlign4(addr) && fReader.isAvailable(inc));
-    if (fError) {
-        return nullptr;
-    }
-
-    fReader.skip(size);
-    return addr;
-}
-
-// All the methods in this file funnel down into either readInt(), readScalar() or skip(),
-// followed by a memcpy. So we've got all our validation in readInt(), readScalar() and skip();
-// if they fail they'll return a zero value or skip nothing, respectively, and set fError to
-// true, which the caller should check to see if an error occurred during the read operation.
-
-bool SkValidatingReadBuffer::readBool() {
-    uint32_t value = this->readInt();
-    // Boolean value should be either 0 or 1
-    this->validate(!(value & ~1));
-    return value != 0;
-}
-
-SkColor SkValidatingReadBuffer::readColor() {
-    return this->readInt();
-}
-
-int32_t SkValidatingReadBuffer::readInt() {
-    const size_t inc = sizeof(int32_t);
-    this->validate(IsPtrAlign4(fReader.peek()) && fReader.isAvailable(inc));
-    return fError ? 0 : fReader.readInt();
-}
-
-SkScalar SkValidatingReadBuffer::readScalar() {
-    const size_t inc = sizeof(SkScalar);
-    this->validate(IsPtrAlign4(fReader.peek()) && fReader.isAvailable(inc));
-    return fError ? 0 : fReader.readScalar();
-}
-
-uint32_t SkValidatingReadBuffer::readUInt() {
-    return this->readInt();
-}
-
-int32_t SkValidatingReadBuffer::read32() {
-    return this->readInt();
-}
-
-uint8_t SkValidatingReadBuffer::peekByte() {
-    if (fReader.available() <= 0) {
-        fError = true;
-        return 0;
-    }
-    return *((uint8_t*) fReader.peek());
-}
-
-void SkValidatingReadBuffer::readString(SkString* string) {
-    const size_t len = this->readUInt();
-    const void* ptr = fReader.peek();
-    const char* cptr = (const char*)ptr;
-
-    // skip over the string + '\0' and then pad to a multiple of 4
-    const size_t alignedSize = SkAlign4(len + 1);
-    this->skip(alignedSize);
-    if (!fError) {
-        this->validate(cptr[len] == '\0');
-    }
-    if (!fError) {
-        string->set(cptr, len);
-    }
-}
-
-void SkValidatingReadBuffer::readColor4f(SkColor4f* color) {
-    const void* ptr = this->skip(sizeof(SkColor4f));
-    if (!fError) {
-        memcpy(color, ptr, sizeof(SkColor4f));
-    } else {
-        *color = SkColor4f::FromColor(SK_ColorBLACK);
-    }
-}
-
-void SkValidatingReadBuffer::readPoint(SkPoint* point) {
-    point->fX = this->readScalar();
-    point->fY = this->readScalar();
-}
-
-void SkValidatingReadBuffer::readPoint3(SkPoint3* point) {
-    point->fX = this->readScalar();
-    point->fY = this->readScalar();
-    point->fZ = this->readScalar();
-}
-
-void SkValidatingReadBuffer::readMatrix(SkMatrix* matrix) {
-    size_t size = 0;
-    if (!fError) {
-        size = SkMatrixPriv::ReadFromMemory(matrix, fReader.peek(), fReader.available());
-        this->validate((SkAlign4(size) == size) && (0 != size));
-    }
-    if (!fError) {
-        (void)this->skip(size);
-    }
-}
-
-void SkValidatingReadBuffer::readIRect(SkIRect* rect) {
-    const void* ptr = this->skip(sizeof(SkIRect));
-    if (!fError) {
-        memcpy(rect, ptr, sizeof(SkIRect));
-    } else {
-        rect->setEmpty();
-    }
-}
-
-void SkValidatingReadBuffer::readRect(SkRect* rect) {
-    const void* ptr = this->skip(sizeof(SkRect));
-    if (!fError) {
-        memcpy(rect, ptr, sizeof(SkRect));
-    } else {
-        rect->setEmpty();
-    }
-}
-
-void SkValidatingReadBuffer::readRRect(SkRRect* rrect) {
-    const void* ptr = this->skip(sizeof(SkRRect));
-    if (!fError) {
-        memcpy(rrect, ptr, sizeof(SkRRect));
-        this->validate(rrect->isValid());
-    }
-
-    if (fError) {
-        rrect->setEmpty();
-    }
-}
-
-void SkValidatingReadBuffer::readRegion(SkRegion* region) {
-    size_t size = 0;
-    if (!fError) {
-        size = region->readFromMemory(fReader.peek(), fReader.available());
-        this->validate((SkAlign4(size) == size) && (0 != size));
-    }
-    if (!fError) {
-        (void)this->skip(size);
-    }
-}
-
-void SkValidatingReadBuffer::readPath(SkPath* path) {
-    size_t size = 0;
-    if (!fError) {
-        size = path->readFromMemory(fReader.peek(), fReader.available());
-        this->validate((SkAlign4(size) == size) && (0 != size));
-    }
-    if (!fError) {
-        (void)this->skip(size);
-    }
-}
-
-bool SkValidatingReadBuffer::readArray(void* value, size_t size, size_t elementSize) {
-    const uint32_t count = this->getArrayCount();
-    this->validate(size == count);
-    (void)this->skip(sizeof(uint32_t)); // Skip array count
-    const uint64_t byteLength64 = sk_64_mul(count, elementSize);
-    const size_t byteLength = count * elementSize;
-    this->validate(byteLength == byteLength64);
-    const void* ptr = this->skip(SkAlign4(byteLength));
-    if (!fError) {
-        memcpy(value, ptr, byteLength);
-        return true;
-    }
-    return false;
-}
-
-bool SkValidatingReadBuffer::readByteArray(void* value, size_t size) {
-    return this->readArray(static_cast<unsigned char*>(value), size, sizeof(unsigned char));
-}
-
-bool SkValidatingReadBuffer::readColorArray(SkColor* colors, size_t size) {
-    return this->readArray(colors, size, sizeof(SkColor));
-}
-
-bool SkValidatingReadBuffer::readColor4fArray(SkColor4f* colors, size_t size) {
-    return this->readArray(colors, size, sizeof(SkColor4f));
-}
-
-bool SkValidatingReadBuffer::readIntArray(int32_t* values, size_t size) {
-    return this->readArray(values, size, sizeof(int32_t));
-}
-
-bool SkValidatingReadBuffer::readPointArray(SkPoint* points, size_t size) {
-    return this->readArray(points, size, sizeof(SkPoint));
-}
-
-bool SkValidatingReadBuffer::readScalarArray(SkScalar* values, size_t size) {
-    return this->readArray(values, size, sizeof(SkScalar));
-}
-
-uint32_t SkValidatingReadBuffer::getArrayCount() {
-    const size_t inc = sizeof(uint32_t);
-    fError = fError || !IsPtrAlign4(fReader.peek()) || !fReader.isAvailable(inc);
-    return fError ? 0 : *(uint32_t*)fReader.peek();
-}
-
-SkFlattenable* SkValidatingReadBuffer::readFlattenable(SkFlattenable::Type type) {
-    // The validating read buffer always uses strings and string-indices for unflattening.
-    SkASSERT(0 == this->factoryCount());
-
-    uint8_t firstByte = this->peekByte();
-    if (fError) {
-        return nullptr;
-    }
-
-    SkString name;
-    if (firstByte) {
-        // If the first byte is non-zero, the flattenable is specified by a string.
-        this->readString(&name);
-        if (fError) {
-            return nullptr;
-        }
-
-        // Add the string to the dictionary.
-        fFlattenableDict.set(fFlattenableDict.count() + 1, name);
-    } else {
-        // Read the index.  We are guaranteed that the first byte
-        // is zeroed, so we must shift down a byte.
-        uint32_t index = this->readUInt() >> 8;
-        if (0 == index) {
-            return nullptr; // writer failed to give us the flattenable
-        }
-
-        SkString* namePtr = fFlattenableDict.find(index);
-        if (!namePtr) {
-            return nullptr;
-        }
-        name = *namePtr;
-    }
-
-    // Is this the type we wanted ?
-    const char* cname = name.c_str();
-    SkFlattenable::Type baseType;
-    if (!SkFlattenable::NameToType(cname, &baseType) || (baseType != type)) {
-        return nullptr;
-    }
-
-    // Get the factory for this flattenable.
-    SkFlattenable::Factory factory = this->getCustomFactory(name);
-    if (!factory) {
-        factory = SkFlattenable::NameToFactory(cname);
-        if (!factory) {
-            return nullptr; // writer failed to give us the flattenable
-        }
-    }
-
-    // If we get here, the factory is non-null.
-    sk_sp<SkFlattenable> obj;
-    uint32_t sizeRecorded = this->readUInt();
-    size_t offset = fReader.offset();
-    obj = (*factory)(*this);
-    // check that we read the amount we expected
-    size_t sizeRead = fReader.offset() - offset;
-    this->validate(sizeRecorded == sizeRead);
-    if (fError) {
-        obj = nullptr;
-    }
-    return obj.release();
-}
diff --git a/src/core/SkValidatingReadBuffer.h b/src/core/SkValidatingReadBuffer.h
index 3203ebac55..ccf66d4528 100644
--- a/src/core/SkValidatingReadBuffer.h
+++ b/src/core/SkValidatingReadBuffer.h
@@ -17,69 +17,7 @@
 
 class SkBitmap;
 
-class SkValidatingReadBuffer : public SkReadBuffer {
-public:
-    SkValidatingReadBuffer(const void* data, size_t size);
-    ~SkValidatingReadBuffer() override;
+// DEPRECATED -- just use SkReadBuffer (so we can delete this header)
+typedef SkReadBuffer SkValidatingReadBuffer;
 
-    SkReadBuffer* clone(const void* data, size_t size) const override {
-        return new SkValidatingReadBuffer(data, size);
-    }
-
-    const void* skip(size_t size) override;
-
-    // primitives
-    bool readBool() override;
-    SkColor readColor() override;
-    int32_t readInt() override;
-    SkScalar readScalar() override;
-    uint32_t readUInt() override;
-    int32_t read32() override;
-
-    // peek
-    uint8_t peekByte() override;
-
-    // strings -- the caller is responsible for freeing the string contents
-    void readString(SkString* string) override;
-
-    // common data structures
-    SkFlattenable* readFlattenable(SkFlattenable::Type type) override;
-    void readColor4f(SkColor4f* color) override;
-    void readPoint(SkPoint* point) override;
-    void readPoint3(SkPoint3* point) override;
-    void readMatrix(SkMatrix* matrix) override;
-    void readIRect(SkIRect* rect) override;
-    void readRect(SkRect* rect) override;
-    void readRRect(SkRRect* rrect) override;
-    void readRegion(SkRegion* region) override;
-    void readPath(SkPath* path) override;
-
-    // binary data and arrays
-    bool readByteArray(void* value, size_t size) override;
-    bool readColorArray(SkColor* colors, size_t size) override;
-    bool readColor4fArray(SkColor4f* colors, size_t size) override;
-    bool readIntArray(int32_t* values, size_t size) override;
-    bool readPointArray(SkPoint* points, size_t size) override;
-    bool readScalarArray(SkScalar* values, size_t size) override;
-
-    // helpers to get info about arrays and binary data
-    uint32_t getArrayCount() override;
-
-    bool validate(bool isValid) override;
-    bool isValid() const override;
-
-private:
-    bool readArray(void* value, size_t size, size_t elementSize);
-
-    void setMemory(const void* data, size_t size);
-
-    static bool IsPtrAlign4(const void* ptr) {
-        return SkIsAlign4((uintptr_t)ptr);
-    }
-
-    bool fError;
-
-    typedef SkReadBuffer INHERITED;
-};
-
-#endif // SkValidatingReadBuffer_DEFINED
+#endif
diff --git a/tests/ImageFilterTest.cpp b/tests/ImageFilterTest.cpp
index 0433d0167c..bcc0bfe1f5 100644
--- a/tests/ImageFilterTest.cpp
+++ b/tests/ImageFilterTest.cpp
@@ -1250,7 +1250,7 @@ DEF_TEST(ImageFilterCrossProcessPictureImageFilter, reporter) {
 
     // Check that, for now, SkPictureImageFilter does not serialize or
     // deserialize its contained picture when the filter is serialized
-    // cross-process. Do this by "laundering" it through SkValidatingReadBuffer.
+    // cross-process. Do this by "laundering" it through SkReadBuffer.
     sk_sp<SkData> data(SkValidatingSerializeFlattenable(imageFilter.get()));
     sk_sp<SkImageFilter> unflattenedFilter = SkValidatingDeserializeImageFilter(data->data(),
                                                                                 data->size());
diff --git a/tests/SerializationTest.cpp b/tests/SerializationTest.cpp
index da3c47cfe9..4cf605a2fc 100644
--- a/tests/SerializationTest.cpp
+++ b/tests/SerializationTest.cpp
@@ -16,13 +16,13 @@
 #include "SkMallocPixelRef.h"
 #include "SkMatrixPriv.h"
 #include "SkOSFile.h"
+#include "SkReadBuffer.h"
 #include "SkPictureRecorder.h"
 #include "SkShaderBase.h"
 #include "SkTableColorFilter.h"
 #include "SkTemplates.h"
 #include "SkTypeface.h"
 #include "SkWriteBuffer.h"
-#include "SkValidatingReadBuffer.h"
 #include "SkXfermodeImageFilter.h"
 #include "sk_tool_utils.h"
 #include "Test.h"
@@ -49,7 +49,7 @@ template<typename T> struct SerializationUtils {
     static void Write(SkWriteBuffer& writer, const T* flattenable) {
         writer.writeFlattenable(flattenable);
     }
-    static void Read(SkValidatingReadBuffer& reader, T** flattenable) {
+    static void Read(SkReadBuffer& reader, T** flattenable) {
         *flattenable = (T*)reader.readFlattenable(T::GetFlattenableType());
     }
 };
@@ -58,7 +58,7 @@ template<> struct SerializationUtils<SkMatrix> {
     static void Write(SkWriteBuffer& writer, const SkMatrix* matrix) {
         writer.writeMatrix(*matrix);
     }
-    static void Read(SkValidatingReadBuffer& reader, SkMatrix* matrix) {
+    static void Read(SkReadBuffer& reader, SkMatrix* matrix) {
         reader.readMatrix(matrix);
     }
 };
@@ -67,7 +67,7 @@ template<> struct SerializationUtils<SkPath> {
     static void Write(SkWriteBuffer& writer, const SkPath* path) {
         writer.writePath(*path);
     }
-    static void Read(SkValidatingReadBuffer& reader, SkPath* path) {
+    static void Read(SkReadBuffer& reader, SkPath* path) {
         reader.readPath(path);
     }
 };
@@ -76,7 +76,7 @@ template<> struct SerializationUtils<SkRegion> {
     static void Write(SkWriteBuffer& writer, const SkRegion* region) {
         writer.writeRegion(*region);
     }
-    static void Read(SkValidatingReadBuffer& reader, SkRegion* region) {
+    static void Read(SkReadBuffer& reader, SkRegion* region) {
         reader.readRegion(region);
     }
 };
@@ -85,7 +85,7 @@ template<> struct SerializationUtils<SkString> {
     static void Write(SkWriteBuffer& writer, const SkString* string) {
         writer.writeString(string->c_str());
     }
-    static void Read(SkValidatingReadBuffer& reader, SkString* string) {
+    static void Read(SkReadBuffer& reader, SkString* string) {
         reader.readString(string);
     }
 };
@@ -94,7 +94,7 @@ template<> struct SerializationUtils<unsigned char> {
     static void Write(SkWriteBuffer& writer, unsigned char* data, uint32_t arraySize) {
         writer.writeByteArray(data, arraySize);
     }
-    static bool Read(SkValidatingReadBuffer& reader, unsigned char* data, uint32_t arraySize) {
+    static bool Read(SkReadBuffer& reader, unsigned char* data, uint32_t arraySize) {
         return reader.readByteArray(data, arraySize);
     }
 };
@@ -103,7 +103,7 @@ template<> struct SerializationUtils<SkColor> {
     static void Write(SkWriteBuffer& writer, SkColor* data, uint32_t arraySize) {
         writer.writeColorArray(data, arraySize);
     }
-    static bool Read(SkValidatingReadBuffer& reader, SkColor* data, uint32_t arraySize) {
+    static bool Read(SkReadBuffer& reader, SkColor* data, uint32_t arraySize) {
         return reader.readColorArray(data, arraySize);
     }
 };
@@ -112,7 +112,7 @@ template<> struct SerializationUtils<SkColor4f> {
     static void Write(SkWriteBuffer& writer, SkColor4f* data, uint32_t arraySize) {
         writer.writeColor4fArray(data, arraySize);
     }
-    static bool Read(SkValidatingReadBuffer& reader, SkColor4f* data, uint32_t arraySize) {
+    static bool Read(SkReadBuffer& reader, SkColor4f* data, uint32_t arraySize) {
         return reader.readColor4fArray(data, arraySize);
     }
 };
@@ -121,7 +121,7 @@ template<> struct SerializationUtils<int32_t> {
     static void Write(SkWriteBuffer& writer, int32_t* data, uint32_t arraySize) {
         writer.writeIntArray(data, arraySize);
     }
-    static bool Read(SkValidatingReadBuffer& reader, int32_t* data, uint32_t arraySize) {
+    static bool Read(SkReadBuffer& reader, int32_t* data, uint32_t arraySize) {
         return reader.readIntArray(data, arraySize);
     }
 };
@@ -130,7 +130,7 @@ template<> struct SerializationUtils<SkPoint> {
     static void Write(SkWriteBuffer& writer, SkPoint* data, uint32_t arraySize) {
         writer.writePointArray(data, arraySize);
     }
-    static bool Read(SkValidatingReadBuffer& reader, SkPoint* data, uint32_t arraySize) {
+    static bool Read(SkReadBuffer& reader, SkPoint* data, uint32_t arraySize) {
         return reader.readPointArray(data, arraySize);
     }
 };
@@ -139,7 +139,7 @@ template<> struct SerializationUtils<SkScalar> {
     static void Write(SkWriteBuffer& writer, SkScalar* data, uint32_t arraySize) {
         writer.writeScalarArray(data, arraySize);
     }
-    static bool Read(SkValidatingReadBuffer& reader, SkScalar* data, uint32_t arraySize) {
+    static bool Read(SkReadBuffer& reader, SkScalar* data, uint32_t arraySize) {
         return reader.readScalarArray(data, arraySize);
     }
 };
@@ -167,13 +167,13 @@ static void TestObjectSerializationNoAlign(T* testObj, skiatest::Reporter* repor
     SerializationTestUtils<T, testInvalid>::InvalidateData(dataWritten);
 
     // Make sure this fails when it should (test with smaller size, but still multiple of 4)
-    SkValidatingReadBuffer buffer(dataWritten, bytesWritten - 4);
+    SkReadBuffer buffer(dataWritten, bytesWritten - 4);
     T obj;
     SerializationUtils<T>::Read(buffer, &obj);
     REPORTER_ASSERT(reporter, !buffer.isValid());
 
     // Make sure this succeeds when it should
-    SkValidatingReadBuffer buffer2(dataWritten, bytesWritten);
+    SkReadBuffer buffer2(dataWritten, bytesWritten);
     size_t offsetBefore = buffer2.offset();
     T obj2;
     SerializationUtils<T>::Read(buffer2, &obj2);
@@ -204,14 +204,14 @@ static T* TestFlattenableSerialization(T* testObj, bool shouldSucceed,
     writer.writeToMemory(dataWritten);
 
     // Make sure this fails when it should (test with smaller size, but still multiple of 4)
-    SkValidatingReadBuffer buffer(dataWritten, bytesWritten - 4);
+    SkReadBuffer buffer(dataWritten, bytesWritten - 4);
     T* obj = nullptr;
     SerializationUtils<T>::Read(buffer, &obj);
     REPORTER_ASSERT(reporter, !buffer.isValid());
     REPORTER_ASSERT(reporter, nullptr == obj);
 
     // Make sure this succeeds when it should
-    SkValidatingReadBuffer buffer2(dataWritten, bytesWritten);
+    SkReadBuffer buffer2(dataWritten, bytesWritten);
     const unsigned char* peekBefore = static_cast<const unsigned char*>(buffer2.skip(0));
     T* obj2 = nullptr;
     SerializationUtils<T>::Read(buffer2, &obj2);
@@ -242,14 +242,14 @@ static void TestArraySerialization(T* data, skiatest::Reporter* reporter) {
     writer.writeToMemory(dataWritten);
 
     // Make sure this fails when it should
-    SkValidatingReadBuffer buffer(dataWritten, bytesWritten);
+    SkReadBuffer buffer(dataWritten, bytesWritten);
     T dataRead[kArraySize];
     bool success = SerializationUtils<T>::Read(buffer, dataRead, kArraySize / 2);
     // This should have failed, since the provided size was too small
     REPORTER_ASSERT(reporter, !success);
 
     // Make sure this succeeds when it should
-    SkValidatingReadBuffer buffer2(dataWritten, bytesWritten);
+    SkReadBuffer buffer2(dataWritten, bytesWritten);
     success = SerializationUtils<T>::Read(buffer2, dataRead, kArraySize);
     // This should have succeeded, since there are enough bytes to read this
     REPORTER_ASSERT(reporter, success);
@@ -548,7 +548,7 @@ DEF_TEST(Serialization, reporter) {
         writer.writeToMemory(static_cast<void*>(data.get()));
 
         // Deserialize picture
-        SkValidatingReadBuffer reader(static_cast<void*>(data.get()), size);
+        SkReadBuffer reader(static_cast<void*>(data.get()), size);
         sk_sp<SkPicture> readPict(SkPicture::MakeFromBuffer(reader));
         REPORTER_ASSERT(reporter, reader.isValid());
         REPORTER_ASSERT(reporter, readPict.get());
diff --git a/tools/debugger/SkDrawCommand.cpp b/tools/debugger/SkDrawCommand.cpp
index 3e05da0572..5f4316f866 100644
--- a/tools/debugger/SkDrawCommand.cpp
+++ b/tools/debugger/SkDrawCommand.cpp
@@ -20,11 +20,11 @@
 #include "SkPaintDefaults.h"
 #include "SkPathEffect.h"
 #include "SkPicture.h"
+#include "SkReadBuffer.h"
 #include "SkTextBlob.h"
 #include "SkTextBlobRunIterator.h"
 #include "SkTHash.h"
 #include "SkTypeface.h"
-#include "SkValidatingReadBuffer.h"
 #include "SkWriteBuffer.h"
 #include "picture_utils.h"
 #include "SkClipOpPriv.h"
@@ -860,7 +860,7 @@ static SkFlattenable* load_flattenable(Json::Value jsonFlattenable,
     }
     const void* data;
     int size = decode_data(jsonFlattenable[SKDEBUGCANVAS_ATTRIBUTE_DATA], urlDataManager, &data);
-    SkValidatingReadBuffer buffer(data, size);
+    SkReadBuffer buffer(data, size);
     sk_sp<SkFlattenable> result = factory(buffer);
     if (!buffer.isValid()) {
         SkDebugf("invalid buffer loading flattenable\n");