Checking structure sizes before reading them from memory to avoid overflowing the buffer's stream.

tests/SerializationTest.cpp

Index: tests/SerializationTest.cpp
diff --git a/tests/SerializationTest.cpp b/tests/SerializationTest.cpp
index d2c692526f62bf3a673aba38568f2a19a7b6f95a..f047ec128470f3393a3ecabac652340eb16a98c2 100644
--- a/tests/SerializationTest.cpp
+++ b/tests/SerializationTest.cpp
@@ -9,140 +9,202 @@
 #include "SkValidatingReadBuffer.h"
 #include "Test.h"
 
-static void Tests(skiatest::Reporter* reporter) {
-    {
-        static const uint32_t arraySize = 512;
-        unsigned char data[arraySize] = {0};
-        SkOrderedWriteBuffer writer(1024);
-        writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag);
-        writer.writeByteArray(data, arraySize);
-        uint32_t bytesWritten = writer.bytesWritten();
-        // This should write the length (in 4 bytes) and the array
-        REPORTER_ASSERT(reporter, (4 + arraySize) == bytesWritten);
+static const uint32_t kArraySize = 64;
+
+template<typename T>
+static void TestAlignment(T* testObj, skiatest::Reporter* reporter) {
+    // Test memory read/write functions directly
+    unsigned char dataWritten[1024];
+    size_t bytesWrittenToMemory = testObj->writeToMemory(dataWritten);
+    REPORTER_ASSERT(reporter, SkAlign4(bytesWrittenToMemory) == bytesWrittenToMemory);
+    size_t bytesReadFromMemory = testObj->readFromMemory(dataWritten, bytesWrittenToMemory);
+    REPORTER_ASSERT(reporter, SkAlign4(bytesReadFromMemory) == bytesReadFromMemory);
+}
 
-        unsigned char dataWritten[1024];
-        writer.writeToMemory(dataWritten);
+template<typename T> struct SerializationUtils {
+};
 
-        // Make sure this fails when it should
-        SkValidatingReadBuffer buffer(dataWritten, bytesWritten);
-        unsigned char dataRead[arraySize];
-        bool success = buffer.readByteArray(dataRead, 256);
-        // This should have failed, since 256 < sizeInBytes
-        REPORTER_ASSERT(reporter, !success);
+template<> struct SerializationUtils<SkMatrix> {
+    static void Write(SkOrderedWriteBuffer& writer, const SkMatrix* matrix) {
+        writer.writeMatrix(*matrix);
+    }
+    static void Read(SkValidatingReadBuffer& reader, SkMatrix* matrix) {
+        reader.readMatrix(matrix);
+    }
+};
 
-        // Make sure this succeeds when it should
-        SkValidatingReadBuffer buffer2(dataWritten, bytesWritten);
-        success = buffer2.readByteArray(dataRead, arraySize);
-        // This should have succeeded, since there are enough bytes to read this
-        REPORTER_ASSERT(reporter, success);
+template<> struct SerializationUtils<SkPath> {
+    static void Write(SkOrderedWriteBuffer& writer, const SkPath* path) {
+        writer.writePath(*path);
     }
+    static void Read(SkValidatingReadBuffer& reader, SkPath* path) {
+        reader.readPath(path);
+    }
+};
 
-    {
-        static const uint32_t arraySize = 64;
-        SkColor data[arraySize];
-        SkOrderedWriteBuffer writer(1024);
-        writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag);
+template<> struct SerializationUtils<SkRegion> {
+    static void Write(SkOrderedWriteBuffer& writer, const SkRegion* region) {
+        writer.writeRegion(*region);
+    }
+    static void Read(SkValidatingReadBuffer& reader, SkRegion* region) {
+        reader.readRegion(region);
+    }
+};
+
+template<> struct SerializationUtils<unsigned char> {
+    static void Write(SkOrderedWriteBuffer& writer, unsigned char* data, uint32_t arraySize) {
+        writer.writeByteArray(data, arraySize);
+    }
+    static bool Read(SkValidatingReadBuffer& reader, unsigned char* data, uint32_t arraySize) {
+        return reader.readByteArray(data, arraySize);
+    }
+};
+
+template<> struct SerializationUtils<SkColor> {
+    static void Write(SkOrderedWriteBuffer& writer, SkColor* data, uint32_t arraySize) {
         writer.writeColorArray(data, arraySize);
-        uint32_t bytesWritten = writer.bytesWritten();
-        // This should write the length (in 4 bytes) and the array
-        REPORTER_ASSERT(reporter, (4 + arraySize * sizeof(SkColor)) == bytesWritten);
+    }
+    static bool Read(SkValidatingReadBuffer& reader, SkColor* data, uint32_t arraySize) {
+        return reader.readColorArray(data, arraySize);
+    }
+};
 
-        unsigned char dataWritten[1024];
-        writer.writeToMemory(dataWritten);
+template<> struct SerializationUtils<int32_t> {
+    static void Write(SkOrderedWriteBuffer& writer, int32_t* data, uint32_t arraySize) {
+        writer.writeIntArray(data, arraySize);
+    }
+    static bool Read(SkValidatingReadBuffer& reader, int32_t* data, uint32_t arraySize) {
+        return reader.readIntArray(data, arraySize);
+    }
+};
 
-        // Make sure this fails when it should
-        SkValidatingReadBuffer buffer(dataWritten, bytesWritten);
-        SkColor dataRead[arraySize];
-        bool success = buffer.readColorArray(dataRead, 32);
-        // This should have failed, since 256 < sizeInBytes
-        REPORTER_ASSERT(reporter, !success);
+template<> struct SerializationUtils<SkPoint> {
+    static void Write(SkOrderedWriteBuffer& writer, SkPoint* data, uint32_t arraySize) {
+        writer.writePointArray(data, arraySize);
+    }
+    static bool Read(SkValidatingReadBuffer& reader, SkPoint* data, uint32_t arraySize) {
+        return reader.readPointArray(data, arraySize);
+    }
+};
 
-        // Make sure this succeeds when it should
-        SkValidatingReadBuffer buffer2(dataWritten, bytesWritten);
-        success = buffer2.readColorArray(dataRead, arraySize);
-        // This should have succeeded, since there are enough bytes to read this
-        REPORTER_ASSERT(reporter, success);
+template<> struct SerializationUtils<SkScalar> {
+    static void Write(SkOrderedWriteBuffer& writer, SkScalar* data, uint32_t arraySize) {
+        writer.writeScalarArray(data, arraySize);
+    }
+    static bool Read(SkValidatingReadBuffer& reader, SkScalar* data, uint32_t arraySize) {
+        return reader.readScalarArray(data, arraySize);
     }
+};
+
+template<typename T>
+static void TestObjectSerialization(T* testObj, skiatest::Reporter* reporter) {
+    SkOrderedWriteBuffer writer(1024);
+    writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag);
+    SerializationUtils<T>::Write(writer, testObj);
+    size_t bytesWritten = writer.bytesWritten();
+    REPORTER_ASSERT(reporter, SkAlign4(bytesWritten) == bytesWritten);
+
+    unsigned char dataWritten[1024];
+    writer.writeToMemory(dataWritten);
+
+    // Make sure this fails when it should (test with smaller size, but still multiple of 4)
+    SkValidatingReadBuffer buffer(dataWritten, bytesWritten - 4);

[sugoi1, 2013/11/04 18:41:37]

This was a silly error. I originally wrote "- 1" instead of "- 4", which
resulted in a non multiple of 4 size, which was causing the buffer to
immediately fail and, since we're expecting this test to fail, the test itself
didn't pick up that a failure here was unexpected.

+    const unsigned char* peekBefore = static_cast<const unsigned char*>(buffer.skip(0));
+    SerializationUtils<T>::Read(buffer, testObj);
+    const unsigned char* peekAfter = static_cast<const unsigned char*>(buffer.skip(0));
+    // This should have failed, since the buffer is too small to read a matrix from it
+    REPORTER_ASSERT(reporter, peekBefore == peekAfter);
+
+    // Make sure this succeeds when it should
+    SkValidatingReadBuffer buffer2(dataWritten, bytesWritten);
+    peekBefore = static_cast<const unsigned char*>(buffer2.skip(0));
+    SerializationUtils<T>::Read(buffer2, testObj);
+    peekAfter = static_cast<const unsigned char*>(buffer2.skip(0));
+    // This should have succeeded, since there are enough bytes to read this
+    REPORTER_ASSERT(reporter, static_cast<size_t>(peekAfter - peekBefore) == bytesWritten);
+
+    TestAlignment(testObj, reporter);
+}
+
+template<typename T>
+static void TestArraySerialization(T* data, skiatest::Reporter* reporter) {
+    SkOrderedWriteBuffer writer(1024);
+    writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag);
+    SerializationUtils<T>::Write(writer, data, kArraySize);
+    size_t bytesWritten = writer.bytesWritten();
+    // This should write the length (in 4 bytes) and the array
+    REPORTER_ASSERT(reporter, (4 + kArraySize * sizeof(T)) == bytesWritten);
+
+    unsigned char dataWritten[1024];
+    writer.writeToMemory(dataWritten);
+
+    // Make sure this fails when it should
+    SkValidatingReadBuffer 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);
+    success = SerializationUtils<T>::Read(buffer2, dataRead, kArraySize);
+    // This should have succeeded, since there are enough bytes to read this
+    REPORTER_ASSERT(reporter, success);
+}
 
