From 0d32b8fda53e02c1036d39f7290d4f59f2b58ca4 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Mon, 16 Apr 2018 21:09:57 +0000 Subject: Revert "Return pdfium::span from WideString::GetBuffer()." This reverts commit 154e18f9a862975abecebe77b8f5fb418418d14c. Reason for revert: Generate CL to merge to beta branch Original change's description: > Return pdfium::span from WideString::GetBuffer(). > > Adds bounds checking "for free", but beware of span outliving > a ReleaseBuffer() call. Scoping as such avoids the possibility > of using an invalid span (and it is flagged as a lifetime issue). > > Change-Id: Ica63f4b1429823d0254ec6951aeaeb08160cb93c > Reviewed-on: https://pdfium-review.googlesource.com/30310 > Reviewed-by: dsinclair > Commit-Queue: Tom Sepez TBR=tsepez@chromium.org,dsinclair@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Change-Id: Ie1ec9434215584a024538ca8edeb59dea555af48 Reviewed-on: https://pdfium-review.googlesource.com/30830 Reviewed-by: Tom Sepez Reviewed-by: dsinclair Commit-Queue: Tom Sepez --- core/fpdfapi/parser/fpdf_parser_decode.cpp | 11 +-- core/fxcrt/cfx_blockbuffer.cpp | 55 +++++++-------- core/fxcrt/widestring.cpp | 65 ++++++++---------- core/fxcrt/widestring.h | 7 +- core/fxcrt/widestring_unittest.cpp | 21 +++--- fxjs/cfxjse_resolveprocessor.cpp | 107 ++++++++++++++--------------- fxjs/xfa/cjx_hostpseudomodel.cpp | 20 +++--- xfa/fxfa/cxfa_textlayout.cpp | 28 ++++---- xfa/fxfa/parser/cxfa_localevalue.cpp | 44 ++++++------ 9 files changed, 167 insertions(+), 191 deletions(-) diff --git a/core/fpdfapi/parser/fpdf_parser_decode.cpp b/core/fpdfapi/parser/fpdf_parser_decode.cpp index 90dca2edcb..d7114b66c5 100644 --- a/core/fpdfapi/parser/fpdf_parser_decode.cpp +++ b/core/fpdfapi/parser/fpdf_parser_decode.cpp @@ -425,7 +425,6 @@ bool PDF_DataDecode(const uint8_t* src_buf, } WideString PDF_DecodeText(const uint8_t* src_data, uint32_t src_len) { - int dest_pos = 0; WideString result; if (src_len >= 2 && ((src_data[0] == 0xfe && src_data[1] == 0xff) || (src_data[0] == 0xff && src_data[1] == 0xfe))) { @@ -433,15 +432,17 @@ WideString PDF_DecodeText(const uint8_t* src_data, uint32_t src_len) { if (!max_chars) return result; - pdfium::span dest_buf = result.GetBuffer(max_chars); bool bBE = src_data[0] == 0xfe || (src_data[0] == 0xff && !src_data[2]); + wchar_t* dest_buf = result.GetBuffer(max_chars); const uint8_t* uni_str = src_data + 2; + int dest_pos = 0; for (uint32_t i = 0; i < max_chars * 2; i += 2) { uint16_t unicode = GetUnicodeFromBytes(uni_str + i, bBE); if (unicode != 0x1b) { dest_buf[dest_pos++] = unicode; continue; } + i += 2; while (i < max_chars * 2) { uint16_t unicode2 = GetUnicodeFromBytes(uni_str + i, bBE); @@ -450,13 +451,13 @@ WideString PDF_DecodeText(const uint8_t* src_data, uint32_t src_len) { break; } } + result.ReleaseBuffer(dest_pos); } else { - pdfium::span dest_buf = result.GetBuffer(src_len); + wchar_t* dest_buf = result.GetBuffer(src_len); for (uint32_t i = 0; i < src_len; ++i) dest_buf[i] = PDFDocEncoding[src_data[i]]; - dest_pos = src_len; + result.ReleaseBuffer(src_len); } - result.ReleaseBuffer(dest_pos); return result; } diff --git a/core/fxcrt/cfx_blockbuffer.cpp b/core/fxcrt/cfx_blockbuffer.cpp index 6a7d98aa18..13134f0e7b 100644 --- a/core/fxcrt/cfx_blockbuffer.cpp +++ b/core/fxcrt/cfx_blockbuffer.cpp @@ -77,39 +77,36 @@ WideString CFX_BlockBuffer::GetTextData(size_t start, size_t length) const { size_t maybeDataLength = m_BufferSize - 1 - m_StartPosition; if (start > maybeDataLength) return WideString(); - length = std::min(length, maybeDataLength); - if (!length) - return WideString(); WideString wsTextData; - { - // Span's lifetime must end before ReleaseBuffer() below. - pdfium::span pBuf = wsTextData.GetBuffer(length); - size_t startBlock = 0; - size_t startInner = 0; - std::tie(startBlock, startInner) = TextDataIndex2BufIndex(start); - - size_t endBlock = 0; - size_t endInner = 0; - std::tie(endBlock, endInner) = TextDataIndex2BufIndex(start + length); - - size_t pointer = 0; - for (size_t i = startBlock; i <= endBlock; ++i) { - size_t bufferPointer = 0; - size_t copyLength = kAllocStep; - if (i == startBlock) { - copyLength -= startInner; - bufferPointer = startInner; - } - if (i == endBlock) - copyLength -= ((kAllocStep - 1) - endInner); - - wchar_t* pBlockBuf = m_BlockArray[i].get(); - memcpy(&pBuf[pointer], pBlockBuf + bufferPointer, - copyLength * sizeof(wchar_t)); - pointer += copyLength; + wchar_t* pBuf = wsTextData.GetBuffer(length); + if (!pBuf) + return WideString(); + + size_t startBlock = 0; + size_t startInner = 0; + std::tie(startBlock, startInner) = TextDataIndex2BufIndex(start); + + size_t endBlock = 0; + size_t endInner = 0; + std::tie(endBlock, endInner) = TextDataIndex2BufIndex(start + length); + + size_t pointer = 0; + for (size_t i = startBlock; i <= endBlock; ++i) { + size_t bufferPointer = 0; + size_t copyLength = kAllocStep; + if (i == startBlock) { + copyLength -= startInner; + bufferPointer = startInner; } + if (i == endBlock) + copyLength -= ((kAllocStep - 1) - endInner); + + wchar_t* pBlockBuf = m_BlockArray[i].get(); + memcpy(pBuf + pointer, pBlockBuf + bufferPointer, + copyLength * sizeof(wchar_t)); + pointer += copyLength; } wsTextData.ReleaseBuffer(length); return wsTextData; diff --git a/core/fxcrt/widestring.cpp b/core/fxcrt/widestring.cpp index a3525593ee..7b5bf66fd3 100644 --- a/core/fxcrt/widestring.cpp +++ b/core/fxcrt/widestring.cpp @@ -252,27 +252,22 @@ Optional GuessSizeForVSWPrintf(const wchar_t* pFormat, Optional TryVSWPrintf(size_t size, const wchar_t* pFormat, va_list argList) { - if (!size) + WideString str; + wchar_t* buffer = str.GetBuffer(size); + + // In the following two calls, there's always space in the buffer for + // a terminating NUL that's not included in nMaxLen. + // For vswprintf(), MSAN won't untaint the buffer on a truncated write's + // -1 return code even though the buffer is written. Probably just as well + // not to trust the vendor's implementation to write anything anyways. + // See https://crbug.com/705912. + memset(buffer, 0, (size + 1) * sizeof(wchar_t)); + int ret = vswprintf(buffer, size + 1, pFormat, argList); + + bool bSufficientBuffer = ret >= 0 || buffer[size - 1] == 0; + if (!bSufficientBuffer) return {}; - WideString str; - { - // Span's lifetime must end before ReleaseBuffer() below. - pdfium::span buffer = str.GetBuffer(size); - - // In the following two calls, there's always space in the WideString - // for a terminating NUL that's not included in the span. - // For vswprintf(), MSAN won't untaint the buffer on a truncated write's - // -1 return code even though the buffer is written. Probably just as well - // not to trust the vendor's implementation to write anything anyways. - // See https://crbug.com/705912. - memset(buffer.data(), 0, (size + 1) * sizeof(wchar_t)); - int ret = vswprintf(buffer.data(), size + 1, pFormat, argList); - - bool bSufficientBuffer = ret >= 0 || buffer[size - 1] == 0; - if (!bSufficientBuffer) - return {}; - } str.ReleaseBuffer(str.GetStringLength()); return {str}; } @@ -304,12 +299,9 @@ WideString GetWideString(uint16_t codepage, const ByteStringView& bstr) { return WideString(); WideString wstr; - { - // Span's lifetime must end before ReleaseBuffer() below. - pdfium::span dest_buf = wstr.GetBuffer(dest_len); - FXSYS_MultiByteToWideChar(codepage, 0, bstr.unterminated_c_str(), src_len, - dest_buf.data(), dest_len); - } + wchar_t* dest_buf = wstr.GetBuffer(dest_len); + FXSYS_MultiByteToWideChar(codepage, 0, bstr.unterminated_c_str(), src_len, + dest_buf, dest_len); wstr.ReleaseBuffer(dest_len); return wstr; } @@ -594,29 +586,29 @@ void WideString::Reserve(size_t len) { GetBuffer(len); } -pdfium::span WideString::GetBuffer(size_t nMinBufLength) { +wchar_t* WideString::GetBuffer(size_t nMinBufLength) { if (!m_pData) { if (nMinBufLength == 0) - return pdfium::span(); + return nullptr; m_pData.Reset(StringData::Create(nMinBufLength)); m_pData->m_nDataLength = 0; m_pData->m_String[0] = 0; - return pdfium::span(m_pData->m_String, m_pData->m_nAllocLength); + return m_pData->m_String; } if (m_pData->CanOperateInPlace(nMinBufLength)) - return pdfium::span(m_pData->m_String, m_pData->m_nAllocLength); + return m_pData->m_String; nMinBufLength = std::max(nMinBufLength, m_pData->m_nDataLength); if (nMinBufLength == 0) - return pdfium::span(); + return nullptr; RetainPtr pNewData(StringData::Create(nMinBufLength)); pNewData->CopyContents(*m_pData); pNewData->m_nDataLength = m_pData->m_nDataLength; m_pData.Swap(pNewData); - return pdfium::span(m_pData->m_String, m_pData->m_nAllocLength); + return m_pData->m_String; } size_t WideString::Delete(size_t index, size_t count) { @@ -893,15 +885,14 @@ WideString WideString::FromUTF8(const ByteStringView& str) { // static WideString WideString::FromUTF16LE(const unsigned short* wstr, size_t wlen) { - if (!wstr || wlen == 0) + if (!wstr || wlen == 0) { return WideString(); + } WideString result; - { - // Span's lifetime must end before ReleaseBuffer() below. - pdfium::span buf = result.GetBuffer(wlen); - for (size_t i = 0; i < wlen; i++) - buf[i] = wstr[i]; + wchar_t* buf = result.GetBuffer(wlen); + for (size_t i = 0; i < wlen; i++) { + buf[i] = wstr[i]; } result.ReleaseBuffer(wlen); return result; diff --git a/core/fxcrt/widestring.h b/core/fxcrt/widestring.h index b668b0292c..cefdfabcd0 100644 --- a/core/fxcrt/widestring.h +++ b/core/fxcrt/widestring.h @@ -17,7 +17,7 @@ #include "core/fxcrt/string_data_template.h" #include "core/fxcrt/string_view_template.h" #include "third_party/base/optional.h" -#include "third_party/base/span.h" + namespace fxcrt { @@ -171,10 +171,7 @@ class WideString { void TrimRight(const WideStringView& targets); void Reserve(size_t len); - - // Note: any modification of the string (including ReleaseBuffer()) may - // invalidate the span, which must not outlive its buffer. - pdfium::span GetBuffer(size_t len); + wchar_t* GetBuffer(size_t len); void ReleaseBuffer(size_t len); int GetInteger() const; diff --git a/core/fxcrt/widestring_unittest.cpp b/core/fxcrt/widestring_unittest.cpp index ec0a55fc1e..48d5c7f8f5 100644 --- a/core/fxcrt/widestring_unittest.cpp +++ b/core/fxcrt/widestring_unittest.cpp @@ -817,21 +817,20 @@ TEST(WideString, Reserve) { } TEST(WideString, GetBuffer) { - WideString str1; { - pdfium::span buffer = str1.GetBuffer(12); - wcscpy(buffer.data(), L"clams"); + WideString str; + wchar_t* buffer = str.GetBuffer(12); + wcscpy(buffer, L"clams"); + str.ReleaseBuffer(str.GetStringLength()); + EXPECT_EQ(L"clams", str); } - str1.ReleaseBuffer(str1.GetStringLength()); - EXPECT_EQ(L"clams", str1); - - WideString str2(L"cl"); { - pdfium::span buffer = str2.GetBuffer(12); - wcscpy(buffer.data() + 2, L"ams"); + WideString str(L"cl"); + wchar_t* buffer = str.GetBuffer(12); + wcscpy(buffer + 2, L"ams"); + str.ReleaseBuffer(str.GetStringLength()); + EXPECT_EQ(L"clams", str); } - str2.ReleaseBuffer(str2.GetStringLength()); - EXPECT_EQ(L"clams", str2); } TEST(WideString, ReleaseBuffer) { diff --git a/fxjs/cfxjse_resolveprocessor.cpp b/fxjs/cfxjse_resolveprocessor.cpp index 46163b55fc..2ca0838a7f 100644 --- a/fxjs/cfxjse_resolveprocessor.cpp +++ b/fxjs/cfxjse_resolveprocessor.cpp @@ -499,69 +499,66 @@ int32_t CFXJSE_ResolveProcessor::GetFilter(const WideStringView& wsExpression, WideString& wsName = rnd.m_wsName; WideString& wsCondition = rnd.m_wsCondition; + wchar_t* pNameBuf = wsName.GetBuffer(iLength - nStart); + wchar_t* pConditionBuf = wsCondition.GetBuffer(iLength - nStart); int32_t nNameCount = 0; int32_t nConditionCount = 0; - { - // Span's lifetime must end before ReleaseBuffer() below. - pdfium::span pNameBuf = wsName.GetBuffer(iLength - nStart); - pdfium::span pConditionBuf = - wsCondition.GetBuffer(iLength - nStart); - std::vector stack; - int32_t nType = -1; - const wchar_t* pSrc = wsExpression.unterminated_c_str(); - wchar_t wPrev = 0; - wchar_t wCur; - bool bIsCondition = false; - while (nStart < iLength) { - wCur = pSrc[nStart++]; - if (wCur == '.') { - if (wPrev == '\\') { - pNameBuf[nNameCount - 1] = wPrev = '.'; - continue; - } - if (nNameCount == 0) { - rnd.m_dwStyles |= XFA_RESOLVENODE_AnyChild; - continue; - } - - wchar_t wLookahead = nStart < iLength ? pSrc[nStart] : 0; - if (wLookahead != '[' && wLookahead != '(' && nType < 0) - break; - } - if (wCur == '[' || wCur == '(') { - bIsCondition = true; - } else if (wCur == '.' && nStart < iLength && - (pSrc[nStart] == '[' || pSrc[nStart] == '(')) { - bIsCondition = true; + std::vector stack; + int32_t nType = -1; + const wchar_t* pSrc = wsExpression.unterminated_c_str(); + wchar_t wPrev = 0; + wchar_t wCur; + bool bIsCondition = false; + while (nStart < iLength) { + wCur = pSrc[nStart++]; + if (wCur == '.') { + if (wPrev == '\\') { + pNameBuf[nNameCount - 1] = wPrev = '.'; + continue; } - if (bIsCondition) - pConditionBuf[nConditionCount++] = wCur; - else - pNameBuf[nNameCount++] = wCur; - - if ((nType == 0 && wCur == ']') || (nType == 1 && wCur == ')') || - (nType == 2 && wCur == '"')) { - nType = stack.empty() ? -1 : stack.back(); - if (!stack.empty()) - stack.pop_back(); - } else if (wCur == '[') { - stack.push_back(nType); - nType = 0; - } else if (wCur == '(') { - stack.push_back(nType); - nType = 1; - } else if (wCur == '"') { - stack.push_back(nType); - nType = 2; + if (nNameCount == 0) { + rnd.m_dwStyles |= XFA_RESOLVENODE_AnyChild; + continue; } - wPrev = wCur; + + wchar_t wLookahead = nStart < iLength ? pSrc[nStart] : 0; + if (wLookahead != '[' && wLookahead != '(' && nType < 0) + break; + } + if (wCur == '[' || wCur == '(') { + bIsCondition = true; + } else if (wCur == '.' && nStart < iLength && + (pSrc[nStart] == '[' || pSrc[nStart] == '(')) { + bIsCondition = true; + } + if (bIsCondition) + pConditionBuf[nConditionCount++] = wCur; + else + pNameBuf[nNameCount++] = wCur; + + if ((nType == 0 && wCur == ']') || (nType == 1 && wCur == ')') || + (nType == 2 && wCur == '"')) { + nType = stack.empty() ? -1 : stack.back(); + if (!stack.empty()) + stack.pop_back(); + } else if (wCur == '[') { + stack.push_back(nType); + nType = 0; + } else if (wCur == '(') { + stack.push_back(nType); + nType = 1; + } else if (wCur == '"') { + stack.push_back(nType); + nType = 2; } - if (!stack.empty()) - return -1; + wPrev = wCur; } + if (!stack.empty()) + return -1; + wsName.ReleaseBuffer(nNameCount); - wsCondition.ReleaseBuffer(nConditionCount); wsName.Trim(); + wsCondition.ReleaseBuffer(nConditionCount); wsCondition.Trim(); rnd.m_uHashName = static_cast(FX_HashCode_GetW(wsName.AsStringView(), false)); diff --git a/fxjs/xfa/cjx_hostpseudomodel.cpp b/fxjs/xfa/cjx_hostpseudomodel.cpp index fe26d3161d..6ca431e78d 100644 --- a/fxjs/xfa/cjx_hostpseudomodel.cpp +++ b/fxjs/xfa/cjx_hostpseudomodel.cpp @@ -29,18 +29,16 @@ int32_t FilterName(const WideStringView& wsExpression, if (nStart >= iLength) return iLength; + wchar_t* pBuf = wsFilter.GetBuffer(iLength - nStart); int32_t nCount = 0; - { - // Span's lifetime must end before ReleaseBuffer() below. - pdfium::span pBuf = wsFilter.GetBuffer(iLength - nStart); - const wchar_t* pSrc = wsExpression.unterminated_c_str(); - while (nStart < iLength) { - wchar_t wCur = pSrc[nStart++]; - if (wCur == ',') - break; - - pBuf[nCount++] = wCur; - } + const wchar_t* pSrc = wsExpression.unterminated_c_str(); + wchar_t wCur; + while (nStart < iLength) { + wCur = pSrc[nStart++]; + if (wCur == ',') + break; + + pBuf[nCount++] = wCur; } wsFilter.ReleaseBuffer(nCount); wsFilter.Trim(); diff --git a/xfa/fxfa/cxfa_textlayout.cpp b/xfa/fxfa/cxfa_textlayout.cpp index 5359a340e5..e360c16dde 100644 --- a/xfa/fxfa/cxfa_textlayout.cpp +++ b/xfa/fxfa/cxfa_textlayout.cpp @@ -911,23 +911,21 @@ void CXFA_TextLayout::ProcessText(WideString& wsText) { if (iLen == 0) return; + wchar_t* psz = wsText.GetBuffer(iLen); int32_t iTrimLeft = 0; - { - // Span's lifetime must end before ReleaseBuffer() below. - pdfium::span psz = wsText.GetBuffer(iLen); - wchar_t wPrev = 0; - for (int32_t i = 0; i < iLen; i++) { - wchar_t wch = psz[i]; - if (wch < 0x20) - wch = 0x20; - if (wch == 0x20 && wPrev == 0x20) - continue; - - wPrev = wch; - psz[iTrimLeft++] = wch; - } + wchar_t wch = 0, wPrev = 0; + for (int32_t i = 0; i < iLen; i++) { + wch = psz[i]; + if (wch < 0x20) + wch = 0x20; + if (wch == 0x20 && wPrev == 0x20) + continue; + + wPrev = wch; + psz[iTrimLeft++] = wch; } - wsText.ReleaseBuffer(iTrimLeft); + wsText.ReleaseBuffer(iLen); + wsText = wsText.Left(iTrimLeft); } void CXFA_TextLayout::EndBreak(CFX_BreakType dwStatus, diff --git a/xfa/fxfa/parser/cxfa_localevalue.cpp b/xfa/fxfa/parser/cxfa_localevalue.cpp index 057eaeba2c..cec59de37f 100644 --- a/xfa/fxfa/parser/cxfa_localevalue.cpp +++ b/xfa/fxfa/parser/cxfa_localevalue.cpp @@ -682,34 +682,32 @@ void CXFA_LocaleValue::GetNumericFormat(WideString& wsFormat, int32_t nDecLen) { ASSERT(wsFormat.IsEmpty()); ASSERT(nIntLen >= -1 && nDecLen >= -1); + int32_t nTotalLen = (nIntLen >= 0 ? nIntLen : 2) + 1 + (nDecLen >= 0 ? nDecLen : 2) + (nDecLen == 0 ? 0 : 1); - { - // Span's lifetime must end before ReleaseBuffer() below. - pdfium::span lpBuf = wsFormat.GetBuffer(nTotalLen); - int32_t nPos = 0; - lpBuf[nPos++] = L's'; - - if (nIntLen == -1) { + wchar_t* lpBuf = wsFormat.GetBuffer(nTotalLen); + int32_t nPos = 0; + lpBuf[nPos++] = L's'; + + if (nIntLen == -1) { + lpBuf[nPos++] = L'z'; + lpBuf[nPos++] = L'*'; + } else { + while (nIntLen) { lpBuf[nPos++] = L'z'; - lpBuf[nPos++] = L'*'; - } else { - while (nIntLen) { - lpBuf[nPos++] = L'z'; - nIntLen--; - } - } - if (nDecLen != 0) { - lpBuf[nPos++] = L'.'; + nIntLen--; } - if (nDecLen == -1) { + } + if (nDecLen != 0) { + lpBuf[nPos++] = L'.'; + } + if (nDecLen == -1) { + lpBuf[nPos++] = L'z'; + lpBuf[nPos++] = L'*'; + } else { + while (nDecLen) { lpBuf[nPos++] = L'z'; - lpBuf[nPos++] = L'*'; - } else { - while (nDecLen) { - lpBuf[nPos++] = L'z'; - nDecLen--; - } + nDecLen--; } } wsFormat.ReleaseBuffer(nTotalLen); -- cgit v1.2.3