From 980a3ea30872cef9ada360aa85e7c3573d7668b5 Mon Sep 17 00:00:00 2001 From: Ryan Harrison Date: Wed, 30 Aug 2017 10:22:55 -0400 Subject: Cleanup CFX_SeekableStreamProxy Clean up being done in preperation for removal of negative length strings. This means that FX_STRSIZE will become unsigned so the return methods cannot return -1. Other cleanup suggested by reviewers is included. BUG=pdfium:828 Change-Id: I2cfb78c8ab7738516e0c9f8a99caec6f6cb12cde Reviewed-on: https://pdfium-review.googlesource.com/12170 Reviewed-by: dsinclair Commit-Queue: Ryan Harrison --- core/fxcrt/cfx_seekablestreamproxy.cpp | 71 +++++++++++++--------------------- core/fxcrt/cfx_seekablestreamproxy.h | 4 +- core/fxcrt/xml/cfx_xmlsyntaxparser.cpp | 6 +-- core/fxcrt/xml/cfx_xmlsyntaxparser.h | 4 +- 4 files changed, 33 insertions(+), 52 deletions(-) diff --git a/core/fxcrt/cfx_seekablestreamproxy.cpp b/core/fxcrt/cfx_seekablestreamproxy.cpp index 6906e1c84d..e610e0a9c3 100644 --- a/core/fxcrt/cfx_seekablestreamproxy.cpp +++ b/core/fxcrt/cfx_seekablestreamproxy.cpp @@ -130,19 +130,11 @@ void SwapByteOrder(wchar_t* pStr, FX_STRSIZE iLength) { } // namespace -#if _FX_ENDIAN_ == _FX_LITTLE_ENDIAN_ #define BOM_MASK 0x00FFFFFF #define BOM_UTF8 0x00BFBBEF #define BOM_UTF16_MASK 0x0000FFFF #define BOM_UTF16_BE 0x0000FFFE #define BOM_UTF16_LE 0x0000FEFF -#else -#define BOM_MASK 0xFFFFFF00 -#define BOM_UTF8 0xEFBBBF00 -#define BOM_UTF16_MASK 0xFFFF0000 -#define BOM_UTF16_BE 0xFEFF0000 -#define BOM_UTF16_LE 0xFFFE0000 -#endif // _FX_ENDIAN_ == _FX_LITTLE_ENDIAN_ CFX_SeekableStreamProxy::CFX_SeekableStreamProxy( const CFX_RetainPtr& stream, @@ -159,8 +151,7 @@ CFX_SeekableStreamProxy::CFX_SeekableStreamProxy( return; } - FX_FILESIZE iPosition = GetPosition(); - Seek(CFX_SeekableStreamProxy::Pos::Begin, 0); + Seek(From::Begin, 0); uint32_t bom = 0; ReadData(reinterpret_cast(&bom), 3); @@ -183,8 +174,7 @@ CFX_SeekableStreamProxy::CFX_SeekableStreamProxy( } } - Seek(CFX_SeekableStreamProxy::Pos::Begin, - std::max(static_cast(m_wBOMLength), iPosition)); + Seek(From::Begin, static_cast(m_wBOMLength)); } CFX_SeekableStreamProxy::CFX_SeekableStreamProxy(uint8_t* data, FX_STRSIZE size) @@ -194,15 +184,17 @@ CFX_SeekableStreamProxy::CFX_SeekableStreamProxy(uint8_t* data, FX_STRSIZE size) CFX_SeekableStreamProxy::~CFX_SeekableStreamProxy() {} -void CFX_SeekableStreamProxy::Seek(CFX_SeekableStreamProxy::Pos eSeek, - FX_FILESIZE iOffset) { +void CFX_SeekableStreamProxy::Seek(From eSeek, FX_FILESIZE iOffset) { switch (eSeek) { - case CFX_SeekableStreamProxy::Pos::Begin: + case From::Begin: m_iPosition = iOffset; break; - case CFX_SeekableStreamProxy::Pos::Current: - m_iPosition += iOffset; - break; + case From::Current: { + pdfium::base::CheckedNumeric new_pos = m_iPosition; + new_pos += iOffset; + m_iPosition = + new_pos.ValueOrDefault(std::numeric_limits::max()); + } break; } m_iPosition = pdfium::clamp(m_iPosition, static_cast(0), GetLength()); @@ -219,23 +211,20 @@ FX_STRSIZE CFX_SeekableStreamProxy::ReadData(uint8_t* pBuffer, ASSERT(pBuffer && iBufferSize > 0); if (m_IsWriteStream) - return -1; + return 0; - iBufferSize = std::min( - iBufferSize, static_cast(m_pStream->GetSize() - m_iPosition)); + iBufferSize = + std::min(iBufferSize, static_cast(GetLength() - m_iPosition)); if (iBufferSize <= 0) return 0; - if (m_pStream->ReadBlock(pBuffer, m_iPosition, iBufferSize)) { - pdfium::base::CheckedNumeric new_pos = m_iPosition; - new_pos += iBufferSize; - if (!new_pos.IsValid()) - return 0; + if (!m_pStream->ReadBlock(pBuffer, m_iPosition, iBufferSize)) + return 0; - m_iPosition = new_pos.ValueOrDie(); - return iBufferSize; - } - return 0; + pdfium::base::CheckedNumeric new_pos = m_iPosition; + new_pos += iBufferSize; + m_iPosition = new_pos.ValueOrDefault(m_iPosition); + return new_pos.IsValid() ? iBufferSize : 0; } FX_STRSIZE CFX_SeekableStreamProxy::ReadString(wchar_t* pStr, @@ -245,7 +234,7 @@ FX_STRSIZE CFX_SeekableStreamProxy::ReadString(wchar_t* pStr, ASSERT(iMaxLength > 0); if (m_IsWriteStream) - return -1; + return 0; if (m_wCodePage == FX_CODEPAGE_UTF16LE || m_wCodePage == FX_CODEPAGE_UTF16BE) { @@ -255,13 +244,8 @@ FX_STRSIZE CFX_SeekableStreamProxy::ReadString(wchar_t* pStr, if (sizeof(wchar_t) > 2 && iMaxLength > 0) UTF16ToWChar(pStr, iMaxLength); -#if _FX_ENDIAN_ == _FX_BIG_ENDIAN_ - if (m_wCodePage == FX_CODEPAGE_UTF16LE) - SwapByteOrder(pStr, iMaxLength); -#else if (m_wCodePage == FX_CODEPAGE_UTF16BE) SwapByteOrder(pStr, iMaxLength); -#endif } else { FX_FILESIZE pos = GetPosition(); @@ -273,12 +257,12 @@ FX_STRSIZE CFX_SeekableStreamProxy::ReadString(wchar_t* pStr, FX_STRSIZE iLen = ReadData(buf.data(), iBytes); if (m_wCodePage != FX_CODEPAGE_UTF8) - return -1; + return 0; FX_STRSIZE iSrc = 0; std::tie(iSrc, iMaxLength) = UTF8Decode( reinterpret_cast(buf.data()), iLen, pStr, iMaxLength); - Seek(CFX_SeekableStreamProxy::Pos::Current, iSrc - iLen); + Seek(From::Current, iSrc - iLen); } else { iMaxLength = 0; } @@ -298,12 +282,9 @@ void CFX_SeekableStreamProxy::WriteString(const CFX_WideStringC& str) { return; } - pdfium::base::CheckedNumeric new_pos = m_iPosition; + pdfium::base::CheckedNumeric new_pos = m_iPosition; new_pos += str.GetLength() * sizeof(wchar_t); - if (!new_pos.IsValid()) { - m_iPosition = std::numeric_limits::max(); - return; - } - - m_iPosition = new_pos.ValueOrDie(); + m_iPosition = new_pos.ValueOrDefault(std::numeric_limits::max()); + m_iPosition = + pdfium::clamp(m_iPosition, static_cast(0), GetLength()); } diff --git a/core/fxcrt/cfx_seekablestreamproxy.h b/core/fxcrt/cfx_seekablestreamproxy.h index d059fb8956..1a8e6f2f3b 100644 --- a/core/fxcrt/cfx_seekablestreamproxy.h +++ b/core/fxcrt/cfx_seekablestreamproxy.h @@ -15,7 +15,7 @@ class CFX_SeekableStreamProxy : public CFX_Retainable { public: - enum class Pos { + enum class From { Begin = 0, Current, }; @@ -28,7 +28,7 @@ class CFX_SeekableStreamProxy : public CFX_Retainable { FX_STRSIZE GetBOMLength() const { return std::max(0, m_wBOMLength); } bool IsEOF() const { return m_iPosition >= GetLength(); } - void Seek(CFX_SeekableStreamProxy::Pos eSeek, FX_FILESIZE iOffset); + void Seek(From eSeek, FX_FILESIZE iOffset); FX_STRSIZE ReadString(wchar_t* pStr, FX_STRSIZE iMaxLength, bool* bEOS); void WriteString(const CFX_WideStringC& str); diff --git a/core/fxcrt/xml/cfx_xmlsyntaxparser.cpp b/core/fxcrt/xml/cfx_xmlsyntaxparser.cpp index e3d690f5ab..bbbc4f4399 100644 --- a/core/fxcrt/xml/cfx_xmlsyntaxparser.cpp +++ b/core/fxcrt/xml/cfx_xmlsyntaxparser.cpp @@ -128,8 +128,8 @@ FX_XmlSyntaxResult CFX_XMLSyntaxParser::DoSyntaxParse() { return m_syntaxParserResult; } - int32_t iStreamLength = m_pStream->GetLength(); - int32_t iPos; + FX_FILESIZE iStreamLength = m_pStream->GetLength(); + FX_FILESIZE iPos; FX_XmlSyntaxResult syntaxParserResult = FX_XmlSyntaxResult::None; while (true) { @@ -141,7 +141,7 @@ FX_XmlSyntaxResult CFX_XMLSyntaxParser::DoSyntaxParse() { m_ParsedChars += m_End; m_iParsedBytes = m_iCurrentPos; if (m_pStream->GetPosition() != m_iCurrentPos) - m_pStream->Seek(CFX_SeekableStreamProxy::Pos::Begin, m_iCurrentPos); + m_pStream->Seek(CFX_SeekableStreamProxy::From::Begin, m_iCurrentPos); m_iBufferChars = m_pStream->ReadString(m_Buffer.data(), m_iXMLPlaneSize, &m_bEOS); diff --git a/core/fxcrt/xml/cfx_xmlsyntaxparser.h b/core/fxcrt/xml/cfx_xmlsyntaxparser.h index 32e55f1ff0..c59cabae19 100644 --- a/core/fxcrt/xml/cfx_xmlsyntaxparser.h +++ b/core/fxcrt/xml/cfx_xmlsyntaxparser.h @@ -102,13 +102,13 @@ class CFX_XMLSyntaxParser { CFX_RetainPtr m_pStream; FX_STRSIZE m_iXMLPlaneSize; - int32_t m_iCurrentPos; + FX_FILESIZE m_iCurrentPos; int32_t m_iCurrentNodeNum; int32_t m_iLastNodeNum; int32_t m_iParsedBytes; FX_FILESIZE m_ParsedChars; std::vector m_Buffer; - int32_t m_iBufferChars; + FX_STRSIZE m_iBufferChars; bool m_bEOS; FX_FILESIZE m_Start; // Start position in m_Buffer FX_FILESIZE m_End; // End position in m_Buffer -- cgit v1.2.3