Re-land "Return pdfium::span<char> from ByteString::GetBuffer().""

This reverts commit 3d523e3cf89440e2ffc6571b1c687ad5e3f0318f.
Fixes bounding errors now caught by tests.

Change-Id: I4d0f1791bdcc45a10615a62abf7a4d20e7e538f2
Reviewed-on: https://pdfium-review.googlesource.com/30799
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>

4 files changed, 49 insertions, 35 deletions

diff --git a/core/fxcrt/bytestring.cpp b/core/fxcrt/bytestring.cpp
index f5687c591e..872de065ba 100644
--- a/core/fxcrt/bytestring.cpp
+++ b/core/fxcrt/bytestring.cpp
@@ -18,6 +18,7 @@
 #include "core/fxcrt/fx_safe_types.h"
 #include "core/fxcrt/string_pool_template.h"
 #include "third_party/base/numerics/safe_math.h"
+#include "third_party/base/span.h"
 #include "third_party/base/stl_util.h"
 
 template class fxcrt::StringDataTemplate<char>;
@@ -81,9 +82,12 @@ ByteString GetByteString(uint16_t codepage, const WideStringView& wstr) {
     return ByteString();
 
   ByteString bstr;
-  char* dest_buf = bstr.GetBuffer(dest_len);
-  FXSYS_WideCharToMultiByte(codepage, 0, wstr.unterminated_c_str(), src_len,
-                            dest_buf, dest_len, nullptr, nullptr);
+  {
+    // Span's lifetime must end before ReleaseBuffer() below.
+    pdfium::span<char> dest_buf = bstr.GetBuffer(dest_len);
+    FXSYS_WideCharToMultiByte(codepage, 0, wstr.unterminated_c_str(), src_len,
+                              dest_buf.data(), dest_len, nullptr, nullptr);
+  }
   bstr.ReleaseBuffer(dest_len);
   return bstr;
 }
