From 9e5f85e89d03a850d435fc951e74e9861a0c1bdd Mon Sep 17 00:00:00 2001 From: "commit-bot@chromium.org" Date: Wed, 12 Mar 2014 14:46:41 +0000 Subject: Fixing SkPicture serialization Fixed a few issues while attempting to use the new serialization path for SkPicture inside a fuzzer: - SkReadBuffer and SkValidatingReadBuffer both had a fReader member instead of sharing the same member, which leads to problems if a base class function is used - In SkPicture, a header is now written as a single chunk of data, so it also has to be read as a single chunk of data - In the SkPicturePlayback destructor, a bad deserialization would lead to a crash if we don't safely unref fOpData - Also in SkPicturePlayback, if we only use a ReadBuffer for the whole deserialization, additional tags must be added to parseBufferTag() - SkValidatingReadBuffer::readBitmap() was broken, but this path wasn't usen't since the only use case for SkValidatingReadBuffer is currently image filters and bitmaps are unflattened as part of the deserialization of SkBitmapSource - SkPictureImageFilter was not deserializable. Added it to SkGlobalInitialization* - Added a test that exercises the SkPicture serialization / deserialization code BUG=skia: R=senorblanco@google.com, senorblanco@chromium.org, reed@google.com, robertphillips@google.com Author: sugoi@chromium.org Review URL: https://codereview.chromium.org/195223003 git-svn-id: http://skia.googlecode.com/svn/trunk@13764 2bbb7eff-a529-9590-31e7-b0007b416f81 --- src/core/SkPicture.cpp | 51 ++++++++++++--------------- src/core/SkPicturePlayback.cpp | 37 ++++++++++++++++++- src/core/SkPicturePlayback.h | 1 + src/core/SkValidatingReadBuffer.cpp | 4 +-- src/core/SkValidatingReadBuffer.h | 1 - src/ports/SkGlobalInitialization_chromium.cpp | 2 ++ src/ports/SkGlobalInitialization_default.cpp | 2 ++ 7 files changed, 65 insertions(+), 33 deletions(-) (limited to 'src') diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp index f83a5fb9aa..9270236572 100644 --- a/src/core/SkPicture.cpp +++ b/src/core/SkPicture.cpp @@ -260,26 +260,29 @@ void SkPicture::draw(SkCanvas* surface, SkDrawPictureCallback* callback) { #include "SkStream.h" static const char kMagic[] = { 's', 'k', 'i', 'a', 'p', 'i', 'c', 't' }; -static const size_t kHeaderSize = sizeof(kMagic) + sizeof(SkPictInfo); -bool SkPicture::InternalOnly_StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) { - if (NULL == stream) { +bool SkPicture::IsValidPictInfo(const SkPictInfo& info) { + if (0 != memcmp(info.fMagic, kMagic, sizeof(kMagic))) { return false; } - // Check magic bytes. - char magic[sizeof(kMagic)]; - if (!stream->read(magic, sizeof(kMagic)) || - (0 != memcmp(magic, kMagic, sizeof(kMagic)))) { + if (info.fVersion < MIN_PICTURE_VERSION || + info.fVersion > CURRENT_PICTURE_VERSION) { return false; } - SkPictInfo info; - if (!stream->read(&info, sizeof(SkPictInfo))) { + return true; +} + +bool SkPicture::InternalOnly_StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) { + if (NULL == stream) { return false; } - if (info.fVersion < MIN_PICTURE_VERSION || info.fVersion > CURRENT_PICTURE_VERSION) { + // Check magic bytes. + SkPictInfo info; + SkASSERT(sizeof(kMagic) == sizeof(info.fMagic)); + if (!stream->read(&info, sizeof(info)) || !IsValidPictInfo(info)) { return false; } @@ -291,19 +294,9 @@ bool SkPicture::InternalOnly_StreamIsSKP(SkStream* stream, SkPictInfo* pInfo) { bool SkPicture::InternalOnly_BufferIsSKP(SkReadBuffer& buffer, SkPictInfo* pInfo) { // Check magic bytes. - char magic[sizeof(kMagic)]; - - if (!buffer.readByteArray(magic, sizeof(kMagic)) || - (0 != memcmp(magic, kMagic, sizeof(kMagic)))) { - return false; - } - SkPictInfo info; - if (!buffer.readByteArray(&info, sizeof(SkPictInfo))) { - return false; - } - - if (info.fVersion < MIN_PICTURE_VERSION || info.fVersion > CURRENT_PICTURE_VERSION) { + SkASSERT(sizeof(kMagic) == sizeof(info.fMagic)); + if (!buffer.readByteArray(&info, sizeof(info)) || !IsValidPictInfo(info)) { return false; } @@ -361,13 +354,13 @@ SkPicture* SkPicture::CreateFromBuffer(SkReadBuffer& buffer) { return SkNEW_ARGS(SkPicture, (playback, info.fWidth, info.fHeight)); } -void SkPicture::createHeader(void* header) const { +void SkPicture::createHeader(SkPictInfo* info) const { // Copy magic bytes at the beginning of the header SkASSERT(sizeof(kMagic) == 8); - memcpy(header, kMagic, sizeof(kMagic)); + SkASSERT(sizeof(kMagic) == sizeof(info->fMagic)); + memcpy(info->fMagic, kMagic, sizeof(kMagic)); // Set picture info after magic bytes in the header - SkPictInfo* info = (SkPictInfo*)(((char*)header) + sizeof(kMagic)); info->fVersion = CURRENT_PICTURE_VERSION; info->fWidth = fWidth; info->fHeight = fHeight; @@ -387,9 +380,9 @@ void SkPicture::serialize(SkWStream* stream, EncodeBitmap encoder) const { playback = SkNEW_ARGS(SkPicturePlayback, (*fRecord)); } - char header[kHeaderSize]; + SkPictInfo header; this->createHeader(&header); - stream->write(header, kHeaderSize); + stream->write(&header, sizeof(header)); if (playback) { stream->writeBool(true); playback->serialize(stream, encoder); @@ -409,9 +402,9 @@ void SkPicture::flatten(SkWriteBuffer& buffer) const { playback = SkNEW_ARGS(SkPicturePlayback, (*fRecord)); } - char header[kHeaderSize]; + SkPictInfo header; this->createHeader(&header); - buffer.writeByteArray(header, kHeaderSize); + buffer.writeByteArray(&header, sizeof(header)); if (playback) { buffer.writeBool(true); playback->flatten(buffer); diff --git a/src/core/SkPicturePlayback.cpp b/src/core/SkPicturePlayback.cpp index 08e53fab8c..05c2c4e56b 100644 --- a/src/core/SkPicturePlayback.cpp +++ b/src/core/SkPicturePlayback.cpp @@ -270,7 +270,7 @@ void SkPicturePlayback::init() { } SkPicturePlayback::~SkPicturePlayback() { - fOpData->unref(); + SkSafeUnref(fOpData); SkSafeUnref(fBitmaps); SkSafeUnref(fPaints); @@ -612,6 +612,41 @@ bool SkPicturePlayback::parseBufferTag(SkReadBuffer& buffer, fPathHeap.reset(SkNEW_ARGS(SkPathHeap, (buffer))); } break; + case SK_PICT_READER_TAG: { + SkAutoMalloc storage(size); + if (!buffer.readByteArray(storage.get(), size) || + !buffer.validate(NULL == fOpData)) { + return false; + } + SkASSERT(NULL == fOpData); + fOpData = SkData::NewFromMalloc(storage.detach(), size); + } break; + case SK_PICT_PICTURE_TAG: { + if (!buffer.validate((0 == fPictureCount) && (NULL == fPictureRefs))) { + return false; + } + fPictureCount = size; + fPictureRefs = SkNEW_ARRAY(SkPicture*, fPictureCount); + bool success = true; + int i = 0; + for ( ; i < fPictureCount; i++) { + fPictureRefs[i] = SkPicture::CreateFromBuffer(buffer); + if (NULL == fPictureRefs[i]) { + success = false; + break; + } + } + if (!success) { + // Delete all of the pictures that were already created (up to but excluding i): + for (int j = 0; j < i; j++) { + fPictureRefs[j]->unref(); + } + // Delete the array + SkDELETE_ARRAY(fPictureRefs); + fPictureCount = 0; + return false; + } + } break; default: // The tag was invalid. return false; diff --git a/src/core/SkPicturePlayback.h b/src/core/SkPicturePlayback.h index 178a408aee..512f24a77e 100644 --- a/src/core/SkPicturePlayback.h +++ b/src/core/SkPicturePlayback.h @@ -40,6 +40,7 @@ struct SkPictInfo { kPtrIs64Bit_Flag = 1 << 2, }; + char fMagic[8]; uint32_t fVersion; uint32_t fWidth; uint32_t fHeight; diff --git a/src/core/SkValidatingReadBuffer.cpp b/src/core/SkValidatingReadBuffer.cpp index ae7a83684b..8db2c68b9a 100644 --- a/src/core/SkValidatingReadBuffer.cpp +++ b/src/core/SkValidatingReadBuffer.cpp @@ -213,10 +213,10 @@ uint32_t SkValidatingReadBuffer::getArrayCount() { void SkValidatingReadBuffer::readBitmap(SkBitmap* bitmap) { const int width = this->readInt(); const int height = this->readInt(); + const bool useBitmapHeap = this->readBool(); const size_t length = this->readUInt(); // A size of zero means the SkBitmap was simply flattened. - this->validate(length == 0); - if (fError) { + if (!this->validate(!useBitmapHeap && (0 == length))) { return; } bitmap->unflatten(*this); diff --git a/src/core/SkValidatingReadBuffer.h b/src/core/SkValidatingReadBuffer.h index 59b5c1acc3..d5e192e1c7 100644 --- a/src/core/SkValidatingReadBuffer.h +++ b/src/core/SkValidatingReadBuffer.h @@ -75,7 +75,6 @@ private: return SkIsAlign4((uintptr_t)ptr); } - SkReader32 fReader; bool fError; typedef SkReadBuffer INHERITED; diff --git a/src/ports/SkGlobalInitialization_chromium.cpp b/src/ports/SkGlobalInitialization_chromium.cpp index 79ed46c70e..dd24eaf5e5 100644 --- a/src/ports/SkGlobalInitialization_chromium.cpp +++ b/src/ports/SkGlobalInitialization_chromium.cpp @@ -50,6 +50,7 @@ #include "SkOffsetImageFilter.h" #include "SkOnce.h" #include "SkPerlinNoiseShader.h" +#include "SkPictureImageFilter.h" #include "SkPixelXorXfermode.h" #include "SkRectShaderImageFilter.h" #include "SkResizeImageFilter.h" @@ -88,6 +89,7 @@ static void InitializeFlattenables(int*) { SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkLine2DPathEffect) SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPath2DPathEffect) SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPerlinNoiseShader) + SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPictureImageFilter) SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPixelXorXfermode) SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkRectShaderImageFilter) SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkResizeImageFilter) diff --git a/src/ports/SkGlobalInitialization_default.cpp b/src/ports/SkGlobalInitialization_default.cpp index 79ed46c70e..dd24eaf5e5 100644 --- a/src/ports/SkGlobalInitialization_default.cpp +++ b/src/ports/SkGlobalInitialization_default.cpp @@ -50,6 +50,7 @@ #include "SkOffsetImageFilter.h" #include "SkOnce.h" #include "SkPerlinNoiseShader.h" +#include "SkPictureImageFilter.h" #include "SkPixelXorXfermode.h" #include "SkRectShaderImageFilter.h" #include "SkResizeImageFilter.h" @@ -88,6 +89,7 @@ static void InitializeFlattenables(int*) { SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkLine2DPathEffect) SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPath2DPathEffect) SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPerlinNoiseShader) + SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPictureImageFilter) SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPixelXorXfermode) SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkRectShaderImageFilter) SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkResizeImageFilter) -- cgit v1.2.3