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>

9 files changed, 191 insertions, 167 deletions

diff --git a/core/fpdfapi/parser/fpdf_parser_decode.cpp b/core/fpdfapi/parser/fpdf_parser_decode.cpp
index d7114b66c5..90dca2edcb 100644
--- a/core/fpdfapi/parser/fpdf_parser_decode.cpp
+++ b/core/fpdfapi/parser/fpdf_parser_decode.cpp
@@ -425,6 +425,7 @@ 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))) {
@@ -432,17 +433,15 @@ WideString PDF_DecodeText(const uint8_t* src_data, uint32_t src_len) {
     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);
@@ -451,13 +450,13 @@ WideString PDF_DecodeText(const uint8_t* src_data, uint32_t src_len) {
           break;
       }
     }
-    result.ReleaseBuffer(dest_pos);
   } else {
-    wchar_t* dest_buf = result.GetBuffer(src_len);
+    pdfium::span<wchar_t> dest_buf = result.GetBuffer(src_len);
     for (uint32_t i = 0; i < src_len; ++i)
       dest_buf[i] = PDFDocEncoding[src_data[i]];
-    result.ReleaseBuffer(src_len);
+    dest_pos = src_len;
   }
+  result.ReleaseBuffer(dest_pos);
   return result;
 }
 
diff --git a/core/fxcrt/cfx_blockbuffer.cpp b/core/fxcrt/cfx_blockbuffer.cpp
index 13134f0e7b..6a7d98aa18 100644
--- a/core/fxcrt/cfx_blockbuffer.cpp
+++ b/core/fxcrt/cfx_blockbuffer.cpp
@@ -77,36 +77,39 @@ 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);
 
-  WideString wsTextData;
-  wchar_t* pBuf = wsTextData.GetBuffer(length);
-  if (!pBuf)
+  length = std::min(length, maybeDataLength);
+  if (!length)
     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;
+  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);
+
+    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;
     }
-    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 7b5bf66fd3..a3525593ee 100644
--- a/core/fxcrt/widestring.cpp
+++ b/core/fxcrt/widestring.cpp
@@ -252,22 +252,27 @@ Optional<size_t> GuessSizeForVSWPrintf(const wchar_t* pFormat,
 Optional<WideString> TryVSWPrintf(size_t size,
                                   const wchar_t* pFormat,
                                   va_list argList) {
-  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)
+  if (!size)
     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};
 }
@@ -299,9 +304,12 @@ WideString GetWideString(uint16_t codepage, const ByteStringView& bstr) {
     return WideString();
 
   WideString wstr;
-  wchar_t* dest_buf = wstr.GetBuffer(dest_len);
-  FXSYS_MultiByteToWideChar(codepage, 0, bstr.unterminated_c_str(), src_len,
-                            dest_buf, dest_len);
+  {
+    // 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);
+  }
   wstr.ReleaseBuffer(dest_len);
   return wstr;
 }
@@ -586,29 +594,29 @@ void WideString::Reserve(size_t len) {
   GetBuffer(len);
 }
 
