commit 2a1cbc0417985485ab660ee6fd98c3bbedf716cd
author Lei Zhang <thestig@chromium.org> Tue Sep 25 20:07:29 2018 +0000
committer Chromium commit bot <commit-bot@chromium.org> Tue Sep 25 20:07:29 2018 +0000
tree 280fc0b1860a6f2757f70c0382b0ae0464c0c9d2
parent 30dc6aaf878b2c55efcf7598fdb8886e06d14e01

Revert "Remove unreachable code in CPDF_DIBBase."

This reverts commit 214982c4d02a720c3b1b3de121cddc62189b8848.

Reason for revert: Turns out it is reachable.

Original change's description:
> Remove unreachable code in CPDF_DIBBase.
> 
> The colorspace is always available when creating image decoders that use
> colorspaces.
> 
> Change-Id: I20ac75edcd614ccc1e83de262c128776e9d03eed
> Reviewed-on: https://pdfium-review.googlesource.com/42872
> Commit-Queue: Ryan Harrison <rharrison@chromium.org>
> Reviewed-by: Ryan Harrison <rharrison@chromium.org>

TBR=thestig@chromium.org,rharrison@chromium.org
BUG=chromium:888743

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I45903543f0e4bf2785660fb8c1e45c34febb6ecf
Reviewed-on: https://pdfium-review.googlesource.com/43050
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

core/fpdfapi/render/cpdf_dibbase.cpp

diff --git a/core/fpdfapi/render/cpdf_dibbase.cpp b/core/fpdfapi/render/cpdf_dibbase.cpp
index 7391296..3557672 100644
--- a/core/fpdfapi/render/cpdf_dibbase.cpp
+++ b/core/fpdfapi/render/cpdf_dibbase.cpp
@@ -511,8 +511,6 @@
 
 bool CPDF_DIBBase::CreateDCTDecoder(pdfium::span<const uint8_t> src_span,
                                     const CPDF_Dictionary* pParams) {
-  ASSERT(m_pColorSpace);  // Assigned in LoadColorInfo().
-
   CCodec_JpegModule* pJpegModule = CPDF_ModuleMgr::Get()->GetJpegModule();
   m_pDecoder = pJpegModule->CreateDecoder(
       src_span, m_Width, m_Height, m_nComponents,
@@ -541,35 +539,40 @@
 
   m_nComponents = static_cast<uint32_t>(comps);
   m_CompData.clear();
-  switch (m_Family) {
-    case PDFCS_DEVICEGRAY:
-    case PDFCS_DEVICERGB:
-    case PDFCS_DEVICECMYK: {
-      uint32_t dwMinComps = ComponentsForFamily(m_Family);
-      if (m_pColorSpace->CountComponents() < dwMinComps ||
-          m_nComponents < dwMinComps) {
-        return false;
+  if (m_pColorSpace) {
+    switch (m_Family) {
+      case PDFCS_DEVICEGRAY:
+      case PDFCS_DEVICERGB:
+      case PDFCS_DEVICECMYK: {
+        uint32_t dwMinComps = ComponentsForFamily(m_Family);
+        if (m_pColorSpace->CountComponents() < dwMinComps ||
+            m_nComponents < dwMinComps) {
+          return false;
+        }
+        break;
       }
-      break;
-    }
-    case PDFCS_LAB: {
-      if (m_nComponents != 3 || m_pColorSpace->CountComponents() < 3)
-        return false;
-      break;
-    }
-    case PDFCS_ICCBASED: {
-      if (!IsAllowedICCComponents(m_nComponents) ||
-          !IsAllowedICCComponents(m_pColorSpace->CountComponents()) ||
-          m_pColorSpace->CountComponents() < m_nComponents) {
-        return false;
+      case PDFCS_LAB: {
+        if (m_nComponents != 3 || m_pColorSpace->CountComponents() < 3)
+          return false;
+        break;
       }
-      break;
+      case PDFCS_ICCBASED: {
+        if (!IsAllowedICCComponents(m_nComponents) ||
+            !IsAllowedICCComponents(m_pColorSpace->CountComponents()) ||
+            m_pColorSpace->CountComponents() < m_nComponents) {
+          return false;
+        }
+        break;
+      }
+      default: {
+        if (m_pColorSpace->CountComponents() != m_nComponents)
+          return false;
+        break;
+      }
     }
-    default: {
-      if (m_pColorSpace->CountComponents() != m_nComponents)
-        return false;
-      break;
-    }
+  } else {
+    if (m_Family == PDFCS_LAB && m_nComponents != 3)
+      return false;
   }
   if (!GetDecodeAndMaskArray(&m_bDefaultDecode, &m_bColorKey))
     return false;
@@ -581,8 +584,6 @@
 }
 
 RetainPtr<CFX_DIBitmap> CPDF_DIBBase::LoadJpxBitmap() {
-  ASSERT(m_pColorSpace);  // Assigned in LoadColorInfo().
-
   CCodec_JpxModule* pJpxModule = CPDF_ModuleMgr::Get()->GetJpxModule();
   auto context = pdfium::MakeUnique<JpxBitMapContext>(pJpxModule);
   context->set_decoder(
@@ -597,13 +598,22 @@
   if (static_cast<int>(width) < m_Width || static_cast<int>(height) < m_Height)
     return nullptr;
 
-  if (components != m_pColorSpace->CountComponents())
-    return nullptr;
-
   bool bSwapRGB = false;
-  if (m_pColorSpace == CPDF_ColorSpace::GetStockCS(PDFCS_DEVICERGB)) {
-    bSwapRGB = true;
-    m_pColorSpace = nullptr;
+  if (m_pColorSpace) {
+    if (components != m_pColorSpace->CountComponents())
+      return nullptr;
+
+    if (m_pColorSpace == CPDF_ColorSpace::GetStockCS(PDFCS_DEVICERGB)) {
+      bSwapRGB = true;
+      m_pColorSpace = nullptr;
+    }
+  } else {
+    if (components == 3) {
+      bSwapRGB = true;
+    } else if (components == 4) {
+      m_pColorSpace = CPDF_ColorSpace::GetStockCS(PDFCS_DEVICECMYK);
+    }
+    m_nComponents = components;
   }
 
   FXDIB_Format format;
@@ -614,7 +624,6 @@
   } else if (components == 4) {
     format = FXDIB_Rgb32;
   } else {
-    // TODO(thestig): Is this reachable? Probably need to validate |components|.
     width = (width * components + 2) / 3;
     format = FXDIB_Rgb;
   }

core/fxcodec/codec/ccodec_jpxmodule.cpp

diff --git a/core/fxcodec/codec/ccodec_jpxmodule.cpp b/core/fxcodec/codec/ccodec_jpxmodule.cpp
index 2465ea7..28221be 100644
--- a/core/fxcodec/codec/ccodec_jpxmodule.cpp
+++ b/core/fxcodec/codec/ccodec_jpxmodule.cpp
@@ -646,8 +646,6 @@
 std::unique_ptr<CJPX_Decoder> CCodec_JpxModule::CreateDecoder(
     pdfium::span<const uint8_t> src_span,
     CPDF_ColorSpace* cs) {
-  // TODO(thestig): |cs| should never be nullptr in production, but
-  // pdf_jpx_fuzzer.cc passes that in.
   auto decoder = pdfium::MakeUnique<CJPX_Decoder>(cs);
   if (!decoder->Init(src_span))
     return nullptr;