diff options
author | Mike Klein <mtklein@chromium.org> | 2017-11-27 12:39:30 -0500 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2017-11-27 18:00:56 +0000 |
commit | c9bc81434adc0c0ea6012167fbda0e131548e683 (patch) | |
tree | ffc99097e19d810a5d15ca97747c291cd607ff12 | |
parent | 57899c7e9f8d703dc0c39b1fc693b1938c487ece (diff) |
Handle null colorspace in SkToSRGBColorFilter.
This was uncovered by the linked fuzzer issue.
I haven't looked hard at it, but I'd guess it's fuzzed an ICC profile
into one that can't be deserialized, and we get a null in CreateProc().
We could probably restrict the null check to just CreateProc(), but
putting it in Make() and asserting in the constructor feels cozy.
BUG=chromium:787718
Change-Id: Ic4b1dad28c00ee5870f22093eedbf34686c32120
Reviewed-on: https://skia-review.googlesource.com/76080
Commit-Queue: Mike Klein <mtklein@chromium.org>
Reviewed-by: Brian Osman <brianosman@google.com>
-rw-r--r-- | gn/tests.gni | 1 | ||||
-rw-r--r-- | src/effects/SkToSRGBColorFilter.cpp | 6 | ||||
-rw-r--r-- | tests/ToSRGBColorFilter.cpp | 29 |
3 files changed, 34 insertions, 2 deletions
diff --git a/gn/tests.gni b/gn/tests.gni index 6579820877..418ce01d95 100644 --- a/gn/tests.gni +++ b/gn/tests.gni @@ -258,6 +258,7 @@ tests_sources = [ "$_tests/Time.cpp", "$_tests/TLSTest.cpp", "$_tests/TopoSortTest.cpp", + "$_tests/ToSRGBColorFilter.cpp", "$_tests/TraceMemoryDumpTest.cpp", "$_tests/TracingTest.cpp", "$_tests/TransferPixelsTest.cpp", diff --git a/src/effects/SkToSRGBColorFilter.cpp b/src/effects/SkToSRGBColorFilter.cpp index fa26350097..948c4d908b 100644 --- a/src/effects/SkToSRGBColorFilter.cpp +++ b/src/effects/SkToSRGBColorFilter.cpp @@ -56,7 +56,7 @@ void SkToSRGBColorFilter::onAppendStages(SkRasterPipeline* p, } sk_sp<SkColorFilter> SkToSRGBColorFilter::Make(sk_sp<SkColorSpace> srcColorSpace) { - if (srcColorSpace->isSRGB()) { + if (!srcColorSpace || srcColorSpace->isSRGB()) { return nullptr; } else { return sk_sp<SkColorFilter>(new SkToSRGBColorFilter(std::move(srcColorSpace))); @@ -64,7 +64,9 @@ sk_sp<SkColorFilter> SkToSRGBColorFilter::Make(sk_sp<SkColorSpace> srcColorSpace } SkToSRGBColorFilter::SkToSRGBColorFilter(sk_sp<SkColorSpace> srcColorSpace) - : fSrcColorSpace(std::move(srcColorSpace)) {} + : fSrcColorSpace(std::move(srcColorSpace)) { + SkASSERT(fSrcColorSpace); +} sk_sp<SkFlattenable> SkToSRGBColorFilter::CreateProc(SkReadBuffer& buffer) { auto data = buffer.readByteArrayAsData(); diff --git a/tests/ToSRGBColorFilter.cpp b/tests/ToSRGBColorFilter.cpp new file mode 100644 index 0000000000..01c5673d4d --- /dev/null +++ b/tests/ToSRGBColorFilter.cpp @@ -0,0 +1,29 @@ +/* + * Copyright 2017 Google Inc. + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "SkColorSpace.h" +#include "SkToSRGBColorFilter.h" +#include "Test.h" + + +DEF_TEST(SkToSRGBColorFilter, r) { + + // sRGB -> sRGB is a no-op. + REPORTER_ASSERT(r, nullptr == SkToSRGBColorFilter::Make(SkColorSpace::MakeSRGB())); + + // The transfer function matters just as much as the gamut. + REPORTER_ASSERT(r, nullptr != SkToSRGBColorFilter::Make(SkColorSpace::MakeSRGBLinear())); + + // We generally interpret nullptr source spaces as sRGB. See also chromium:787718. + REPORTER_ASSERT(r, nullptr == SkToSRGBColorFilter::Make(nullptr)); + + // Here's a realistic conversion. + auto dci_p3 = SkColorSpace::MakeRGB(SkColorSpace::kLinear_RenderTargetGamma, + SkColorSpace::kDCIP3_D65_Gamut); + REPORTER_ASSERT(r, nullptr != SkToSRGBColorFilter::Make(dci_p3)); + +} |