diff options
| -rw-r--r-- | include/core/SkRRect.h | 7 | ||||
| -rw-r--r-- | src/core/SkRRect.cpp | 24 | ||||
| -rw-r--r-- | tests/RoundRectTest.cpp | 65 |
3 files changed, 88 insertions, 8 deletions
diff --git a/include/core/SkRRect.h b/include/core/SkRRect.h index f359c9c167..eb983b6c0e 100644 --- a/include/core/SkRRect.h +++ b/include/core/SkRRect.h @@ -46,7 +46,7 @@ class SkMatrix; */ class SK_API SkRRect { public: - SkRRect() { /* unititialized */ } + SkRRect() { this->setEmpty(); } SkRRect(const SkRRect&) = default; SkRRect& operator=(const SkRRect&) = default; @@ -135,7 +135,7 @@ public: fRect = rect; fRect.sort(); - if (fRect.isEmpty()) { + if (fRect.isEmpty() || !fRect.isFinite()) { this->setEmpty(); return; } @@ -178,7 +178,7 @@ public: fRect = oval; fRect.sort(); - if (fRect.isEmpty()) { + if (fRect.isEmpty() || !fRect.isFinite()) { this->setEmpty(); return; } @@ -290,6 +290,7 @@ public: bool contains(const SkRect& rect) const; bool isValid() const; + static bool AreRectAndRadiiValid(const SkRect&, const SkVector[4]); enum { kSizeInMemory = 12 * sizeof(SkScalar) diff --git a/src/core/SkRRect.cpp b/src/core/SkRRect.cpp index 730868a440..f1b85fbc28 100644 --- a/src/core/SkRRect.cpp +++ b/src/core/SkRRect.cpp @@ -465,7 +465,12 @@ size_t SkRRect::readFromMemory(const void* buffer, size_t length) { if (length < kSizeInMemory) { return 0; } - + // Note that the buffer may be smaller than SkRRect. It is important not to access + // bufferAsRRect->fType. + const SkRRect* bufferAsRRect = reinterpret_cast<const SkRRect*>(buffer); + if (!AreRectAndRadiiValid(bufferAsRRect->fRect, bufferAsRRect->fRadii)) { + return 0; + } // Deserialize rect and corners, then rederive the type tag. memcpy(this, buffer, kSizeInMemory); this->computeType(); @@ -505,6 +510,10 @@ static bool are_radius_check_predicates_valid(SkScalar rad, SkScalar min, SkScal } bool SkRRect::isValid() const { + if (!AreRectAndRadiiValid(fRect, fRadii)) { + return false; + } + bool allRadiiZero = (0 == fRadii[0].fX && 0 == fRadii[0].fY); bool allCornersSquare = (0 == fRadii[0].fX || 0 == fRadii[0].fY); bool allRadiiSame = true; @@ -570,14 +579,19 @@ bool SkRRect::isValid() const { break; } + return true; +} + +bool SkRRect::AreRectAndRadiiValid(const SkRect& rect, const SkVector radii[4]) { + if (!rect.isFinite()) { + return false; + } for (int i = 0; i < 4; ++i) { - if (!are_radius_check_predicates_valid(fRadii[i].fX, fRect.fLeft, fRect.fRight) || - !are_radius_check_predicates_valid(fRadii[i].fY, fRect.fTop, fRect.fBottom)) { + if (!are_radius_check_predicates_valid(radii[i].fX, rect.fLeft, rect.fRight) || + !are_radius_check_predicates_valid(radii[i].fY, rect.fTop, rect.fBottom)) { return false; } } - return true; } - /////////////////////////////////////////////////////////////////////////////// diff --git a/tests/RoundRectTest.cpp b/tests/RoundRectTest.cpp index ce9744571b..d9b7a9aa08 100644 --- a/tests/RoundRectTest.cpp +++ b/tests/RoundRectTest.cpp @@ -722,6 +722,70 @@ static void test_issue_2696(skiatest::Reporter* reporter) { } } +void test_read_rrect(skiatest::Reporter* reporter, const SkRRect& rrect, bool shouldSucceed) { + // It would be cleaner to call rrect.writeToMemory into a buffer. However, writeToMemory asserts + // that the rrect is valid and our caller may have fiddled with the internals of rrect to make + // it invalid. + const void* buffer = reinterpret_cast<const void*>(&rrect); + SkRRect deserialized; + size_t size = deserialized.readFromMemory(buffer, sizeof(SkRRect)); + if (shouldSucceed) { + REPORTER_ASSERT(reporter, size == SkRRect::kSizeInMemory); + if (size) { + REPORTER_ASSERT(reporter, rrect == deserialized); + REPORTER_ASSERT(reporter, rrect.getType() == deserialized.getType()); + } + } else { + REPORTER_ASSERT(reporter, !size); + } +} + +static void test_read(skiatest::Reporter* reporter) { + static const SkRect kRect = {10.f, 10.f, 20.f, 20.f}; + static const SkRect kNaNRect = {10.f, 10.f, 20.f, SK_ScalarNaN}; + static const SkRect kInfRect = {10.f, 10.f, SK_ScalarInfinity, 20.f}; + SkRRect rrect; + + test_read_rrect(reporter, SkRRect::MakeEmpty(), true); + test_read_rrect(reporter, SkRRect::MakeRect(kRect), true); + // These get coerced to empty. + test_read_rrect(reporter, SkRRect::MakeRect(kInfRect), true); + test_read_rrect(reporter, SkRRect::MakeRect(kNaNRect), true); + + rrect.setRect(kRect); + SkRect* innerRect = reinterpret_cast<SkRect*>(&rrect); + SkASSERT(*innerRect == kRect); + *innerRect = kInfRect; + test_read_rrect(reporter, rrect, false); + *innerRect = kNaNRect; + test_read_rrect(reporter, rrect, false); + + test_read_rrect(reporter, SkRRect::MakeOval(kRect), true); + test_read_rrect(reporter, SkRRect::MakeOval(kInfRect), true); + test_read_rrect(reporter, SkRRect::MakeOval(kNaNRect), true); + rrect.setOval(kRect); + *innerRect = kInfRect; + test_read_rrect(reporter, rrect, false); + *innerRect = kNaNRect; + test_read_rrect(reporter, rrect, false); + + test_read_rrect(reporter, SkRRect::MakeRectXY(kRect, 5.f, 5.f), true); + // rrect should scale down the radii to make this legal + test_read_rrect(reporter, SkRRect::MakeRectXY(kRect, 5.f, 400.f), true); + + static const SkVector kRadii[4] = {{0.5f, 1.f}, {1.5f, 2.f}, {2.5f, 3.f}, {3.5f, 4.f}}; + rrect.setRectRadii(kRect, kRadii); + test_read_rrect(reporter, rrect, true); + SkScalar* innerRadius = reinterpret_cast<SkScalar*>(&rrect) + 6; + SkASSERT(*innerRadius == 1.5f); + *innerRadius = 400.f; + test_read_rrect(reporter, rrect, false); + *innerRadius = SK_ScalarInfinity; + test_read_rrect(reporter, rrect, false); + *innerRadius = SK_ScalarNaN; + test_read_rrect(reporter, rrect, false); +} + DEF_TEST(RoundRect, reporter) { test_round_rect_basic(reporter); test_round_rect_rects(reporter); @@ -735,4 +799,5 @@ DEF_TEST(RoundRect, reporter) { test_tricky_radii(reporter); test_empty_crbug_458524(reporter); test_empty(reporter); + test_read(reporter); } |