in the attempt to fix 627393, changed IFX_FileRead's readBlock to return the length it reads

core/fxcodec/codec/fx_codec_progress.cpp

 // Copyright 2014 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.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "core/fxcodec/codec/ccodec_progressivedecoder.h"
 
 #include <algorithm>
 
@@ skipping 321 matching lines @@
     }
   } else {
     uint32_t dwConsume = m_SrcSize - dwAvail;
     if (dwAvail) {
       FXSYS_memmove(m_pSrcBuf, m_pSrcBuf + dwConsume, dwAvail);
     }
     if (dwSize > dwConsume) {
       dwSize = dwConsume;
     }
   }
-  if (!m_pFile->ReadBlock(m_pSrcBuf + dwAvail, m_offSet, dwSize)) {
+  uint32_t readSize = m_pFile->ReadBlock(m_pSrcBuf + dwAvail, m_offSet, dwSize);
+  if (readSize != dwSize && !m_pFile->IsEOF()) {
     err_status = FXCODEC_STATUS_ERR_READ;
     return false;
   }
-  m_offSet += dwSize;
-  pJpegModule->Input(m_pJpegContext, m_pSrcBuf, dwSize + dwAvail);
+  m_offSet += readSize;
+  pJpegModule->Input(m_pJpegContext, m_pSrcBuf, readSize + dwAvail);
   return true;
 }
 
 bool CCodec_ProgressiveDecoder::PngReadHeaderFunc(void* pModule,
                                                   int width,
                                                   int height,
                                                   int bpc,
                                                   int pass,
                                                   int* color_type,
                                                   double* gamma) {
@@ skipping 247 matching lines @@
     }
   } else {
     uint32_t dwConsume = m_SrcSize - dwAvail;
     if (dwAvail) {
       FXSYS_memmove(m_pSrcBuf, m_pSrcBuf + dwConsume, dwAvail);
     }
     if (dwSize > dwConsume) {
       dwSize = dwConsume;
     }
   }
-  if (!m_pFile->ReadBlock(m_pSrcBuf + dwAvail, m_offSet, dwSize)) {
+  uint32_t readSize = m_pFile->ReadBlock(m_pSrcBuf + dwAvail, m_offSet, dwSize);
+  if (readSize != dwSize && !m_pFile->IsEOF()) {
     err_status = FXCODEC_STATUS_ERR_READ;
     return false;
   }
-  m_offSet += dwSize;
-  pGifModule->Input(m_pGifContext, m_pSrcBuf, dwSize + dwAvail);
+  m_offSet += readSize;
+  pGifModule->Input(m_pGifContext, m_pSrcBuf, readSize + dwAvail);
   return true;
 }
 
 void CCodec_ProgressiveDecoder::GifRecordCurrentPositionCallback(
     void* pModule,
     uint32_t& cur_pos) {
   CCodec_ProgressiveDecoder* pCodec = (CCodec_ProgressiveDecoder*)pModule;
   uint32_t remain_size =
       pCodec->m_pCodecMgr->GetGifModule()->GetAvailInput(pCodec->m_pGifContext);
   cur_pos = pCodec->m_offSet - remain_size;
@@ skipping 264 matching lines @@
     }
   } else {
     uint32_t dwConsume = m_SrcSize - dwAvail;
     if (dwAvail) {
       FXSYS_memmove(m_pSrcBuf, m_pSrcBuf + dwConsume, dwAvail);
     }
     if (dwSize > dwConsume) {
       dwSize = dwConsume;
     }
   }
