Revert "Return pdfium::span<wchar_t> from WideString::GetBuffer()."
This reverts commit 154e18f9a862975abecebe77b8f5fb418418d14c.
Reason for revert: Generate CL to merge to beta branch
Original change's description:
> Return pdfium::span<wchar_t> 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 <dsinclair@chromium.org>
> Commit-Queue: Tom Sepez <tsepez@chromium.org>
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 <tsepez@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
(cherry picked from commit 0d32b8fda53e02c1036d39f7290d4f59f2b58ca4)
Reviewed-on: https://pdfium-review.googlesource.com/30951
diff --git a/core/fpdfapi/parser/fpdf_parser_decode.cpp b/core/fpdfapi/parser/fpdf_parser_decode.cpp
index 90dca2e..d7114b6 100644
--- a/core/fpdfapi/parser/fpdf_parser_decode.cpp
+++ b/core/fpdfapi/parser/fpdf_parser_decode.cpp
@@ -425,7 +425,6 @@
}
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 @@
if (!max_chars)
return result;
- pdfium::span<wchar_t> 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 @@
break;
}
}
+ result.ReleaseBuffer(dest_pos);
} else {
- pdfium::span<wchar_t> 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 6a7d98a..13134f0 100644
--- a/core/fxcrt/cfx_blockbuffer.cpp
+++ b/core/fxcrt/cfx_blockbuffer.cpp
@@ -77,39 +77,36 @@
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<wchar_t> pBuf = wsTextData.GetBuffer(length);
- size_t startBlock = 0;
- size_t startInner = 0;
- std::tie(startBlock, startInner) = TextDataIndex2BufIndex(start);
+ wchar_t* pBuf = wsTextData.GetBuffer(length);
+ if (!pBuf)
+ return WideString();
- size_t endBlock = 0;
- size_t endInner = 0;
- std::tie(endBlock, endInner) = TextDataIndex2BufIndex(start + length);
+ size_t startBlock = 0;
+ size_t startInner = 0;
+ std::tie(startBlock, startInner) = TextDataIndex2BufIndex(start);
- 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);
+ size_t endBlock = 0;
+ size_t endInner = 0;
+ std::tie(endBlock, endInner) = TextDataIndex2BufIndex(start + length);
- wchar_t* pBlockBuf = m_BlockArray[i].get();
- memcpy(&pBuf[pointer], pBlockBuf + bufferPointer,
- copyLength * sizeof(wchar_t));
- pointer += copyLength;
+ 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 a352559..7b5bf66 100644
--- a/core/fxcrt/widestring.cpp
+++ b/core/fxcrt/widestring.cpp
@@ -252,27 +252,22 @@
Optional<WideString> 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<wchar_t> 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 @@
return WideString();
WideString wstr;
- {
- // Span's lifetime must end before ReleaseBuffer() below.
- pdfium::span<wchar_t> 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 @@
GetBuffer(len);
}
-pdfium::span<wchar_t> WideString::GetBuffer(size_t nMinBufLength) {
+wchar_t* WideString::GetBuffer(size_t nMinBufLength) {
if (!m_pData) {
if (nMinBufLength == 0)
- return pdfium::span<wchar_t>();
+ return nullptr;
m_pData.Reset(StringData::Create(nMinBufLength));
m_pData->m_nDataLength = 0;
m_pData->m_String[0] = 0;
- return pdfium::span<wchar_t>(m_pData->m_String, m_pData->m_nAllocLength);
+ return m_pData->m_String;
}
if (m_pData->CanOperateInPlace(nMinBufLength))
- return pdfium::span<wchar_t>(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<wchar_t>();
+ return nullptr;
RetainPtr<StringData> pNewData(StringData::Create(nMinBufLength));
pNewData->CopyContents(*m_pData);
pNewData->m_nDataLength = m_pData->m_nDataLength;
m_pData.Swap(pNewData);
- return pdfium::span<wchar_t>(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 @@
// 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<wchar_t> 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 f6c2437..30a423d 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 {
@@ -163,10 +163,7 @@
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<wchar_t> 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 ad91249..473d59c 100644
--- a/core/fxcrt/widestring_unittest.cpp
+++ b/core/fxcrt/widestring_unittest.cpp
@@ -813,21 +813,20 @@
}
TEST(WideString, GetBuffer) {
- WideString str1;
{
- pdfium::span<wchar_t> 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<wchar_t> 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 46163b5..2ca0838 100644
--- a/fxjs/cfxjse_resolveprocessor.cpp
+++ b/fxjs/cfxjse_resolveprocessor.cpp
@@ -499,69 +499,66 @@
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<wchar_t> pNameBuf = wsName.GetBuffer(iLength - nStart);
- pdfium::span<wchar_t> pConditionBuf =
- wsCondition.GetBuffer(iLength - nStart);
- std::vector<int32_t> 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;
- }
+ std::vector<int32_t> 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;
- }
- 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;
- }
- wPrev = wCur;
+ wchar_t wLookahead = nStart < iLength ? pSrc[nStart] : 0;
+ if (wLookahead != '[' && wLookahead != '(' && nType < 0)
+ break;
}
- if (!stack.empty())
- return -1;
+ 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;
+ }
+ 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<XFA_HashCode>(FX_HashCode_GetW(wsName.AsStringView(), false));
diff --git a/fxjs/xfa/cjx_hostpseudomodel.cpp b/fxjs/xfa/cjx_hostpseudomodel.cpp
index fe26d31..6ca431e 100644
--- a/fxjs/xfa/cjx_hostpseudomodel.cpp
+++ b/fxjs/xfa/cjx_hostpseudomodel.cpp
@@ -29,18 +29,16 @@
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<wchar_t> pBuf = wsFilter.GetBuffer(iLength - nStart);
- const wchar_t* pSrc = wsExpression.unterminated_c_str();
- while (nStart < iLength) {
- wchar_t wCur = pSrc[nStart++];
- if (wCur == ',')
- break;
+ const wchar_t* pSrc = wsExpression.unterminated_c_str();
+ wchar_t wCur;
+ while (nStart < iLength) {
+ wCur = pSrc[nStart++];
+ if (wCur == ',')
+ break;
- pBuf[nCount++] = wCur;
- }
+ pBuf[nCount++] = wCur;
}
wsFilter.ReleaseBuffer(nCount);
wsFilter.Trim();
diff --git a/xfa/fxfa/cxfa_textlayout.cpp b/xfa/fxfa/cxfa_textlayout.cpp
index 5359a34..e360c16 100644
--- a/xfa/fxfa/cxfa_textlayout.cpp
+++ b/xfa/fxfa/cxfa_textlayout.cpp
@@ -911,23 +911,21 @@
if (iLen == 0)
return;
+ wchar_t* psz = wsText.GetBuffer(iLen);
int32_t iTrimLeft = 0;
- {
- // Span's lifetime must end before ReleaseBuffer() below.
- pdfium::span<wchar_t> 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;
+ 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;
- }
+ 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 b129960..8ef67e5 100644
--- a/xfa/fxfa/parser/cxfa_localevalue.cpp
+++ b/xfa/fxfa/parser/cxfa_localevalue.cpp
@@ -682,34 +682,32 @@
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<wchar_t> lpBuf = wsFormat.GetBuffer(nTotalLen);
- int32_t nPos = 0;
- lpBuf[nPos++] = L's';
+ wchar_t* lpBuf = wsFormat.GetBuffer(nTotalLen);
+ int32_t nPos = 0;
+ lpBuf[nPos++] = L's';
- if (nIntLen == -1) {
+ 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--;
- }
+ nIntLen--;
}
- if (nDecLen != 0) {
- lpBuf[nPos++] = L'.';
- }
- 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);