-wchar_t* WideString::GetBuffer(size_t nMinBufLength) {
+pdfium::span<wchar_t> WideString::GetBuffer(size_t nMinBufLength) {
   if (!m_pData) {
     if (nMinBufLength == 0)
-      return nullptr;
+      return pdfium::span<wchar_t>();
 
     m_pData.Reset(StringData::Create(nMinBufLength));
     m_pData->m_nDataLength = 0;
     m_pData->m_String[0] = 0;
-    return m_pData->m_String;
+    return pdfium::span<wchar_t>(m_pData->m_String, m_pData->m_nAllocLength);
   }
 
   if (m_pData->CanOperateInPlace(nMinBufLength))
-    return m_pData->m_String;
+    return pdfium::span<wchar_t>(m_pData->m_String, m_pData->m_nAllocLength);
 
   nMinBufLength = std::max(nMinBufLength, m_pData->m_nDataLength);
   if (nMinBufLength == 0)
-    return nullptr;
+    return pdfium::span<wchar_t>();
 
   RetainPtr<StringData> pNewData(StringData::Create(nMinBufLength));
   pNewData->CopyContents(*m_pData);
   pNewData->m_nDataLength = m_pData->m_nDataLength;
   m_pData.Swap(pNewData);
-  return m_pData->m_String;
+  return pdfium::span<wchar_t>(m_pData->m_String, m_pData->m_nAllocLength);
 }
 
 size_t WideString::Delete(size_t index, size_t count) {
@@ -885,14 +893,15 @@ 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;
-  wchar_t* buf = result.GetBuffer(wlen);
-  for (size_t i = 0; i < wlen; i++) {
-    buf[i] = wstr[i];
+  {
+    // 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];
   }
   result.ReleaseBuffer(wlen);
   return result;
diff --git a/core/fxcrt/widestring.h b/core/fxcrt/widestring.h
index 30a423d9aa..f6c24375a0 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,7 +163,10 @@ class WideString {
   void TrimRight(const WideStringView& targets);
 
   void Reserve(size_t len);
-  wchar_t* GetBuffer(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);
   void ReleaseBuffer(size_t len);
 
   int GetInteger() const;
diff --git a/core/fxcrt/widestring_unittest.cpp b/core/fxcrt/widestring_unittest.cpp
index 473d59c491..ad91249c8e 100644
--- a/core/fxcrt/widestring_unittest.cpp
+++ b/core/fxcrt/widestring_unittest.cpp
@@ -813,20 +813,21 @@ TEST(WideString, Reserve) {
 }
 
 TEST(WideString, GetBuffer) {
+  WideString str1;
   {
-    WideString str;
-    wchar_t* buffer = str.GetBuffer(12);
-    wcscpy(buffer, L"clams");
-    str.ReleaseBuffer(str.GetStringLength());
-    EXPECT_EQ(L"clams", str);
+    pdfium::span<wchar_t> buffer = str1.GetBuffer(12);
+    wcscpy(buffer.data(), L"clams");
   }
+  str1.ReleaseBuffer(str1.GetStringLength());
+  EXPECT_EQ(L"clams", str1);
+
+  WideString str2(L"cl");
   {
-    WideString str(L"cl");
-    wchar_t* buffer = str.GetBuffer(12);
-    wcscpy(buffer + 2, L"ams");
-    str.ReleaseBuffer(str.GetStringLength());
-    EXPECT_EQ(L"clams", str);
+    pdfium::span<wchar_t> buffer = str2.GetBuffer(12);
+    wcscpy(buffer.data() + 2, L"ams");
   }
+  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 2ca0838a7f..46163b55fc 100644
--- a/fxjs/cfxjse_resolveprocessor.cpp
+++ b/fxjs/cfxjse_resolveprocessor.cpp
@@ -499,66 +499,69 @@ 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;
-  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;
+  {
+    // 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;
+        }
+
+        wchar_t wLookahead = nStart < iLength ? pSrc[nStart] : 0;
+        if (wLookahead != '[' && wLookahead != '(' && nType < 0)
+          break;
       }
-      if (nNameCount == 0) {
-        rnd.m_dwStyles |= XFA_RESOLVENODE_AnyChild;
-        continue;
+      if (wCur == '[' || wCur == '(') {
+        bIsCondition = true;
+      } else if (wCur == '.' && nStart < iLength &&
+                 (pSrc[nStart] == '[' || pSrc[nStart] == '(')) {
+        bIsCondition = true;
       }
-
-      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 (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;
     }
-    wPrev = wCur;
+    if (!stack.empty())
+      return -1;
   }
-  if (!stack.empty())
-    return -1;
-
   wsName.ReleaseBuffer(nNameCount);
-  wsName.Trim();
   wsCondition.ReleaseBuffer(nConditionCount);
+  wsName.Trim();
   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 6ca431e78d..fe26d3161d 100644
--- a/fxjs/xfa/cjx_hostpseudomodel.cpp
+++ b/fxjs/xfa/cjx_hostpseudomodel.cpp
@@ -29,16 +29,18 @@ int32_t FilterName(const WideStringView& wsExpression,
   if (nStart >= iLength)
     return iLength;
 
-  wchar_t* pBuf = wsFilter.GetBuffer(iLength - nStart);
   int32_t nCount = 0;
-  const wchar_t* pSrc = wsExpression.unterminated_c_str();
-  wchar_t wCur;
-  while (nStart < iLength) {
-    wCur = pSrc[nStart++];
-    if (wCur == ',')
-      break;
-
-    pBuf[nCount++] = wCur;
+  {
+    // 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;
+
+      pBuf[nCount++] = wCur;
+    }
   }
   wsFilter.ReleaseBuffer(nCount);
   wsFilter.Trim();
diff --git a/xfa/fxfa/cxfa_textlayout.cpp b/xfa/fxfa/cxfa_textlayout.cpp
index e360c16dde..5359a340e5 100644
--- a/xfa/fxfa/cxfa_textlayout.cpp
+++ b/xfa/fxfa/cxfa_textlayout.cpp
@@ -911,21 +911,23 @@ void CXFA_TextLayout::ProcessText(WideString& wsText) {
   if (iLen == 0)
     return;
 
-  wchar_t* psz = wsText.GetBuffer(iLen);
   int32_t iTrimLeft = 0;
-  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;
+  {
+    // 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;
+
+      wPrev = wch;
+      psz[iTrimLeft++] = wch;
+    }
   }
-  wsText.ReleaseBuffer(iLen);
-  wsText = wsText.Left(iTrimLeft);
+  wsText.ReleaseBuffer(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 8ef67e5183..b129960f7d 100644
--- a/xfa/fxfa/parser/cxfa_localevalue.cpp
+++ b/xfa/fxfa/parser/cxfa_localevalue.cpp
@@ -682,32 +682,34 @@ 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);
-  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) {
+  {
+    // Span's lifetime must end before ReleaseBuffer() below.
+    pdfium::span<wchar_t> lpBuf = wsFormat.GetBuffer(nTotalLen);
+    int32_t nPos = 0;
+    lpBuf[nPos++] = L's';
+
+    if (nIntLen == -1) {
       lpBuf[nPos++] = L'z';
-      nIntLen--;
+      lpBuf[nPos++] = L'*';
+    } else {
+      while (nIntLen) {
+        lpBuf[nPos++] = L'z';
+        nIntLen--;
+      }
     }
-  }
-  if (nDecLen != 0) {
-    lpBuf[nPos++] = L'.';
-  }
-  if (nDecLen == -1) {
-    lpBuf[nPos++] = L'z';
-    lpBuf[nPos++] = L'*';
-  } else {
-    while (nDecLen) {
+    if (nDecLen != 0) {
+      lpBuf[nPos++] = L'.';
+    }
+    if (nDecLen == -1) {
       lpBuf[nPos++] = L'z';
-      nDecLen--;
+      lpBuf[nPos++] = L'*';
+    } else {
+      while (nDecLen) {
+        lpBuf[nPos++] = L'z';
+        nDecLen--;
+      }
     }
   }
   wsFormat.ReleaseBuffer(nTotalLen);