@@ -120,19 +124,21 @@ ByteString ByteString::FormatV(const char* pFormat, va_list argList) {
   va_end(argListCopy);
 
   if (nMaxLen <= 0)
-    return "";
+    return ByteString();
 
   ByteString ret;
-  char* buf = ret.GetBuffer(nMaxLen);
-  if (buf) {
+  {
+    // Span's lifetime must end before ReleaseBuffer() below.
+    pdfium::span<char> buf = ret.GetBuffer(nMaxLen);
+
     // In the following two calls, there's always space in the buffer for
     // a terminating NUL that's not included in nMaxLen.
-    memset(buf, 0, nMaxLen + 1);
+    memset(buf.data(), 0, nMaxLen + 1);
     va_copy(argListCopy, argList);
-    vsnprintf(buf, nMaxLen + 1, pFormat, argListCopy);
+    vsnprintf(buf.data(), nMaxLen + 1, pFormat, argListCopy);
     va_end(argListCopy);
-    ret.ReleaseBuffer(ret.GetStringLength());
   }
+  ret.ReleaseBuffer(ret.GetStringLength());
   return ret;
 }
 
@@ -419,29 +425,29 @@ void ByteString::Reserve(size_t len) {
   GetBuffer(len);
 }
 
-char* ByteString::GetBuffer(size_t nMinBufLength) {
+pdfium::span<char> ByteString::GetBuffer(size_t nMinBufLength) {
   if (!m_pData) {
     if (nMinBufLength == 0)
-      return nullptr;
+      return pdfium::span<char>();
 
     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<char>(m_pData->m_String, m_pData->m_nAllocLength);
   }
 
   if (m_pData->CanOperateInPlace(nMinBufLength))
-    return m_pData->m_String;
+    return pdfium::span<char>(m_pData->m_String, m_pData->m_nAllocLength);
 
   nMinBufLength = std::max(nMinBufLength, m_pData->m_nDataLength);
   if (nMinBufLength == 0)
-    return nullptr;
+    return pdfium::span<char>();
 
   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<char>(m_pData->m_String, m_pData->m_nAllocLength);
 }
 
 size_t ByteString::Delete(size_t index, size_t count) {
diff --git a/core/fxcrt/bytestring.h b/core/fxcrt/bytestring.h
index ccb539b2ef..247238573d 100644
--- a/core/fxcrt/bytestring.h
+++ b/core/fxcrt/bytestring.h
@@ -17,6 +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 {
 
@@ -153,7 +154,10 @@ class ByteString {
   size_t Delete(size_t index, size_t count = 1);
 
   void Reserve(size_t len);
-  char* GetBuffer(size_t len);
+
+  // Note: any modification of the string (including ReleaseBuffer()) may
+  // invalidate the span, which must not outlive its buffer.
+  pdfium::span<char> GetBuffer(size_t len);
   void ReleaseBuffer(size_t len);
 
   ByteString Mid(size_t first, size_t count) const;
diff --git a/core/fxcrt/bytestring_unittest.cpp b/core/fxcrt/bytestring_unittest.cpp
index f7e1559af9..d030535edd 100644
--- a/core/fxcrt/bytestring_unittest.cpp
+++ b/core/fxcrt/bytestring_unittest.cpp
@@ -843,22 +843,23 @@ TEST(ByteString, Reserve) {
 }
 
 TEST(ByteString, GetBuffer) {
+  ByteString str1;
   {
-    ByteString str;
-    char* buffer = str.GetBuffer(12);
+    pdfium::span<char> buffer = str1.GetBuffer(12);
     // NOLINTNEXTLINE(runtime/printf)
-    strcpy(buffer, "clams");
-    str.ReleaseBuffer(str.GetStringLength());
-    EXPECT_EQ("clams", str);
+    strcpy(buffer.data(), "clams");
   }
+  str1.ReleaseBuffer(str1.GetStringLength());
+  EXPECT_EQ("clams", str1);
+
+  ByteString str2("cl");
   {
-    ByteString str("cl");
-    char* buffer = str.GetBuffer(12);
+    pdfium::span<char> buffer = str2.GetBuffer(12);
     // NOLINTNEXTLINE(runtime/printf)
-    strcpy(buffer + 2, "ams");
-    str.ReleaseBuffer(str.GetStringLength());
-    EXPECT_EQ("clams", str);
+    strcpy(&buffer[2], "ams");
   }
+  str2.ReleaseBuffer(str2.GetStringLength());
+  EXPECT_EQ("clams", str2);
 }
 
 TEST(ByteString, ReleaseBuffer) {
diff --git a/core/fxcrt/widestring.cpp b/core/fxcrt/widestring.cpp
index a3525593ee..25f253ea11 100644
--- a/core/fxcrt/widestring.cpp
+++ b/core/fxcrt/widestring.cpp
@@ -667,18 +667,21 @@ ByteString WideString::UTF8Encode() const {
 }
 
 ByteString WideString::UTF16LE_Encode() const {
-  if (!m_pData) {
+  if (!m_pData)
     return ByteString("\0\0", 2);
-  }
-  int len = m_pData->m_nDataLength;
+
   ByteString result;
-  char* buffer = result.GetBuffer(len * 2 + 2);
-  for (int i = 0; i < len; i++) {
-    buffer[i * 2] = m_pData->m_String[i] & 0xff;
-    buffer[i * 2 + 1] = m_pData->m_String[i] >> 8;
+  int len = m_pData->m_nDataLength;
+  {
+    // Span's lifetime must end before ReleaseBuffer() below.
+    pdfium::span<char> buffer = result.GetBuffer(len * 2 + 2);
+    for (int i = 0; i < len; i++) {
+      buffer[i * 2] = m_pData->m_String[i] & 0xff;
+      buffer[i * 2 + 1] = m_pData->m_String[i] >> 8;
+    }
+    buffer[len * 2] = 0;
+    buffer[len * 2 + 1] = 0;
   }
-  buffer[len * 2] = 0;
-  buffer[len * 2 + 1] = 0;
   result.ReleaseBuffer(len * 2 + 2);
   return result;
 }