aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar Mike Klein <mtklein@chromium.org>2017-11-27 12:39:30 -0500
committerGravatar Skia Commit-Bot <skia-commit-bot@chromium.org>2017-11-27 18:00:56 +0000
commitc9bc81434adc0c0ea6012167fbda0e131548e683 (patch)
treeffc99097e19d810a5d15ca97747c291cd607ff12
parent57899c7e9f8d703dc0c39b1fc693b1938c487ece (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.gni1
-rw-r--r--src/effects/SkToSRGBColorFilter.cpp6
-rw-r--r--tests/ToSRGBColorFilter.cpp29
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));
+
+}