+static void Tests(skiatest::Reporter* reporter) {
+    // Test matrix serialization
     {
-        static const uint32_t arraySize = 64;
-        int32_t data[arraySize];
-        SkOrderedWriteBuffer writer(1024);
-        writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag);
-        writer.writeIntArray(data, arraySize);
-        uint32_t bytesWritten = writer.bytesWritten();
-        // This should write the length (in 4 bytes) and the array
-        REPORTER_ASSERT(reporter, (4 + arraySize * sizeof(int32_t)) == bytesWritten);
+        SkMatrix matrix = SkMatrix::I();
+        TestObjectSerialization(&matrix, reporter);
+     }
 
-        unsigned char dataWritten[1024];
-        writer.writeToMemory(dataWritten);
+    // Test path serialization
+    {
+        SkPath path;
+        TestObjectSerialization(&path, reporter);
+    }
 
-        // Make sure this fails when it should
-        SkValidatingReadBuffer buffer(dataWritten, bytesWritten);
-        int32_t dataRead[arraySize];
-        bool success = buffer.readIntArray(dataRead, 32);
-        // This should have failed, since 256 < sizeInBytes
-        REPORTER_ASSERT(reporter, !success);
+    // Test region serialization
+    {
+        SkRegion region;
+        TestObjectSerialization(&region, reporter);
+    }
 
