Add unit test for CDFP_CalRGB::TranslateImageLine()
Then fix write beyond end-of-buffer for the bTransMask case, where
the incremented pointers are then passed to ReverseRGB(), The code
in that block is already using the correct order, and looks to be
merely missing a return or an else clause.
-- add new method to get classes for test.
-- re-order if to avoid some indentation.
-- add initialization to avoid uninit memory during test.
Change-Id: I3f5fd3149e16d12e5e22e1efc3f2b2e601456187
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/84634
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/page/BUILD.gn b/core/fpdfapi/page/BUILD.gn
index ca21c78..9f39824 100644
--- a/core/fpdfapi/page/BUILD.gn
+++ b/core/fpdfapi/page/BUILD.gn
@@ -117,6 +117,7 @@
pdfium_unittest_source_set("unittests") {
sources = [
+ "cpdf_colorspace_unittest.cpp",
"cpdf_devicecs_unittest.cpp",
"cpdf_function_unittest.cpp",
"cpdf_pageobjectholder_unittest.cpp",
diff --git a/core/fpdfapi/page/cpdf_colorspace.cpp b/core/fpdfapi/page/cpdf_colorspace.cpp
index 170d61e..fd2a80d 100644
--- a/core/fpdfapi/page/cpdf_colorspace.cpp
+++ b/core/fpdfapi/page/cpdf_colorspace.cpp
@@ -139,8 +139,8 @@
explicit CPDF_CalGray(CPDF_Document* pDoc);
float m_Gamma = kDefaultGamma;
- float m_WhitePoint[kBlackWhitePointCount];
- float m_BlackPoint[kBlackWhitePointCount];
+ float m_WhitePoint[kBlackWhitePointCount] = {1.0f, 1.0f, 1.0f};
+ float m_BlackPoint[kBlackWhitePointCount] = {0.0f, 0.0f, 0.0f};
};
class CPDF_CalRGB final : public CPDF_ColorSpace {
@@ -169,8 +169,8 @@
explicit CPDF_CalRGB(CPDF_Document* pDoc);
- float m_WhitePoint[kBlackWhitePointCount];
- float m_BlackPoint[kBlackWhitePointCount];
+ float m_WhitePoint[kBlackWhitePointCount] = {1.0f, 1.0f, 1.0f};
+ float m_BlackPoint[kBlackWhitePointCount] = {0.0f, 0.0f, 0.0f};
float m_Gamma[kGammaCount];
float m_Matrix[kMatrixCount];
bool m_bHasGamma = false;
@@ -206,8 +206,8 @@
explicit CPDF_LabCS(CPDF_Document* pDoc);
- float m_WhitePoint[kBlackWhitePointCount];
- float m_BlackPoint[kBlackWhitePointCount];
+ float m_WhitePoint[kBlackWhitePointCount] = {1.0f, 1.0f, 1.0f};
+ float m_BlackPoint[kBlackWhitePointCount] = {0.0f, 0.0f, 0.0f};
float m_Ranges[kRangesCount];
};
@@ -539,36 +539,11 @@
if (pArray->size() == 1)
return ColorspaceFromName(familyname);
- RetainPtr<CPDF_ColorSpace> pCS;
- switch (familyname.GetID()) {
- case FXBSTR_ID('C', 'a', 'l', 'G'):
- pCS = pdfium::MakeRetain<CPDF_CalGray>(pDoc);
- break;
- case FXBSTR_ID('C', 'a', 'l', 'R'):
- pCS = pdfium::MakeRetain<CPDF_CalRGB>(pDoc);
- break;
- case FXBSTR_ID('L', 'a', 'b', 0):
- pCS = pdfium::MakeRetain<CPDF_LabCS>(pDoc);
- break;
- case FXBSTR_ID('I', 'C', 'C', 'B'):
- pCS = pdfium::MakeRetain<CPDF_ICCBasedCS>(pDoc);
- break;
- case FXBSTR_ID('I', 'n', 'd', 'e'):
- case FXBSTR_ID('I', 0, 0, 0):
- pCS = pdfium::MakeRetain<CPDF_IndexedCS>(pDoc);
- break;
- case FXBSTR_ID('S', 'e', 'p', 'a'):
- pCS = pdfium::MakeRetain<CPDF_SeparationCS>(pDoc);
- break;
- case FXBSTR_ID('D', 'e', 'v', 'i'):
- pCS = pdfium::MakeRetain<CPDF_DeviceNCS>(pDoc);
- break;
- case FXBSTR_ID('P', 'a', 't', 't'):
- pCS = pdfium::MakeRetain<CPDF_PatternCS>(pDoc);
- break;
- default:
- return nullptr;
- }
+ RetainPtr<CPDF_ColorSpace> pCS =
+ CPDF_ColorSpace::AllocateColorSpaceForID(pDoc, familyname.GetID());
+ if (!pCS)
+ return nullptr;
+
pCS->m_pArray.Reset(pArray);
pCS->m_nComponents = pCS->v_Load(pDoc, pArray, pVisited);
if (pCS->m_nComponents == 0)
@@ -578,6 +553,33 @@
}
// static
+RetainPtr<CPDF_ColorSpace> CPDF_ColorSpace::AllocateColorSpaceForID(
+ CPDF_Document* pDocument,
+ uint32_t family_id) {
+ switch (family_id) {
+ case FXBSTR_ID('C', 'a', 'l', 'G'):
+ return pdfium::MakeRetain<CPDF_CalGray>(pDocument);
+ case FXBSTR_ID('C', 'a', 'l', 'R'):
+ return pdfium::MakeRetain<CPDF_CalRGB>(pDocument);
+ case FXBSTR_ID('L', 'a', 'b', 0):
+ return pdfium::MakeRetain<CPDF_LabCS>(pDocument);
+ case FXBSTR_ID('I', 'C', 'C', 'B'):
+ return pdfium::MakeRetain<CPDF_ICCBasedCS>(pDocument);
+ case FXBSTR_ID('I', 'n', 'd', 'e'):
+ case FXBSTR_ID('I', 0, 0, 0):
+ return pdfium::MakeRetain<CPDF_IndexedCS>(pDocument);
+ case FXBSTR_ID('S', 'e', 'p', 'a'):
+ return pdfium::MakeRetain<CPDF_SeparationCS>(pDocument);
+ case FXBSTR_ID('D', 'e', 'v', 'i'):
+ return pdfium::MakeRetain<CPDF_DeviceNCS>(pDocument);
+ case FXBSTR_ID('P', 'a', 't', 't'):
+ return pdfium::MakeRetain<CPDF_PatternCS>(pDocument);
+ default:
+ return nullptr;
+ }
+}
+
+// static
uint32_t CPDF_ColorSpace::ComponentsForFamily(Family family) {
switch (family) {
case Family::kDeviceGray:
@@ -800,24 +802,26 @@
int image_width,
int image_height,
bool bTransMask) const {
- if (bTransMask) {
- float Cal[3];
- float R;
- float G;
- float B;
- for (int i = 0; i < pixels; i++) {
- Cal[0] = static_cast<float>(pSrcBuf[2]) / 255;
- Cal[1] = static_cast<float>(pSrcBuf[1]) / 255;
- Cal[2] = static_cast<float>(pSrcBuf[0]) / 255;
- GetRGB(Cal, &R, &G, &B);
- pDestBuf[0] = FXSYS_roundf(B * 255);
- pDestBuf[1] = FXSYS_roundf(G * 255);
- pDestBuf[2] = FXSYS_roundf(R * 255);
- pSrcBuf += 3;
- pDestBuf += 3;
- }
+ if (!bTransMask) {
+ fxcodec::ReverseRGB(pDestBuf, pSrcBuf, pixels);
+ return;
}
- fxcodec::ReverseRGB(pDestBuf, pSrcBuf, pixels);
+
+ float Cal[3];
+ float R;
+ float G;
+ float B;
+ for (int i = 0; i < pixels; i++) {
+ Cal[0] = static_cast<float>(pSrcBuf[2]) / 255;
+ Cal[1] = static_cast<float>(pSrcBuf[1]) / 255;
+ Cal[2] = static_cast<float>(pSrcBuf[0]) / 255;
+ GetRGB(Cal, &R, &G, &B);
+ pDestBuf[0] = FXSYS_roundf(B * 255);
+ pDestBuf[1] = FXSYS_roundf(G * 255);
+ pDestBuf[2] = FXSYS_roundf(R * 255);
+ pSrcBuf += 3;
+ pDestBuf += 3;
+ }
}
CPDF_LabCS::CPDF_LabCS(CPDF_Document* pDoc)
diff --git a/core/fpdfapi/page/cpdf_colorspace.h b/core/fpdfapi/page/cpdf_colorspace.h
index 3e9bd9c..c4bf8d4 100644
--- a/core/fpdfapi/page/cpdf_colorspace.h
+++ b/core/fpdfapi/page/cpdf_colorspace.h
@@ -77,6 +77,10 @@
const CPDF_Object* pObj,
std::set<const CPDF_Object*>* pVisited);
+ static RetainPtr<CPDF_ColorSpace> AllocateColorSpaceForID(
+ CPDF_Document* pDocument,
+ uint32_t family_id);
+
static uint32_t ComponentsForFamily(Family family);
static bool IsValidIccComponents(int components);
diff --git a/core/fpdfapi/page/cpdf_colorspace_unittest.cpp b/core/fpdfapi/page/cpdf_colorspace_unittest.cpp
new file mode 100644
index 0000000..8ff25ff
--- /dev/null
+++ b/core/fpdfapi/page/cpdf_colorspace_unittest.cpp
@@ -0,0 +1,34 @@
+// Copyright 2021 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/fpdfapi/page/cpdf_colorspace.h"
+
+#include <stdint.h>
+#include <string.h>
+
+#include "core/fxcrt/retain_ptr.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(CPDF_CalRGB, TranslateImageLine) {
+ const uint8_t kSrc[12] = {255, 0, 0, 0, 255, 0, 0, 0, 255, 128, 128, 128};
+ const uint8_t kExpectMask[12] = {255, 58, 0, 0, 255, 0,
+ 70, 0, 255, 188, 188, 188};
+ const uint8_t kExpectNomask[12] = {0, 0, 255, 0, 255, 0,
+ 255, 0, 0, 128, 128, 128};
+
+ RetainPtr<CPDF_ColorSpace> pCal = CPDF_ColorSpace::AllocateColorSpaceForID(
+ nullptr, FXBSTR_ID('C', 'a', 'l', 'R'));
+ ASSERT_TRUE(pCal);
+
+ uint8_t dst[12];
+ memset(dst, 0xbd, sizeof(dst));
+ pCal->TranslateImageLine(dst, kSrc, 4, 4, 1, true);
+ for (size_t i = 0; i < 12; ++i)
+ EXPECT_EQ(dst[i], kExpectMask[i]) << " at " << i;
+
+ memset(dst, 0xbd, sizeof(dst));
+ pCal->TranslateImageLine(dst, kSrc, 4, 4, 1, false);
+ for (size_t i = 0; i < 12; ++i)
+ EXPECT_EQ(dst[i], kExpectNomask[i]) << " at " << i;
+}