-  if (!m_pFile->ReadBlock(m_pSrcBuf + dwAvail, m_offSet, dwSize)) {
+  uint32_t readSize = m_pFile->ReadBlock(m_pSrcBuf + dwAvail, m_offSet, dwSize);
+  if (readSize != dwSize && !m_pFile->IsEOF()) {
     err_status = FXCODEC_STATUS_ERR_READ;
     return false;
   }
-  m_offSet += dwSize;
-  pBmpModule->Input(m_pBmpContext, m_pSrcBuf, dwSize + dwAvail);
+  m_offSet += readSize;
+  pBmpModule->Input(m_pBmpContext, m_pSrcBuf, readSize + dwAvail);
   return true;
 }
 
 bool CCodec_ProgressiveDecoder::BmpInputImagePositionBufCallback(
     void* pModule,
     uint32_t rcd_pos) {
   CCodec_ProgressiveDecoder* pCodec = (CCodec_ProgressiveDecoder*)pModule;
   pCodec->m_offSet = rcd_pos;
   FXCODEC_STATUS error_status = FXCODEC_STATUS_ERROR;
   return pCodec->BmpReadMoreData(pCodec->m_pCodecMgr->GetBmpModule(),
@@ skipping 136 matching lines @@
         return false;
       }
       pBmpModule->InputImagePositionBufCallback =
           BmpInputImagePositionBufCallback;
       pBmpModule->ReadScanlineCallback = BmpReadScanlineCallback;
       m_pBmpContext = pBmpModule->Start((void*)this);
       if (!m_pBmpContext) {
         m_status = FXCODEC_STATUS_ERR_MEMORY;
         return false;
       }
-      bool bResult = m_pFile->ReadBlock(m_pSrcBuf, 0, size);
-      if (!bResult) {
+      uint32_t readSize = m_pFile->ReadBlock(m_pSrcBuf, 0, size);
+      if (readSize != size && !m_pFile->IsEOF()) {
         m_status = FXCODEC_STATUS_ERR_READ;
         return false;
       }
+      size = readSize;
       m_offSet += size;
       pBmpModule->Input(m_pBmpContext, m_pSrcBuf, size);
       uint32_t* pPalette = nullptr;
       int32_t readResult = pBmpModule->ReadHeader(
           m_pBmpContext, &m_SrcWidth, &m_SrcHeight, &m_BmpIsTopBottom,
           &m_SrcComponents, &m_SrcPaletteNumber, &pPalette, pAttribute);
       while (readResult == 2) {
         FXCODEC_STATUS error_status = FXCODEC_STATUS_ERR_FORMAT;
         if (!BmpReadMoreData(pBmpModule, error_status)) {
           m_status = error_status;
@@ skipping 27 matching lines @@
       CCodec_JpegModule* pJpegModule = m_pCodecMgr->GetJpegModule();
       if (!pJpegModule) {
         m_status = FXCODEC_STATUS_ERR_MEMORY;
         return false;
       }
       m_pJpegContext = pJpegModule->Start();
       if (!m_pJpegContext) {
         m_status = FXCODEC_STATUS_ERR_MEMORY;
         return false;
       }
-      bool bResult = m_pFile->ReadBlock(m_pSrcBuf, 0, size);
-      if (!bResult) {
+      uint32_t readSize = m_pFile->ReadBlock(m_pSrcBuf, 0, size);
+      if (readSize != size && !m_pFile->IsEOF()) {
         m_status = FXCODEC_STATUS_ERR_READ;
         return false;
       }
+      size = readSize;
       m_offSet += size;
       pJpegModule->Input(m_pJpegContext, m_pSrcBuf, size);
       int32_t readResult =
           pJpegModule->ReadHeader(m_pJpegContext, &m_SrcWidth, &m_SrcHeight,
                                   &m_SrcComponents, pAttribute);
       while (readResult == 2) {
         FXCODEC_STATUS error_status = FXCODEC_STATUS_ERR_FORMAT;
         if (!JpegReadMoreData(pJpegModule, error_status)) {
           m_status = error_status;
           return false;
@@ skipping 24 matching lines @@
           CCodec_ProgressiveDecoder::PngReadHeaderFunc;
       pPngModule->AskScanlineBufCallback =
           CCodec_ProgressiveDecoder::PngAskScanlineBufFunc;
       pPngModule->FillScanlineBufCompletedCallback =
           CCodec_ProgressiveDecoder::PngFillScanlineBufCompletedFunc;
       m_pPngContext = pPngModule->Start((void*)this);
       if (!m_pPngContext) {
         m_status = FXCODEC_STATUS_ERR_MEMORY;
         return false;
       }
-      bool bResult = m_pFile->ReadBlock(m_pSrcBuf, 0, size);
-      if (!bResult) {
+      uint32_t readSize = m_pFile->ReadBlock(m_pSrcBuf, 0, size);
+      if (readSize != size && !m_pFile->IsEOF()) {
         m_status = FXCODEC_STATUS_ERR_READ;
         return false;
       }
+      size = readSize;
       m_offSet += size;
-      bResult = pPngModule->Input(m_pPngContext, m_pSrcBuf, size, pAttribute);
+      bool bResult =
+          pPngModule->Input(m_pPngContext, m_pSrcBuf, size, pAttribute);
       while (bResult) {
         uint32_t remain_size = (uint32_t)m_pFile->GetSize() - m_offSet;
         uint32_t input_size =
             remain_size > FXCODEC_BLOCK_SIZE ? FXCODEC_BLOCK_SIZE : remain_size;
         if (input_size == 0) {
           if (m_pPngContext) {
             pPngModule->Finish(m_pPngContext);
           }
           m_pPngContext = nullptr;
           m_status = FXCODEC_STATUS_ERR_FORMAT;
           return false;
         }
         if (m_pSrcBuf && input_size > m_SrcSize) {
           FX_Free(m_pSrcBuf);
           m_pSrcBuf = FX_Alloc(uint8_t, input_size);
           FXSYS_memset(m_pSrcBuf, 0, input_size);
           m_SrcSize = input_size;
         }
-        bResult = m_pFile->ReadBlock(m_pSrcBuf, m_offSet, input_size);
-        if (!bResult) {
+        uint32_t readSize = m_pFile->ReadBlock(m_pSrcBuf, m_offSet, input_size);
+        if (readSize != input_size && !m_pFile->IsEOF()) {
           m_status = FXCODEC_STATUS_ERR_READ;
           return false;
         }
-        m_offSet += input_size;
+        m_offSet += readSize;
         bResult =
-            pPngModule->Input(m_pPngContext, m_pSrcBuf, input_size, pAttribute);
+            pPngModule->Input(m_pPngContext, m_pSrcBuf, readSize, pAttribute);

[Tom Sepez, 2016/11/16 18:32:01]

I think we want to issue reads for the full size even if we previously got a
short read here.

This is the last statement of the loop, and when changed to a full read, can be
combined with line 1182:

while (pPngModule->Input(m_pPngContext, m_pSrcBuf, size, pAttribute)) {

       }
       ASSERT(!bResult);

[Tom Sepez, 2016/11/16 18:32:00]

This is the only use of bResult outside the loop, and there aren't any breaks in
the loop, so we can remove this line and bResult entirely.

       if (m_pPngContext) {
         pPngModule->Finish(m_pPngContext);
         m_pPngContext = nullptr;
       }
       if (m_SrcPassNumber == 0) {
         m_status = FXCODEC_STATUS_ERR_FORMAT;
         return false;
       }
       return true;
     }
     case FXCODEC_IMAGE_GIF: {
       CCodec_GifModule* pGifModule = m_pCodecMgr->GetGifModule();
       if (!pGifModule) {
         m_status = FXCODEC_STATUS_ERR_MEMORY;
         return false;
       }
       pGifModule->RecordCurrentPositionCallback =
           CCodec_ProgressiveDecoder::GifRecordCurrentPositionCallback;
       pGifModule->AskLocalPaletteBufCallback =
           CCodec_ProgressiveDecoder::GifAskLocalPaletteBufCallback;
       pGifModule->InputRecordPositionBufCallback =
           CCodec_ProgressiveDecoder::GifInputRecordPositionBufCallback;
       pGifModule->ReadScanlineCallback =
           CCodec_ProgressiveDecoder::GifReadScanlineCallback;
       m_pGifContext = pGifModule->Start((void*)this);
       if (!m_pGifContext) {
         m_status = FXCODEC_STATUS_ERR_MEMORY;
         return false;
       }
-      bool bResult = m_pFile->ReadBlock(m_pSrcBuf, 0, size);
-      if (!bResult) {
+      uint32_t readSize = m_pFile->ReadBlock(m_pSrcBuf, 0, size);
+      if (readSize != size && !m_pFile->IsEOF()) {
         m_status = FXCODEC_STATUS_ERR_READ;
         return false;
       }
+      size = readSize;
       m_offSet += size;
       pGifModule->Input(m_pGifContext, m_pSrcBuf, size);

[Tom Sepez, 2016/11/16 18:32:00]

Again, do we want a full block read here?

       m_SrcComponents = 1;
       int32_t readResult = pGifModule->ReadHeader(
           m_pGifContext, &m_SrcWidth, &m_SrcHeight, &m_GifPltNumber,
           (void**)&m_pGifPalette, &m_GifBgIndex, nullptr);
       while (readResult == 2) {
         FXCODEC_STATUS error_status = FXCODEC_STATUS_ERR_FORMAT;
         if (!GifReadMoreData(pGifModule, error_status)) {
           m_status = error_status;
           return false;
         }
@@ skipping 881 matching lines @@
           m_pFile = nullptr;
           m_status = FXCODEC_STATUS_DECODE_FINISH;
           return m_status;
         }
         if (m_pSrcBuf && input_size > m_SrcSize) {
           FX_Free(m_pSrcBuf);
           m_pSrcBuf = FX_Alloc(uint8_t, input_size);
           FXSYS_memset(m_pSrcBuf, 0, input_size);
           m_SrcSize = input_size;
         }
-        bool bResult = m_pFile->ReadBlock(m_pSrcBuf, m_offSet, input_size);
-        if (!bResult) {
+        uint32_t readSize = m_pFile->ReadBlock(m_pSrcBuf, m_offSet, input_size);
+        if (readSize != input_size && !m_pFile->IsEOF()) {
           m_pDeviceBitmap = nullptr;
           m_pFile = nullptr;
           m_status = FXCODEC_STATUS_ERR_READ;
           return m_status;
         }
-        m_offSet += input_size;
-        bResult =
-            pPngModule->Input(m_pPngContext, m_pSrcBuf, input_size, nullptr);
+        m_offSet += readSize;
+        bool bResult =

[Tom Sepez, 2016/11/16 18:32:00]

nit: local not needed.

+            pPngModule->Input(m_pPngContext, m_pSrcBuf, readSize, nullptr);
         if (!bResult) {
           m_pDeviceBitmap = nullptr;
           m_pFile = nullptr;
           m_status = FXCODEC_STATUS_ERROR;
           return m_status;
         }
         if (pPause && pPause->NeedToPauseNow()) {
           m_status = FXCODEC_STATUS_DECODE_TOBECONTINUE;
           return m_status;
         }
@@ skipping 206 matching lines @@
       return m_status;
     }
     default:
       return FXCODEC_STATUS_ERROR;
   }
 }
 
 CCodec_ProgressiveDecoder* CCodec_ModuleMgr::CreateProgressiveDecoder() {
   return new CCodec_ProgressiveDecoder(this);
 }