Adding size parameter to read array functions
In some cases, the allocated array into which the data will be read is using getArrayCount() to allocate itself, which should be safe, but some cases use fixed length arrays or compute the array size before reading, which could overflow if the stream is compromised.
To prevent that from happening, I added a check that will verify that the number of bytes to read will not exceed the capacity of the input buffer argument passed to all the read...Array() functions.
I chose to use the byte array for this initial version, so that "size" represents the same value across all read...Array() functions, but I could also use the element count, if it is preferred.
Note : readPointArray and writePointArray are unused, so I could also remove them
BUG=
R=reed@google.com, mtklein@google.com, senorblanco@chromium.org
Author: sugoi@chromium.org
Review URL: https://codereview.chromium.org/37803002
git-svn-id: http://skia.googlecode.com/svn/trunk@12058 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/gyp/tests.gyp b/gyp/tests.gyp
index 6b286bb..ec5bca0 100644
--- a/gyp/tests.gyp
+++ b/gyp/tests.gyp
@@ -123,6 +123,7 @@
'../tests/RTreeTest.cpp',
'../tests/SHA1Test.cpp',
'../tests/ScalarTest.cpp',
+ '../tests/SerializationTest.cpp',
'../tests/ShaderImageFilterTest.cpp',
'../tests/ShaderOpacityTest.cpp',
'../tests/Sk64Test.cpp',
diff --git a/include/core/SkFlattenableBuffers.h b/include/core/SkFlattenableBuffers.h
index 6e44550..8a94bb1 100644
--- a/include/core/SkFlattenableBuffers.h
+++ b/include/core/SkFlattenableBuffers.h
@@ -99,11 +99,24 @@
virtual void readPath(SkPath* path) = 0;
// binary data and arrays
- virtual uint32_t readByteArray(void* value) = 0;
- virtual uint32_t readColorArray(SkColor* colors) = 0;
- virtual uint32_t readIntArray(int32_t* values) = 0;
- virtual uint32_t readPointArray(SkPoint* points) = 0;
- virtual uint32_t readScalarArray(SkScalar* values) = 0;
+
+ /**
+ * In the following read.*Array(...) functions, the size parameter specifies the allocation
+ * size in number of elements (or in bytes, for void*) of the pointer parameter. If the
+ * pointer parameter's size does not match the size to be read, the pointer parameter's memory
+ * will then stay uninitialized, the cursor will be moved to the end of the stream and, in the
+ * case where isValidating() is true, an error flag will be set internally (see
+ * SkValidatingReadBuffer).
+ * If the sizes match, then "size" amount of memory will be read.
+ *
+ * @param size amount of memory expected to be read
+ * @return true if the size parameter matches the size to be read, false otherwise
+ */
+ virtual bool readByteArray(void* value, size_t size) = 0;
+ virtual bool readColorArray(SkColor* colors, size_t size) = 0;
+ virtual bool readIntArray(int32_t* values, size_t size) = 0;
+ virtual bool readPointArray(SkPoint* points, size_t size) = 0;
+ virtual bool readScalarArray(SkScalar* values, size_t size) = 0;
/** This helper peeks into the buffer and reports back the length of the next array in
* the buffer but does not change the state of the buffer.
@@ -127,7 +140,7 @@
SkData* readByteArrayAsData() {
size_t len = this->getArrayCount();
void* buffer = sk_malloc_throw(len);
- (void)this->readByteArray(buffer);
+ (void)this->readByteArray(buffer, len);
return SkData::NewFromMalloc(buffer, len);
}
diff --git a/src/core/SkColorTable.cpp b/src/core/SkColorTable.cpp
index 242ea6b..38a46c5 100644
--- a/src/core/SkColorTable.cpp
+++ b/src/core/SkColorTable.cpp
@@ -93,10 +93,10 @@
fAlphaType = SkToU8(buffer.readUInt());
fCount = buffer.getArrayCount();
fColors = (SkPMColor*)sk_malloc_throw(fCount * sizeof(SkPMColor));
- SkDEBUGCODE(const uint32_t countRead =) buffer.readColorArray(fColors);
+ SkDEBUGCODE(bool success =) buffer.readColorArray(fColors, fCount);
#ifdef SK_DEBUG
SkASSERT((unsigned)fCount <= 256);
- SkASSERT(countRead == fCount);
+ SkASSERT(success);
#endif
}
diff --git a/src/core/SkFlattenableBuffers.cpp b/src/core/SkFlattenableBuffers.cpp
index dbf66da..54d18d1 100644
--- a/src/core/SkFlattenableBuffers.cpp
+++ b/src/core/SkFlattenableBuffers.cpp
@@ -37,7 +37,7 @@
void* SkFlattenableReadBuffer::readFunctionPtr() {
void* proc;
SkASSERT(sizeof(void*) == this->getArrayCount());
- this->readByteArray(&proc);
+ this->readByteArray(&proc, sizeof(void*));
return proc;
}
diff --git a/src/core/SkMallocPixelRef.cpp b/src/core/SkMallocPixelRef.cpp
index 341ac9e..f229e9d 100644
--- a/src/core/SkMallocPixelRef.cpp
+++ b/src/core/SkMallocPixelRef.cpp
@@ -54,7 +54,7 @@
: INHERITED(buffer, NULL) {
fSize = buffer.getArrayCount();
fStorage = sk_malloc_throw(fSize);
- buffer.readByteArray(fStorage);
+ buffer.readByteArray(fStorage, fSize);
if (buffer.readBool()) {
fCTable = SkNEW_ARGS(SkColorTable, (buffer));
} else {
diff --git a/src/core/SkOrderedReadBuffer.cpp b/src/core/SkOrderedReadBuffer.cpp
index d9aa8bd..3184118 100644
--- a/src/core/SkOrderedReadBuffer.cpp
+++ b/src/core/SkOrderedReadBuffer.cpp
@@ -137,38 +137,37 @@
fReader.readPath(path);
}
-uint32_t SkOrderedReadBuffer::readByteArray(void* value) {
- const uint32_t length = fReader.readU32();
- memcpy(value, fReader.skip(SkAlign4(length)), length);
- return length;
+bool SkOrderedReadBuffer::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);
+ return true;
+ }
+ SkASSERT(false);
+ fReader.skip(fReader.available());
+ return false;
}
-uint32_t SkOrderedReadBuffer::readColorArray(SkColor* colors) {
- const uint32_t count = fReader.readU32();
- const uint32_t byteLength = count * sizeof(SkColor);
- memcpy(colors, fReader.skip(SkAlign4(byteLength)), byteLength);
- return count;
+bool SkOrderedReadBuffer::readByteArray(void* value, size_t size) {
+ return readArray(static_cast<unsigned char*>(value), size, sizeof(unsigned char));
}
-uint32_t SkOrderedReadBuffer::readIntArray(int32_t* values) {
- const uint32_t count = fReader.readU32();
- const uint32_t byteLength = count * sizeof(int32_t);
- memcpy(values, fReader.skip(SkAlign4(byteLength)), byteLength);
- return count;
+bool SkOrderedReadBuffer::readColorArray(SkColor* colors, size_t size) {
+ return readArray(colors, size, sizeof(SkColor));
}
-uint32_t SkOrderedReadBuffer::readPointArray(SkPoint* points) {
- const uint32_t count = fReader.readU32();
- const uint32_t byteLength = count * sizeof(SkPoint);
- memcpy(points, fReader.skip(SkAlign4(byteLength)), byteLength);
- return count;
+bool SkOrderedReadBuffer::readIntArray(int32_t* values, size_t size) {
+ return readArray(values, size, sizeof(int32_t));
}
-uint32_t SkOrderedReadBuffer::readScalarArray(SkScalar* values) {
- const uint32_t count = fReader.readU32();
- const uint32_t byteLength = count * sizeof(SkScalar);
- memcpy(values, fReader.skip(SkAlign4(byteLength)), byteLength);
- return count;
+bool SkOrderedReadBuffer::readPointArray(SkPoint* points, size_t size) {
+ return readArray(points, size, sizeof(SkPoint));
+}
+
+bool SkOrderedReadBuffer::readScalarArray(SkScalar* values, size_t size) {
+ return readArray(values, size, sizeof(SkScalar));
}
uint32_t SkOrderedReadBuffer::getArrayCount() {
diff --git a/src/core/SkOrderedReadBuffer.h b/src/core/SkOrderedReadBuffer.h
index d864465..2c4f480 100644
--- a/src/core/SkOrderedReadBuffer.h
+++ b/src/core/SkOrderedReadBuffer.h
@@ -61,11 +61,11 @@
virtual void readPath(SkPath* path) SK_OVERRIDE;
// binary data and arrays
- virtual uint32_t readByteArray(void* value) SK_OVERRIDE;
- virtual uint32_t readColorArray(SkColor* colors) SK_OVERRIDE;
- virtual uint32_t readIntArray(int32_t* values) SK_OVERRIDE;
- virtual uint32_t readPointArray(SkPoint* points) SK_OVERRIDE;
- virtual uint32_t readScalarArray(SkScalar* values) SK_OVERRIDE;
+ virtual bool readByteArray(void* value, size_t size) SK_OVERRIDE;
+ virtual bool readColorArray(SkColor* colors, size_t size) SK_OVERRIDE;
+ virtual bool readIntArray(int32_t* values, size_t size) SK_OVERRIDE;
+ virtual bool readPointArray(SkPoint* points, size_t size) SK_OVERRIDE;
+ virtual bool readScalarArray(SkScalar* values, size_t size) SK_OVERRIDE;
// helpers to get info about arrays and binary data
virtual uint32_t getArrayCount() SK_OVERRIDE;
@@ -113,6 +113,8 @@
}
private:
+ bool readArray(void* value, size_t size, size_t elementSize);
+
SkReader32 fReader;
void* fMemoryPtr;
diff --git a/src/core/SkValidatingReadBuffer.cpp b/src/core/SkValidatingReadBuffer.cpp
index 323273d..9f094f9 100644
--- a/src/core/SkValidatingReadBuffer.cpp
+++ b/src/core/SkValidatingReadBuffer.cpp
@@ -20,8 +20,16 @@
SkValidatingReadBuffer::~SkValidatingReadBuffer() {
}
+void 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;
+ }
+}
+
void SkValidatingReadBuffer::setMemory(const void* data, size_t size) {
- fError = fError || !IsPtrAlign4(data) || (SkAlign4(size) != size);
+ this->validate(IsPtrAlign4(data) && (SkAlign4(size) == size));
if (!fError) {
fReader.setMemory(data, size);
}
@@ -30,7 +38,7 @@
const void* SkValidatingReadBuffer::skip(size_t size) {
size_t inc = SkAlign4(size);
const void* addr = fReader.peek();
- fError = fError || !IsPtrAlign4(addr) || !fReader.isAvailable(inc);
+ this->validate(IsPtrAlign4(addr) && fReader.isAvailable(inc));
if (!fError) {
fReader.skip(size);
}
@@ -45,9 +53,7 @@
bool SkValidatingReadBuffer::readBool() {
uint32_t value = this->readInt();
// Boolean value should be either 0 or 1
- if (value & ~1) {
- fError = true;
- }
+ this->validate(!(value & ~1));
return value != 0;
}
@@ -61,13 +67,13 @@
int32_t SkValidatingReadBuffer::readInt() {
const size_t inc = sizeof(int32_t);
- fError = fError || !IsPtrAlign4(fReader.peek()) || !fReader.isAvailable(inc);
+ this->validate(IsPtrAlign4(fReader.peek()) && fReader.isAvailable(inc));
return fError ? 0 : fReader.readInt();
}
SkScalar SkValidatingReadBuffer::readScalar() {
const size_t inc = sizeof(SkScalar);
- fError = fError || !IsPtrAlign4(fReader.peek()) || !fReader.isAvailable(inc);
+ this->validate(IsPtrAlign4(fReader.peek()) && fReader.isAvailable(inc));
return fError ? 0 : fReader.readScalar();
}
@@ -87,7 +93,7 @@
// skip over the string + '\0' and then pad to a multiple of 4
const size_t alignedSize = SkAlign4(len + 1);
this->skip(alignedSize);
- fError = fError || (cptr[len] != '\0');
+ this->validate(cptr[len] == '\0');
if (!fError) {
string->set(cptr, len);
}
@@ -95,7 +101,7 @@
void* SkValidatingReadBuffer::readEncodedString(size_t* length, SkPaint::TextEncoding encoding) {
const int32_t encodingType = fReader.readInt();
- fError = fError || (encodingType != encoding);
+ this->validate(encodingType == encoding);
*length = this->readInt();
const void* ptr = this->skip(SkAlign4(*length));
void* data = NULL;
@@ -113,7 +119,7 @@
void SkValidatingReadBuffer::readMatrix(SkMatrix* matrix) {
const size_t size = matrix->readFromMemory(fReader.peek());
- fError = fError || (SkAlign4(size) != size);
+ this->validate(SkAlign4(size) == size);
if (!fError) {
(void)this->skip(size);
}
@@ -135,7 +141,7 @@
void SkValidatingReadBuffer::readRegion(SkRegion* region) {
const size_t size = region->readFromMemory(fReader.peek());
- fError = fError || (SkAlign4(size) != size);
+ this->validate(SkAlign4(size) == size);
if (!fError) {
(void)this->skip(size);
}
@@ -143,64 +149,43 @@
void SkValidatingReadBuffer::readPath(SkPath* path) {
const size_t size = path->readFromMemory(fReader.peek());
- fError = fError || (SkAlign4(size) != size);
+ this->validate(SkAlign4(size) == size);
if (!fError) {
(void)this->skip(size);
}
}
-uint32_t SkValidatingReadBuffer::readByteArray(void* value) {
- const uint32_t length = this->readUInt();
- const void* ptr = this->skip(SkAlign4(length));
- if (!fError) {
- memcpy(value, ptr, length);
- return length;
- }
- return 0;
-}
-
-uint32_t SkValidatingReadBuffer::readColorArray(SkColor* colors) {
- const uint32_t count = this->readUInt();
- const uint32_t byteLength = count * sizeof(SkColor);
+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 size_t byteLength = count * elementSize;
const void* ptr = this->skip(SkAlign4(byteLength));
if (!fError) {
- memcpy(colors, ptr, byteLength);
- return count;
+ memcpy(value, ptr, byteLength);
+ return true;
}
- return 0;
+ return false;
}
-uint32_t SkValidatingReadBuffer::readIntArray(int32_t* values) {
- const uint32_t count = this->readUInt();
- const uint32_t byteLength = count * sizeof(int32_t);
- const void* ptr = this->skip(SkAlign4(byteLength));
- if (!fError) {
- memcpy(values, ptr, byteLength);
- return count;
- }
- return 0;
+bool SkValidatingReadBuffer::readByteArray(void* value, size_t size) {
+ return readArray(static_cast<unsigned char*>(value), size, sizeof(unsigned char));
}
-uint32_t SkValidatingReadBuffer::readPointArray(SkPoint* points) {
- const uint32_t count = this->readUInt();
- const uint32_t byteLength = count * sizeof(SkPoint);
- const void* ptr = this->skip(SkAlign4(byteLength));
- if (!fError) {
- memcpy(points, ptr, byteLength);
- return count;
- }
- return 0;
+bool SkValidatingReadBuffer::readColorArray(SkColor* colors, size_t size) {
+ return readArray(colors, size, sizeof(SkColor));
}
-uint32_t SkValidatingReadBuffer::readScalarArray(SkScalar* values) {
- const uint32_t count = this->readUInt();
- const uint32_t byteLength = count * sizeof(SkScalar);
- const void* ptr = this->skip(SkAlign4(byteLength));
- if (!fError) {
- memcpy(values, ptr, byteLength);
- return count;
- }
- return 0;
+bool SkValidatingReadBuffer::readIntArray(int32_t* values, size_t size) {
+ return readArray(values, size, sizeof(int32_t));
+}
+
+bool SkValidatingReadBuffer::readPointArray(SkPoint* points, size_t size) {
+ return readArray(points, size, sizeof(SkPoint));
+}
+
+bool SkValidatingReadBuffer::readScalarArray(SkScalar* values, size_t size) {
+ return readArray(values, size, sizeof(SkScalar));
}
uint32_t SkValidatingReadBuffer::getArrayCount() {
@@ -212,12 +197,17 @@
const int height = this->readInt();
const size_t length = this->readUInt();
// A size of zero means the SkBitmap was simply flattened.
- fError = fError || (length != 0);
+ this->validate(length == 0);
if (fError) {
return;
}
bitmap->unflatten(*this);
- fError = fError || (bitmap->width() != width) || (bitmap->height() != height);
+ this->validate((bitmap->width() == width) && (bitmap->height() == height));
+}
+
+SkTypeface* SkValidatingReadBuffer::readTypeface() {
+ // TODO: Implement this (securely) when needed
+ return NULL;
}
SkFlattenable* SkValidatingReadBuffer::readFlattenable(SkFlattenable::Type type) {
@@ -248,7 +238,7 @@
obj = (*factory)(*this);
// check that we read the amount we expected
uint32_t sizeRead = fReader.offset() - offset;
- fError = fError || (sizeRecorded != sizeRead);
+ this->validate(sizeRecorded == sizeRead);
if (fError) {
// we could try to fix up the offset...
delete obj;
diff --git a/src/core/SkValidatingReadBuffer.h b/src/core/SkValidatingReadBuffer.h
index c854b0a..4d1919b 100644
--- a/src/core/SkValidatingReadBuffer.h
+++ b/src/core/SkValidatingReadBuffer.h
@@ -47,24 +47,24 @@
virtual void readPath(SkPath* path) SK_OVERRIDE;
// binary data and arrays
- virtual uint32_t readByteArray(void* value) SK_OVERRIDE;
- virtual uint32_t readColorArray(SkColor* colors) SK_OVERRIDE;
- virtual uint32_t readIntArray(int32_t* values) SK_OVERRIDE;
- virtual uint32_t readPointArray(SkPoint* points) SK_OVERRIDE;
- virtual uint32_t readScalarArray(SkScalar* values) SK_OVERRIDE;
+ virtual bool readByteArray(void* value, size_t size) SK_OVERRIDE;
+ virtual bool readColorArray(SkColor* colors, size_t size) SK_OVERRIDE;
+ virtual bool readIntArray(int32_t* values, size_t size) SK_OVERRIDE;
+ virtual bool readPointArray(SkPoint* points, size_t size) SK_OVERRIDE;
+ virtual bool readScalarArray(SkScalar* values, size_t size) SK_OVERRIDE;
// helpers to get info about arrays and binary data
virtual uint32_t getArrayCount() SK_OVERRIDE;
virtual void readBitmap(SkBitmap* bitmap) SK_OVERRIDE;
// TODO: Implement this (securely) when needed
- virtual SkTypeface* readTypeface() SK_OVERRIDE { return NULL; }
+ virtual SkTypeface* readTypeface() SK_OVERRIDE;
- virtual void validate(bool isValid) SK_OVERRIDE {
- fError = fError || !isValid;
- }
+ virtual void validate(bool isValid) SK_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) {
diff --git a/src/effects/SkBicubicImageFilter.cpp b/src/effects/SkBicubicImageFilter.cpp
index 778df3f..d667415 100644
--- a/src/effects/SkBicubicImageFilter.cpp
+++ b/src/effects/SkBicubicImageFilter.cpp
@@ -41,8 +41,8 @@
}
SkBicubicImageFilter::SkBicubicImageFilter(SkFlattenableReadBuffer& buffer) : INHERITED(buffer) {
- SkDEBUGCODE(uint32_t readSize =) buffer.readScalarArray(fCoefficients);
- SkASSERT(readSize == 16);
+ SkDEBUGCODE(bool success =) buffer.readScalarArray(fCoefficients, 16);
+ SkASSERT(success);
fScale.fWidth = buffer.readScalar();
fScale.fHeight = buffer.readScalar();
buffer.validate(SkScalarIsFinite(fScale.fWidth) &&
diff --git a/src/effects/SkColorMatrixFilter.cpp b/src/effects/SkColorMatrixFilter.cpp
index f7b283e..d8eb1f1 100644
--- a/src/effects/SkColorMatrixFilter.cpp
+++ b/src/effects/SkColorMatrixFilter.cpp
@@ -308,7 +308,7 @@
SkColorMatrixFilter::SkColorMatrixFilter(SkFlattenableReadBuffer& buffer)
: INHERITED(buffer) {
SkASSERT(buffer.getArrayCount() == 20);
- buffer.readScalarArray(fMatrix.fMat);
+ buffer.readScalarArray(fMatrix.fMat, 20);
this->initState(fMatrix.fMat);
for (int i = 0; i < 20; ++i) {
buffer.validate(SkScalarIsFinite(fMatrix.fMat[i]));
diff --git a/src/effects/SkDashPathEffect.cpp b/src/effects/SkDashPathEffect.cpp
index 4aa46ab..4099058 100644
--- a/src/effects/SkDashPathEffect.cpp
+++ b/src/effects/SkDashPathEffect.cpp
@@ -555,5 +555,5 @@
fCount = buffer.getArrayCount();
fIntervals = (SkScalar*)sk_malloc_throw(sizeof(SkScalar) * fCount);
- buffer.readScalarArray(fIntervals);
+ buffer.readScalarArray(fIntervals, fCount);
}
diff --git a/src/effects/SkEmbossMaskFilter.cpp b/src/effects/SkEmbossMaskFilter.cpp
index 31beb51..2d31250 100644
--- a/src/effects/SkEmbossMaskFilter.cpp
+++ b/src/effects/SkEmbossMaskFilter.cpp
@@ -132,7 +132,7 @@
SkEmbossMaskFilter::SkEmbossMaskFilter(SkFlattenableReadBuffer& buffer)
: SkMaskFilter(buffer) {
SkASSERT(buffer.getArrayCount() == sizeof(Light));
- buffer.readByteArray(&fLight);
+ buffer.readByteArray(&fLight, sizeof(Light));
SkASSERT(fLight.fPad == 0); // for the font-cache lookup to be clean
fBlurSigma = buffer.readScalar();
#ifndef DELETE_THIS_CODE_WHEN_SKPS_ARE_REBUILT_AT_V13_AND_ALL_OTHER_INSTANCES_TOO
diff --git a/src/effects/SkKernel33MaskFilter.cpp b/src/effects/SkKernel33MaskFilter.cpp
index 485001b..821eebd 100644
--- a/src/effects/SkKernel33MaskFilter.cpp
+++ b/src/effects/SkKernel33MaskFilter.cpp
@@ -120,8 +120,8 @@
SkKernel33MaskFilter::SkKernel33MaskFilter(SkFlattenableReadBuffer& rb)
: SkKernel33ProcMaskFilter(rb) {
- SkDEBUGCODE(const uint32_t count = )rb.readIntArray(&fKernel[0][0]);
- SkASSERT(9 == count);
+ SkDEBUGCODE(bool success = )rb.readIntArray(&fKernel[0][0], 9);
+ SkASSERT(success);
fShift = rb.readInt();
}
diff --git a/src/effects/SkMatrixConvolutionImageFilter.cpp b/src/effects/SkMatrixConvolutionImageFilter.cpp
index cac30e6..4864aec 100644
--- a/src/effects/SkMatrixConvolutionImageFilter.cpp
+++ b/src/effects/SkMatrixConvolutionImageFilter.cpp
@@ -59,18 +59,20 @@
SkMatrixConvolutionImageFilter::SkMatrixConvolutionImageFilter(SkFlattenableReadBuffer& buffer)
: INHERITED(buffer) {
+ // We need to be able to read at most SK_MaxS32 bytes, so divide that
+ // by the size of a scalar to know how many scalars we can read.
+ static const int32_t kMaxSize = SK_MaxS32 / sizeof(SkScalar);
fKernelSize.fWidth = buffer.readInt();
fKernelSize.fHeight = buffer.readInt();
if ((fKernelSize.fWidth >= 1) && (fKernelSize.fHeight >= 1) &&
// Make sure size won't be larger than a signed int,
// which would still be extremely large for a kernel,
// but we don't impose a hard limit for kernel size
- (SK_MaxS32 / fKernelSize.fWidth >= fKernelSize.fHeight)) {
- uint32_t size = fKernelSize.fWidth * fKernelSize.fHeight;
+ (kMaxSize / fKernelSize.fWidth >= fKernelSize.fHeight)) {
+ size_t size = fKernelSize.fWidth * fKernelSize.fHeight;
fKernel = SkNEW_ARRAY(SkScalar, size);
- uint32_t readSize = buffer.readScalarArray(fKernel);
- SkASSERT(readSize == size);
- buffer.validate(readSize == size);
+ SkDEBUGCODE(bool success =) buffer.readScalarArray(fKernel, size);
+ SkASSERT(success);
} else {
fKernel = 0;
}
diff --git a/src/effects/SkMergeImageFilter.cpp b/src/effects/SkMergeImageFilter.cpp
index 4de1093..93e2335 100755
--- a/src/effects/SkMergeImageFilter.cpp
+++ b/src/effects/SkMergeImageFilter.cpp
@@ -161,10 +161,9 @@
if (hasModes) {
this->initAllocModes();
int nbInputs = countInputs();
- bool sizeMatches = buffer.getArrayCount() == nbInputs * sizeof(fModes[0]);
- buffer.validate(sizeMatches);
- SkASSERT(sizeMatches);
- buffer.readByteArray(fModes);
+ size_t size = nbInputs * sizeof(fModes[0]);
+ SkASSERT(buffer.getArrayCount() == size);
+ buffer.readByteArray(fModes, size);
for (int i = 0; i < nbInputs; ++i) {
buffer.validate(SkIsValidMode((SkXfermode::Mode)fModes[i]));
}
diff --git a/src/effects/SkTableColorFilter.cpp b/src/effects/SkTableColorFilter.cpp
index 083b54c..e15baf6 100644
--- a/src/effects/SkTableColorFilter.cpp
+++ b/src/effects/SkTableColorFilter.cpp
@@ -189,7 +189,7 @@
size_t size = buffer.getArrayCount();
SkASSERT(size <= sizeof(storage));
- buffer.readByteArray(storage);
+ buffer.readByteArray(storage, size);
SkDEBUGCODE(size_t raw = ) SkPackBits::Unpack8(storage, size, fStorage);
diff --git a/src/effects/SkTableMaskFilter.cpp b/src/effects/SkTableMaskFilter.cpp
index 5bff4de..8d3b81a 100644
--- a/src/effects/SkTableMaskFilter.cpp
+++ b/src/effects/SkTableMaskFilter.cpp
@@ -77,7 +77,7 @@
SkTableMaskFilter::SkTableMaskFilter(SkFlattenableReadBuffer& rb)
: INHERITED(rb) {
SkASSERT(256 == rb.getArrayCount());
- rb.readByteArray(fTable);
+ rb.readByteArray(fTable, 256);
}
///////////////////////////////////////////////////////////////////////////////
diff --git a/src/effects/gradients/SkGradientShader.cpp b/src/effects/gradients/SkGradientShader.cpp
index c90adfe..2776199 100644
--- a/src/effects/gradients/SkGradientShader.cpp
+++ b/src/effects/gradients/SkGradientShader.cpp
@@ -159,7 +159,7 @@
} else {
fOrigColors = fStorage;
}
- buffer.readColorArray(fOrigColors);
+ buffer.readColorArray(fOrigColors, colorCount);
{
uint32_t packed = buffer.readUInt();
diff --git a/src/images/SkImageRef.cpp b/src/images/SkImageRef.cpp
index ead835f..1a8284b 100644
--- a/src/images/SkImageRef.cpp
+++ b/src/images/SkImageRef.cpp
@@ -176,7 +176,7 @@
size_t length = buffer.getArrayCount();
fStream = SkNEW_ARGS(SkMemoryStream, (length));
- buffer.readByteArray((void*)fStream->getMemoryBase());
+ buffer.readByteArray((void*)fStream->getMemoryBase(), length);
fPrev = fNext = NULL;
fFactory = NULL;
diff --git a/tests/SerializationTest.cpp b/tests/SerializationTest.cpp
new file mode 100644
index 0000000..d2c6925
--- /dev/null
+++ b/tests/SerializationTest.cpp
@@ -0,0 +1,150 @@
+/*
+ * 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 "SkOrderedWriteBuffer.h"
+#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);
+
+ unsigned char dataWritten[1024];
+ writer.writeToMemory(dataWritten);
+
+ // 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);
+
+ // 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);
+ }
+
+ {
+ static const uint32_t arraySize = 64;
+ SkColor data[arraySize];
+ SkOrderedWriteBuffer writer(1024);
+ writer.setFlags(SkOrderedWriteBuffer::kValidation_Flag);
+ 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);
+
+ unsigned char dataWritten[1024];
+ writer.writeToMemory(dataWritten);
+
+ // 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);
+
+ // 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);
+ }
+
+ {
+ 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);
+
+ unsigned char dataWritten[1024];
+ writer.writeToMemory(dataWritten);
+
+ // 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);
+
+ // 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);
+ }
+
+ {
+ 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 dataWritten[1024];
+ writer.writeToMemory(dataWritten);
+
+ // 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);
+
+ // 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);
+ }
+
+ {
+ 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);
+ }
+}
+
+#include "TestClassDef.h"
+DEFINE_TESTCLASS("Serialization", SerializationClass, Tests)