From 214982c4d02a720c3b1b3de121cddc62189b8848 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Fri, 21 Sep 2018 15:35:42 +0000 Subject: 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 Reviewed-by: Ryan Harrison --- core/fpdfapi/render/cpdf_dibbase.cpp | 83 +++++++++++++++------------------ core/fxcodec/codec/ccodec_jpxmodule.cpp | 2 + 2 files changed, 39 insertions(+), 46 deletions(-) diff --git a/core/fpdfapi/render/cpdf_dibbase.cpp b/core/fpdfapi/render/cpdf_dibbase.cpp index 3557672e81..7391296597 100644 --- a/core/fpdfapi/render/cpdf_dibbase.cpp +++ b/core/fpdfapi/render/cpdf_dibbase.cpp @@ -511,6 +511,8 @@ CPDF_DIBBase::LoadState CPDF_DIBBase::CreateDecoder() { bool CPDF_DIBBase::CreateDCTDecoder(pdfium::span 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 @@ bool CPDF_DIBBase::CreateDCTDecoder(pdfium::span src_span, m_nComponents = static_cast(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; - } - 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; + 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; } - default: { - if (m_pColorSpace->CountComponents() != m_nComponents) - 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; } + 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; @@ -584,6 +581,8 @@ bool CPDF_DIBBase::CreateDCTDecoder(pdfium::span src_span, } RetainPtr CPDF_DIBBase::LoadJpxBitmap() { + ASSERT(m_pColorSpace); // Assigned in LoadColorInfo(). + CCodec_JpxModule* pJpxModule = CPDF_ModuleMgr::Get()->GetJpxModule(); auto context = pdfium::MakeUnique(pJpxModule); context->set_decoder( @@ -598,22 +597,13 @@ RetainPtr CPDF_DIBBase::LoadJpxBitmap() { if (static_cast(width) < m_Width || static_cast(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 @@ RetainPtr CPDF_DIBBase::LoadJpxBitmap() { } 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; } diff --git a/core/fxcodec/codec/ccodec_jpxmodule.cpp b/core/fxcodec/codec/ccodec_jpxmodule.cpp index 28221be340..2465ea7bd0 100644 --- a/core/fxcodec/codec/ccodec_jpxmodule.cpp +++ b/core/fxcodec/codec/ccodec_jpxmodule.cpp @@ -646,6 +646,8 @@ CCodec_JpxModule::~CCodec_JpxModule() {} std::unique_ptr CCodec_JpxModule::CreateDecoder( pdfium::span 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(cs); if (!decoder->Init(src_span)) return nullptr; -- cgit v1.2.3