Expose SkPath validation as boolean

As a part of serializing SkPaths, I want to be able to know (without
asserting) whether or not a path is valid so that I can discard
potentially malicious deserialized paths.

Currently, SkPath(Ref) both just have asserting validation functions
which can't be used externally.  This patch adds accessors that don't
assert.

Bug: chromium:752755 skia:6955
Change-Id: I4d0ceb31ec660b87e3fda438392ad2b60a27a0da
Reviewed-on: https://skia-review.googlesource.com/31720
Commit-Queue: Mike Klein <mtklein@chromium.org>
Reviewed-by: Mike Reed <reed@google.com>
This commit is contained in:
Adrienne Walker 2017-08-10 12:16:37 -07:00 committed by Skia Commit-Bot
parent 6a653a5f7d
commit ad8da8ea99
4 changed files with 69 additions and 33 deletions

View File

@ -1119,7 +1119,9 @@ public:
static const int kPathRefGenIDBitCnt = 32;
#endif
SkDEBUGCODE(void validate() const;)
bool isValid() const;
bool pathRefIsValid() const { return fPathRef->isValid(); }
SkDEBUGCODE(void validate() const { SkASSERT(this->isValid()); } )
SkDEBUGCODE(void experimentalValidateRef() const { fPathRef->validate(); } )
private:

View File

@ -311,7 +311,8 @@ public:
void addGenIDChangeListener(GenIDChangeListener* listener);
SkDEBUGCODE(void validate() const;)
bool isValid() const;
SkDEBUGCODE(void validate() const { SkASSERT(this->isValid()); } )
private:
enum SerializationOffsets {

View File

@ -2219,37 +2219,46 @@ void SkPath::dumpHex() const {
this->dump(nullptr, false, true);
}
#ifdef SK_DEBUG
void SkPath::validate() const {
SkASSERT((fFillType & ~3) == 0);
bool SkPath::isValid() const {
if ((fFillType & ~3) != 0) {
return false;
}
#ifdef SK_DEBUG_PATH
if (!fBoundsIsDirty) {
SkRect bounds;
bool isFinite = compute_pt_bounds(&bounds, *fPathRef.get());
SkASSERT(SkToBool(fIsFinite) == isFinite);
if (SkToBool(fIsFinite) != isFinite) {
return false;
}
if (fPathRef->countPoints() <= 1) {
// if we're empty, fBounds may be empty but translated, so we can't
// necessarily compare to bounds directly
// try path.addOval(2, 2, 2, 2) which is empty, but the bounds will
// be [2, 2, 2, 2]
SkASSERT(bounds.isEmpty());
SkASSERT(fBounds.isEmpty());
if (!bounds.isEmpty() || !fBounds.isEmpty()) {
return false;
}
} else {
if (bounds.isEmpty()) {
SkASSERT(fBounds.isEmpty());
if (!fBounds.isEmpty()) {
return false;
}
} else {
if (!fBounds.isEmpty()) {
SkASSERT(fBounds.contains(bounds));
if (!fBounds.contains(bounds)) {
return false;
}
}
}
}
}
#endif // SK_DEBUG_PATH
return true;
}
#endif // SK_DEBUG
///////////////////////////////////////////////////////////////////////////////

View File

@ -6,6 +6,7 @@
*/
#include "SkBuffer.h"
#include "SkNx.h"
#include "SkOnce.h"
#include "SkPath.h"
#include "SkPathRef.h"
@ -742,28 +743,47 @@ uint8_t SkPathRef::Iter::peek() const {
return next <= fVerbStop ? (uint8_t) SkPath::kDone_Verb : *next;
}
#ifdef SK_DEBUG
#include "SkNx.h"
void SkPathRef::validate() const {
SkASSERT(static_cast<ptrdiff_t>(fFreeSpace) >= 0);
SkASSERT(reinterpret_cast<intptr_t>(fVerbs) - reinterpret_cast<intptr_t>(fPoints) >= 0);
SkASSERT((nullptr == fPoints) == (nullptr == fVerbs));
SkASSERT(!(nullptr == fPoints && 0 != fFreeSpace));
SkASSERT(!(nullptr == fPoints && 0 != fFreeSpace));
SkASSERT(!(nullptr == fPoints && fPointCnt));
SkASSERT(!(nullptr == fVerbs && fVerbCnt));
SkASSERT(this->currSize() ==
fFreeSpace + sizeof(SkPoint) * fPointCnt + sizeof(uint8_t) * fVerbCnt);
bool SkPathRef::isValid() const {
if (static_cast<ptrdiff_t>(fFreeSpace) < 0) {
return false;
}
if (reinterpret_cast<intptr_t>(fVerbs) - reinterpret_cast<intptr_t>(fPoints) < 0) {
return false;
}
if ((nullptr == fPoints) != (nullptr == fVerbs)) {
return false;
}
if (nullptr == fPoints && 0 != fFreeSpace) {
return false;
}
if (nullptr == fPoints && 0 != fFreeSpace) {
return false;
}
if (nullptr == fPoints && fPointCnt) {
return false;
}
if (nullptr == fVerbs && fVerbCnt) {
return false;
}
if (this->currSize() !=
fFreeSpace + sizeof(SkPoint) * fPointCnt + sizeof(uint8_t) * fVerbCnt) {
return false;
}
if (fIsOval || fIsRRect) {
// Currently we don't allow both of these to be set, even though ovals are round rects.
SkASSERT(fIsOval != fIsRRect);
// Currently we don't allow both of these to be set, even though ovals are ro
if (fIsOval == fIsRRect) {
return false;
}
if (fIsOval) {
SkASSERT(fRRectOrOvalStartIdx < 4);
if (fRRectOrOvalStartIdx >= 4) {
return false;
}
} else {
SkASSERT(fRRectOrOvalStartIdx < 8);
if (fRRectOrOvalStartIdx >= 8) {
return false;
}
}
}
@ -787,13 +807,15 @@ void SkPathRef::validate() const {
}
#endif
SkASSERT(!fPoints[i].isFinite() ||
(!(point < leftTop).anyTrue() && !(point > rightBot).anyTrue()));
if (fPoints[i].isFinite() && (point < leftTop).anyTrue() && !(point > rightBot).anyTrue())
return false;
if (!fPoints[i].isFinite()) {
isFinite = false;
}
}
SkASSERT(SkToBool(fIsFinite) == isFinite);
if (SkToBool(fIsFinite) != isFinite) {
return false;
}
}
#ifdef SK_DEBUG_PATH
@ -824,7 +846,9 @@ void SkPathRef::validate() const {
break;
}
}
SkASSERT(mask == fSegmentMask);
if (mask != fSegmentMask) {
return false;
}
#endif // SK_DEBUG_PATH
return true;
}
#endif