Rewrite SkVertices serialization to use SkReadBuffer/SkWriteBuffer
These classes are much safer (there's no way to safely deserialize a string with SkReader32 without knowledge of how it works internally). Prior to this CL, SkVertices was the only complex type that had manual serialization using the lower level types - now it works like everything else. Additionally: the versioning can now be tied to picture versions going forward (like everything else). Bug: oss-fuzz:22909 Bug: oss-fuzz:22918 Bug: skia:9984 Bug: skia:10304 Change-Id: I3cf537eb765b5c8ce98b554c0f200e5d67c33d14 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/293349 Reviewed-by: Mike Klein <mtklein@google.com> Commit-Queue: Brian Osman <brianosman@google.com>
This commit is contained in:
Brian Osman
Skia Commit-Bot
parent
0bfae282f4
commit
6b3d6e9210
@ -158,6 +158,7 @@ public:
|
||||
std::unique_ptr<uint8_t[]> fIntermediateFanIndices;
|
||||
|
||||
friend class SkVertices;
|
||||
friend class SkVerticesPriv;
|
||||
};
|
||||
|
||||
uint32_t uniqueID() const { return fUniqueID; }
|
||||
@ -166,18 +167,6 @@ public:
|
||||
// returns approximate byte size of the vertices object
|
||||
size_t approximateSize() const;
|
||||
|
||||
/**
|
||||
* Recreate a vertices from a buffer previously created by calling encode().
|
||||
* Returns null if the data is corrupt or the length is incorrect for the contents.
|
||||
*/
|
||||
static sk_sp<SkVertices> Decode(const void* buffer, size_t length);
|
||||
|
||||
/**
|
||||
* Pack the vertices object into a byte buffer. This can be used to recreate the vertices
|
||||
* by calling Decode() with the buffer.
|
||||
*/
|
||||
sk_sp<SkData> encode() const;
|
||||
|
||||
// Provides access to functions that aren't part of the public API.
|
||||
SkVerticesPriv priv();
|
||||
const SkVerticesPriv priv() const;
|
||||
|
||||
@ -15,6 +15,7 @@
|
||||
#include "src/core/SkPictureRecord.h"
|
||||
#include "src/core/SkReadBuffer.h"
|
||||
#include "src/core/SkTextBlobPriv.h"
|
||||
#include "src/core/SkVerticesPriv.h"
|
||||
#include "src/core/SkWriteBuffer.h"
|
||||
|
||||
#include <new>
|
||||
@ -174,7 +175,7 @@ void SkPictureData::flattenToBuffer(SkWriteBuffer& buffer, bool textBlobsOnly) c
|
||||
if (!fVertices.empty()) {
|
||||
write_tag_size(buffer, SK_PICT_VERTICES_BUFFER_TAG, fVertices.count());
|
||||
for (const auto& vert : fVertices) {
|
||||
buffer.writeDataAsByteArray(vert->encode().get());
|
||||
vert->priv().encode(buffer);
|
||||
}
|
||||
}
|
||||
|
||||
@ -374,10 +375,6 @@ bool SkPictureData::parseStreamTag(SkStream* stream,
|
||||
static sk_sp<SkImage> create_image_from_buffer(SkReadBuffer& buffer) {
|
||||
return buffer.readImage();
|
||||
}
|
||||
static sk_sp<SkVertices> create_vertices_from_buffer(SkReadBuffer& buffer) {
|
||||
auto data = buffer.readByteArrayAsData();
|
||||
return data ? SkVertices::Decode(data->data(), data->size()) : nullptr;
|
||||
}
|
||||
|
||||
static sk_sp<SkDrawable> create_drawable_from_buffer(SkReadBuffer& buffer) {
|
||||
return sk_sp<SkDrawable>((SkDrawable*)buffer.readFlattenable(SkFlattenable::kSkDrawable_Type));
|
||||
@ -440,7 +437,7 @@ void SkPictureData::parseBufferTag(SkReadBuffer& buffer, uint32_t tag, uint32_t
|
||||
new_array_from_buffer(buffer, size, fTextBlobs, SkTextBlobPriv::MakeFromBuffer);
|
||||
break;
|
||||
case SK_PICT_VERTICES_BUFFER_TAG:
|
||||
new_array_from_buffer(buffer, size, fVertices, create_vertices_from_buffer);
|
||||
new_array_from_buffer(buffer, size, fVertices, SkVerticesPriv::Decode);
|
||||
break;
|
||||
case SK_PICT_IMAGE_BUFFER_TAG:
|
||||
new_array_from_buffer(buffer, size, fImages, create_image_from_buffer);
|
||||
|
||||
@ -75,6 +75,7 @@ public:
|
||||
// V72: SkColorFilter_Matrix domain (rgba vs. hsla)
|
||||
// V73: Use SkColor4f in per-edge AA quad API
|
||||
// V74: MorphologyImageFilter internal radius is SkScaler
|
||||
// V75: SkVertices switched from unsafe use of SkReader32 to SkReadBuffer (like everything else)
|
||||
|
||||
enum Version {
|
||||
kTileModeInBlurImageFilter_Version = 56,
|
||||
@ -96,10 +97,11 @@ public:
|
||||
kMatrixColorFilterDomain_Version = 72,
|
||||
kEdgeAAQuadColor4f_Version = 73,
|
||||
kMorphologyTakesScalar_Version = 74,
|
||||
kVerticesUseReadBuffer_Version = 75,
|
||||
|
||||
// Only SKPs within the min/current picture version range (inclusive) can be read.
|
||||
kMin_Version = kTileModeInBlurImageFilter_Version,
|
||||
kCurrent_Version = kMorphologyTakesScalar_Version
|
||||
kCurrent_Version = kVerticesUseReadBuffer_Version
|
||||
};
|
||||
|
||||
static_assert(kMin_Version <= 62, "Remove kFontAxes_bad from SkFontDescriptor.cpp");
|
||||
|
||||
@ -11,11 +11,11 @@
|
||||
#include "include/private/SkTo.h"
|
||||
#include "src/core/SkCanvasPriv.h"
|
||||
#include "src/core/SkOpts.h"
|
||||
#include "src/core/SkReader32.h"
|
||||
#include "src/core/SkReadBuffer.h"
|
||||
#include "src/core/SkSafeMath.h"
|
||||
#include "src/core/SkSafeRange.h"
|
||||
#include "src/core/SkVerticesPriv.h"
|
||||
#include "src/core/SkWriter32.h"
|
||||
#include "src/core/SkWriteBuffer.h"
|
||||
#include <atomic>
|
||||
#include <new>
|
||||
|
||||
@ -370,173 +370,138 @@ bool SkVerticesPriv::hasUsage(SkVertices::Attribute::Usage u) const {
|
||||
|
||||
///////////////////////////////////////////////////////////////////////////////////////////////////
|
||||
|
||||
enum EncodedVerticesVersions {
|
||||
kNamedMarkers_Version = 4, // Marker IDs changed to strings
|
||||
|
||||
kCurrent_Version = kNamedMarkers_Version
|
||||
};
|
||||
|
||||
struct EncodedAttribute {
|
||||
SkVertices::Attribute::Type fType;
|
||||
SkVertices::Attribute::Usage fUsage;
|
||||
bool fHasMarkerName;
|
||||
};
|
||||
|
||||
struct Header_v4 {
|
||||
uint32_t fPacked;
|
||||
int32_t fVertexCount;
|
||||
int32_t fIndexCount;
|
||||
int32_t fAttributeCount;
|
||||
// [EncodedAttributes] + [MarkerNames] + [pos] + [customData] + [texs] + [colors] + [indices]
|
||||
};
|
||||
|
||||
#define kCurrentHeaderSize sizeof(Header_v4)
|
||||
|
||||
// storage = packed | vertex_count | index_count | attr_count
|
||||
// | pos[] | custom[] | texs[] | colors[] | indices[]
|
||||
|
||||
#define kMode_Mask 0x0FF
|
||||
#define kHasTexs_Mask 0x100
|
||||
#define kHasColors_Mask 0x200
|
||||
// new as of 3/2020
|
||||
#define kVersion_Shift 24
|
||||
#define kVersion_Mask (0xFF << kVersion_Shift)
|
||||
|
||||
sk_sp<SkData> SkVertices::encode() const {
|
||||
// packed has room for addtional flags in the future (e.g. versioning)
|
||||
uint32_t packed = static_cast<uint32_t>(fMode);
|
||||
void SkVerticesPriv::encode(SkWriteBuffer& buffer) const {
|
||||
// packed has room for additional flags in the future
|
||||
uint32_t packed = static_cast<uint32_t>(fVertices->fMode);
|
||||
SkASSERT((packed & ~kMode_Mask) == 0); // our mode fits in the mask bits
|
||||
if (fTexs) {
|
||||
if (fVertices->fTexs) {
|
||||
packed |= kHasTexs_Mask;
|
||||
}
|
||||
if (fColors) {
|
||||
if (fVertices->fColors) {
|
||||
packed |= kHasColors_Mask;
|
||||
}
|
||||
packed |= kCurrent_Version << kVersion_Shift;
|
||||
|
||||
size_t attrSize = SkAlign4(sizeof(EncodedAttribute) * fAttributeCount);
|
||||
for (int i = 0; i < fAttributeCount; ++i) {
|
||||
if (fAttributes[i].fMarkerName) {
|
||||
attrSize += SkWriter32::WriteStringSize(fAttributes[i].fMarkerName);
|
||||
}
|
||||
}
|
||||
|
||||
Sizes sizes = this->getSizes();
|
||||
SkVertices::Sizes sizes = fVertices->getSizes();
|
||||
SkASSERT(!sizes.fBuilderTriFanISize);
|
||||
// need to force alignment to 4 for SkWriter32 -- will pad w/ 0s as needed
|
||||
const size_t size = SkAlign4(kCurrentHeaderSize + attrSize + sizes.fArrays);
|
||||
|
||||
sk_sp<SkData> data = SkData::MakeUninitialized(size);
|
||||
SkWriter32 writer(data->writable_data(), data->size());
|
||||
|
||||
// Header
|
||||
writer.write32(packed);
|
||||
writer.write32(fVertexCount);
|
||||
writer.write32(fIndexCount);
|
||||
writer.write32(fAttributeCount);
|
||||
buffer.writeUInt(packed);
|
||||
buffer.writeInt(fVertices->fVertexCount);
|
||||
buffer.writeInt(fVertices->fIndexCount);
|
||||
buffer.writeInt(fVertices->fAttributeCount);
|
||||
|
||||
// Encoded attributes (may not be 4 byte aligned)
|
||||
EncodedAttribute* encodedAttrs =
|
||||
(EncodedAttribute*)writer.reservePad(fAttributeCount * sizeof(EncodedAttribute));
|
||||
for (int i = 0; i < fAttributeCount; ++i) {
|
||||
encodedAttrs[i] = {fAttributes[i].fType, fAttributes[i].fUsage,
|
||||
SkToBool(fAttributes[i].fMarkerName)};
|
||||
}
|
||||
|
||||
// Marker names
|
||||
for (int i = 0; i < fAttributeCount; ++i) {
|
||||
if (fAttributes[i].fMarkerName) {
|
||||
writer.writeString(fAttributes[i].fMarkerName);
|
||||
}
|
||||
// Attribute metadata
|
||||
for (int i = 0; i < fVertices->fAttributeCount; ++i) {
|
||||
buffer.writeInt(static_cast<int>(fVertices->fAttributes[i].fType));
|
||||
buffer.writeInt(static_cast<int>(fVertices->fAttributes[i].fUsage));
|
||||
buffer.writeString(fVertices->fAttributes[i].fMarkerName);
|
||||
}
|
||||
|
||||
// Data arrays
|
||||
writer.write(fPositions, sizes.fVSize);
|
||||
writer.write(fCustomData, sizes.fDSize);
|
||||
writer.write(fTexs, sizes.fTSize);
|
||||
writer.write(fColors, sizes.fCSize);
|
||||
buffer.writeByteArray(fVertices->fPositions, sizes.fVSize);
|
||||
buffer.writeByteArray(fVertices->fCustomData, sizes.fDSize);
|
||||
buffer.writeByteArray(fVertices->fTexs, sizes.fTSize);
|
||||
buffer.writeByteArray(fVertices->fColors, sizes.fCSize);
|
||||
// if index-count is odd, we won't be 4-bytes aligned, so we call the pad version
|
||||
writer.writePad(fIndices, sizes.fISize);
|
||||
|
||||
return data;
|
||||
buffer.writeByteArray(fVertices->fIndices, sizes.fISize);
|
||||
}
|
||||
|
||||
sk_sp<SkVertices> SkVertices::Decode(const void* data, size_t length) {
|
||||
if (length < sizeof(Header_v4)) {
|
||||
sk_sp<SkVertices> SkVerticesPriv::Decode(SkReadBuffer& buffer) {
|
||||
if (buffer.isVersionLT(SkPicturePriv::kVerticesUseReadBuffer_Version)) {
|
||||
// Old versions used an embedded blob that was serialized with SkWriter32/SkReader32.
|
||||
// We don't support loading those, but skip over the vertices to keep the buffer valid.
|
||||
auto data = buffer.readByteArrayAsData();
|
||||
(void)data;
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
SkReader32 reader(data, length);
|
||||
SkSafeRange safe;
|
||||
auto decode = [](SkReadBuffer& buffer) -> sk_sp<SkVertices> {
|
||||
SkSafeRange safe;
|
||||
|
||||
const uint32_t packed = reader.readInt();
|
||||
const unsigned version = safe.checkLE<unsigned>((packed & kVersion_Mask) >> kVersion_Shift,
|
||||
kCurrent_Version);
|
||||
const int vertexCount = safe.checkGE(reader.readInt(), 0);
|
||||
const int indexCount = safe.checkGE(reader.readInt(), 0);
|
||||
const int attrCount = safe.checkGE(reader.readInt(), 0);
|
||||
const VertexMode mode = safe.checkLE<VertexMode>(packed & kMode_Mask,
|
||||
SkVertices::kLast_VertexMode);
|
||||
const bool hasTexs = SkToBool(packed & kHasTexs_Mask);
|
||||
const bool hasColors = SkToBool(packed & kHasColors_Mask);
|
||||
const uint32_t packed = buffer.readUInt();
|
||||
const int vertexCount = safe.checkGE(buffer.readInt(), 0);
|
||||
const int indexCount = safe.checkGE(buffer.readInt(), 0);
|
||||
const int attrCount = safe.checkGE(buffer.readInt(), 0);
|
||||
const SkVertices::VertexMode mode = safe.checkLE<SkVertices::VertexMode>(
|
||||
packed & kMode_Mask, SkVertices::kLast_VertexMode);
|
||||
const bool hasTexs = SkToBool(packed & kHasTexs_Mask);
|
||||
const bool hasColors = SkToBool(packed & kHasColors_Mask);
|
||||
|
||||
if (!safe // Invalid header fields
|
||||
|| attrCount > kMaxCustomAttributes // Too many custom attributes?
|
||||
|| version < kNamedMarkers_Version // Old (unsupported) version
|
||||
|| (attrCount > 0 && (hasTexs || hasColors))) { // Overspecified (incompatible features)
|
||||
return nullptr;
|
||||
}
|
||||
if (!safe // Invalid header fields
|
||||
|| attrCount > SkVertices::kMaxCustomAttributes // Too many custom attributes?
|
||||
|| (attrCount > 0 && (hasTexs || hasColors))) { // Overspecified (incompatible features)
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
if (!reader.isAvailable(attrCount * sizeof(EncodedAttribute))) {
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
Attribute attrs[kMaxCustomAttributes];
|
||||
const EncodedAttribute* encodedAttrs =
|
||||
(const EncodedAttribute*)reader.skip(attrCount * sizeof(EncodedAttribute));
|
||||
for (int i = 0; i < attrCount; ++i) {
|
||||
attrs[i] = Attribute(encodedAttrs[i].fType, encodedAttrs[i].fUsage,
|
||||
encodedAttrs[i].fHasMarkerName ? reader.readString() : nullptr);
|
||||
}
|
||||
|
||||
const Desc desc{
|
||||
mode, vertexCount, indexCount, hasTexs, hasColors, attrCount ? attrs : nullptr, attrCount
|
||||
};
|
||||
Sizes sizes(desc);
|
||||
if (!sizes.isValid()) {
|
||||
return nullptr;
|
||||
}
|
||||
// logically we can be only 2-byte aligned, but our buffer is always 4-byte aligned
|
||||
if (reader.available() != SkAlign4(sizes.fArrays)) {
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
Builder builder(desc);
|
||||
if (!builder.isValid()) {
|
||||
return nullptr;
|
||||
}
|
||||
SkSafeMath safe_math;
|
||||
|
||||
reader.read(builder.positions(), sizes.fVSize);
|
||||
reader.read(builder.customData(), sizes.fDSize);
|
||||
reader.read(builder.texCoords(), sizes.fTSize);
|
||||
reader.read(builder.colors(), sizes.fCSize);
|
||||
size_t isize = (mode == kTriangleFan_VertexMode) ? sizes.fBuilderTriFanISize : sizes.fISize;
|
||||
reader.read(builder.indices(), isize);
|
||||
if (indexCount > 0) {
|
||||
// validate that the indices are in range
|
||||
const uint16_t* indices = builder.indices();
|
||||
for (int i = 0; i < indexCount; ++i) {
|
||||
if (indices[i] >= (unsigned)vertexCount) {
|
||||
SkVertices::Attribute attrs[SkVertices::kMaxCustomAttributes];
|
||||
SkString attrNames[SkVertices::kMaxCustomAttributes];
|
||||
for (int i = 0; i < attrCount; ++i) {
|
||||
auto type = buffer.checkRange(SkVertices::Attribute::Type::kFloat,
|
||||
SkVertices::Attribute::Type::kByte4_unorm);
|
||||
auto usage = buffer.checkRange(SkVertices::Attribute::Usage::kRaw,
|
||||
SkVertices::Attribute::Usage::kPosition);
|
||||
buffer.readString(&attrNames[i]);
|
||||
const char* markerName = attrNames[i].isEmpty() ? nullptr : attrNames[i].c_str();
|
||||
if (markerName && !SkCanvasPriv::ValidateMarker(markerName)) {
|
||||
return nullptr;
|
||||
}
|
||||
attrs[i] = SkVertices::Attribute(type, usage, markerName);
|
||||
}
|
||||
}
|
||||
|
||||
if (!safe_math.ok()) {
|
||||
return nullptr;
|
||||
// Ensure that all of the attribute metadata was valid before proceeding
|
||||
if (!buffer.isValid()) {
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
const SkVertices::Desc desc{mode, vertexCount, indexCount, hasTexs, hasColors,
|
||||
attrCount ? attrs : nullptr, attrCount};
|
||||
SkVertices::Sizes sizes(desc);
|
||||
if (!sizes.isValid()) {
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
SkVertices::Builder builder(desc);
|
||||
if (!builder.isValid()) {
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
buffer.readByteArray(builder.positions(), sizes.fVSize);
|
||||
buffer.readByteArray(builder.customData(), sizes.fDSize);
|
||||
buffer.readByteArray(builder.texCoords(), sizes.fTSize);
|
||||
buffer.readByteArray(builder.colors(), sizes.fCSize);
|
||||
size_t isize = (mode == SkVertices::kTriangleFan_VertexMode) ? sizes.fBuilderTriFanISize
|
||||
: sizes.fISize;
|
||||
buffer.readByteArray(builder.indices(), isize);
|
||||
|
||||
if (!buffer.isValid()) {
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
if (indexCount > 0) {
|
||||
// validate that the indices are in range
|
||||
const uint16_t* indices = builder.indices();
|
||||
for (int i = 0; i < indexCount; ++i) {
|
||||
if (indices[i] >= (unsigned)vertexCount) {
|
||||
return nullptr;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return builder.detach();
|
||||
};
|
||||
|
||||
if (auto verts = decode(buffer)) {
|
||||
return verts;
|
||||
}
|
||||
return builder.detach();
|
||||
buffer.validate(false);
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
void SkVertices::operator delete(void* p) {
|
||||
|
||||
@ -10,7 +10,10 @@
|
||||
|
||||
#include "include/core/SkVertices.h"
|
||||
|
||||
struct SkVertices_DeprecatedBone { float values[6]; };
|
||||
class SkReadBuffer;
|
||||
class SkWriteBuffer;
|
||||
|
||||
struct SkVertices_DeprecatedBone { float values[6]; };
|
||||
|
||||
/** Class that adds methods to SkVertices that are only intended for use internal to Skia.
|
||||
This class is purely a privileged window into SkVertices. It should never have additional
|
||||
@ -41,6 +44,9 @@ public:
|
||||
// Never called due to RVO in priv(), but must exist for MSVC 2017.
|
||||
SkVerticesPriv(const SkVerticesPriv&) = default;
|
||||
|
||||
void encode(SkWriteBuffer&) const;
|
||||
static sk_sp<SkVertices> Decode(SkReadBuffer&);
|
||||
|
||||
private:
|
||||
explicit SkVerticesPriv(SkVertices* vertices) : fVertices(vertices) {}
|
||||
SkVerticesPriv& operator=(const SkVerticesPriv&) = delete;
|
||||
|
||||
@ -8,7 +8,10 @@
|
||||
#include "include/core/SkCanvas.h"
|
||||
#include "include/core/SkSurface.h"
|
||||
#include "include/core/SkVertices.h"
|
||||
#include "src/core/SkAutoMalloc.h"
|
||||
#include "src/core/SkReadBuffer.h"
|
||||
#include "src/core/SkVerticesPriv.h"
|
||||
#include "src/core/SkWriteBuffer.h"
|
||||
#include "tests/Test.h"
|
||||
#include "tools/ToolUtils.h"
|
||||
|
||||
@ -73,9 +76,16 @@ static bool equal(const SkVertices* vert0, const SkVertices* vert1) {
|
||||
}
|
||||
|
||||
static void self_test(sk_sp<SkVertices> v0, skiatest::Reporter* reporter) {
|
||||
sk_sp<SkData> data = v0->encode();
|
||||
sk_sp<SkVertices> v1 = SkVertices::Decode(data->data(), data->size());
|
||||
SkBinaryWriteBuffer writer;
|
||||
v0->priv().encode(writer);
|
||||
|
||||
SkAutoMalloc buf(writer.bytesWritten());
|
||||
writer.writeToMemory(buf.get());
|
||||
SkReadBuffer reader(buf.get(), writer.bytesWritten());
|
||||
|
||||
sk_sp<SkVertices> v1 = SkVerticesPriv::Decode(reader);
|
||||
|
||||
REPORTER_ASSERT(reporter, v1 != nullptr);
|
||||
REPORTER_ASSERT(reporter, v0->uniqueID() != 0);
|
||||
REPORTER_ASSERT(reporter, v1->uniqueID() != 0);
|
||||
REPORTER_ASSERT(reporter, v0->uniqueID() != v1->uniqueID());
|
||||
|
||||
Loading…
Reference in New Issue
Block a user