commit 214982c4d02a720c3b1b3de121cddc62189b8848
author Lei Zhang <thestig@chromium.org> Fri Sep 21 15:35:42 2018 +0000
committer Chromium commit bot <commit-bot@chromium.org> Fri Sep 21 15:35:42 2018 +0000
tree 7a1332cfe41ada262662e743bd98a6c688546b3d
parent 8590a98297c5a56e63efc63931c1b3998160e049

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>

core/fpdfapi/render/cpdf_dibbase.cpp

diff --git a/core/fpdfapi/render/cpdf_dibbase.cpp b/core/fpdfapi/render/cpdf_dibbase.cpp
index 3557672..7391296 100644
--- a/core/fpdfapi/render/cpdf_dibbase.cpp
+++ b/core/fpdfapi/render/cpdf_dibbase.cpp
@@ -511,6 +511,8 @@
 
 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,
@@ -539,40 +541,35 @@
 
   m_nComponents = static_cast<uint32_t>(comps);
   m_CompData.clear();
-  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;
+  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;
       }
-      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;
-        }
-        break;
-      }
-      default: {
-        if (m_pColorSpace->CountComponents() != m_nComponents)
-          return false;
-        break;
-      }
+      break;
     }
-  } else {
-    if (m_Family == PDFCS_LAB && m_nComponents != 3)
-      return false;
+    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;
+      }
+      break;
+    }
+    default: {
+      if (m_pColorSpace->CountComponents() != m_nComponents)
+        return false;
+      break;
+    }
   }
   if (!GetDecodeAndMaskArray(&m_bDefaultDecode, &m_bColorKey))
     return false;
@@ -584,6 +581,8 @@
 }
 
 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(
@@ -598,22 +597,13 @@
   if (static_cast<int>(width) < m_Width || static_cast<int>(height) < m_Height)
     return nullptr;
 
-  bool bSwapRGB = false;
-  if (m_pColorSpace) {
-    if (components != m_pColorSpace->CountComponents())
-      return nullptr;
+  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;
+  bool bSwapRGB = false;
+  if (m_pColorSpace == CPDF_ColorSpace::GetStockCS(PDFCS_DEVICERGB)) {
+    bSwapRGB = true;
+    m_pColorSpace = nullptr;
   }
 
   FXDIB_Format format;
@@ -624,6 +614,7 @@
   } 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 28221be..2465ea7 100644
--- a/core/fxcodec/codec/ccodec_jpxmodule.cpp
+++ b/core/fxcodec/codec/ccodec_jpxmodule.cpp
@@ -646,6 +646,8 @@
 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;