-        // Make sure this succeeds when it should
-        SkValidatingReadBuffer buffer2(dataWritten, bytesWritten);
-        success = buffer2.readIntArray(dataRead, arraySize);
-        // This should have succeeded, since there are enough bytes to read this
-        REPORTER_ASSERT(reporter, success);
+    // Test rrect serialization
+    {
+        SkRRect rrect;
+        TestAlignment(&rrect, reporter);
     }
 
+    // Test readByteArray
     {
-        static const uint32_t arraySize = 64;
-        SkPoint data[arraySize];
-        SkOrderedWriteBuffer writer(1024);
-        writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag);
-        writer.writePointArray(data, arraySize);
-        uint32_t bytesWritten = writer.bytesWritten();
-        // This should write the length (in 4 bytes) and the array
-        REPORTER_ASSERT(reporter, (4 + arraySize * sizeof(SkPoint)) == bytesWritten);
+        unsigned char data[kArraySize] = {0};
+        TestArraySerialization(data, reporter);
+    }
 
-        unsigned char dataWritten[1024];
-        writer.writeToMemory(dataWritten);
+    // Test readColorArray
+    {
+        SkColor data[kArraySize];
+        TestArraySerialization(data, reporter);
+    }
 
-        // Make sure this fails when it should
-        SkValidatingReadBuffer buffer(dataWritten, bytesWritten);
-        SkPoint dataRead[arraySize];
-        bool success = buffer.readPointArray(dataRead, 32);
-        // This should have failed, since 256 < sizeInBytes
-        REPORTER_ASSERT(reporter, !success);
+    // Test readIntArray
+    {
+        int32_t data[kArraySize];
+        TestArraySerialization(data, reporter);
+    }
 
-        // Make sure this succeeds when it should
-        SkValidatingReadBuffer buffer2(dataWritten, bytesWritten);
-        success = buffer2.readPointArray(dataRead, arraySize);
-        // This should have succeeded, since there are enough bytes to read this
-        REPORTER_ASSERT(reporter, success);
+    // Test readPointArray
+    {
+        SkPoint data[kArraySize];
+        TestArraySerialization(data, reporter);
     }
 
+    // Test readScalarArray
     {
-        static const uint32_t arraySize = 64;
-        SkScalar data[arraySize];
-        SkOrderedWriteBuffer writer(1024);
-        writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag);
-        writer.writeScalarArray(data, arraySize);
-        uint32_t bytesWritten = writer.bytesWritten();
-        // This should write the length (in 4 bytes) and the array
-        REPORTER_ASSERT(reporter, (4 + arraySize * sizeof(SkScalar)) == bytesWritten);
-
-        unsigned char dataWritten[1024];
-        writer.writeToMemory(dataWritten);
-
-        // Make sure this fails when it should
-        SkValidatingReadBuffer buffer(dataWritten, bytesWritten);
-        SkScalar dataRead[arraySize];
-        bool success = buffer.readScalarArray(dataRead, 32);
-        // This should have failed, since 256 < sizeInBytes
-        REPORTER_ASSERT(reporter, !success);
-
-        // Make sure this succeeds when it should
-        SkValidatingReadBuffer buffer2(dataWritten, bytesWritten);
-        success = buffer2.readScalarArray(dataRead, arraySize);
-        // This should have succeeded, since there are enough bytes to read this
-        REPORTER_ASSERT(reporter, success);
+        SkScalar data[kArraySize];
+        TestArraySerialization(data, reporter);
     }
 }