From 2a1cbc0417985485ab660ee6fd98c3bbedf716cd Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Tue, 25 Sep 2018 20:07:29 +0000 Subject: 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 > Reviewed-by: Ryan Harrison 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 Commit-Queue: Lei Zhang --- core/fpdfapi/render/cpdf_dibbase.cpp | 83 ++++++++++++++++++--------------- core/fxcodec/codec/ccodec_jpxmodule.cpp | 2 - 2 files changed, 46 insertions(+), 39 deletions(-) diff --git a/core/fpdfapi/render/cpdf_dibbase.cpp b/core/fpdfapi/render/cpdf_dibbase.cpp index 7391296597..3557672e81 100644 --- a/core/fpdfapi/render/cpdf_dibbase.cpp +++ b/core/fpdfapi/render/cpdf_dibbase.cpp @@ -511,8 +511,6 @@ 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, @@ -541,35 +539,40 @@ bool CPDF_DIBBase::CreateDCTDecoder(pdfium::span src_span, m_nComponents = static_cast(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; + } + 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; - } - 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 @@ 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( @@ -597,13 +598,22 @@ RetainPtr CPDF_DIBBase::LoadJpxBitmap() { if (static_cast(width) < m_Width || static_cast(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 @@ 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 2465ea7bd0..28221be340 100644 --- a/core/fxcodec/codec/ccodec_jpxmodule.cpp +++ b/core/fxcodec/codec/ccodec_jpxmodule.cpp @@ -646,8 +646,6 @@ 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