From 1dbfe996b946f0d2a1afc52669ff5fca22a85070 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Tue, 17 Apr 2018 17:19:30 +0000 Subject: Re-land "Return pdfium::span 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 Reviewed-by: dsinclair --- core/fpdfapi/page/cpdf_image.cpp | 17 ++++---- core/fpdfapi/parser/fpdf_parser_decode.cpp | 45 +++++++++++---------- core/fpdfapi/parser/fpdf_parser_utility.cpp | 51 +++++++++++++----------- core/fxcrt/bytestring.cpp | 36 ++++++++++------- core/fxcrt/bytestring.h | 6 ++- core/fxcrt/bytestring_unittest.cpp | 21 +++++----- core/fxcrt/widestring.cpp | 21 +++++----- core/fxge/cfx_folderfontinfo.cpp | 14 ++++--- core/fxge/win32/fx_win32_dib.cpp | 62 +++++++++++++++-------------- fpdfsdk/cpdfsdk_formfillenvironment.cpp | 2 +- fxjs/cfxjse_formcalc_context.cpp | 16 +++++--- fxjs/cjs_publicmethods.cpp | 9 +++-- 12 files changed, 171 insertions(+), 129 deletions(-) diff --git a/core/fpdfapi/page/cpdf_image.cpp b/core/fpdfapi/page/cpdf_image.cpp index 68a6a3243c..9ce4268a36 100644 --- a/core/fpdfapi/page/cpdf_image.cpp +++ b/core/fpdfapi/page/cpdf_image.cpp @@ -202,13 +202,16 @@ void CPDF_Image::SetImage(const RetainPtr& pBitmap) { pCS->AddNew("DeviceRGB"); pCS->AddNew(1); ByteString ct; - char* pBuf = ct.GetBuffer(6); - pBuf[0] = (char)reset_r; - pBuf[1] = (char)reset_g; - pBuf[2] = (char)reset_b; - pBuf[3] = (char)set_r; - pBuf[4] = (char)set_g; - pBuf[5] = (char)set_b; + { + // Span's lifetime must end before ReleaseBuffer() below. + pdfium::span pBuf = ct.GetBuffer(6); + pBuf[0] = (char)reset_r; + pBuf[1] = (char)reset_g; + pBuf[2] = (char)reset_b; + pBuf[3] = (char)set_r; + pBuf[4] = (char)set_g; + pBuf[5] = (char)set_b; + } ct.ReleaseBuffer(6); pCS->AddNew(ct, true); } diff --git a/core/fpdfapi/parser/fpdf_parser_decode.cpp b/core/fpdfapi/parser/fpdf_parser_decode.cpp index e879615ddd..edfbaf4331 100644 --- a/core/fpdfapi/parser/fpdf_parser_decode.cpp +++ b/core/fpdfapi/parser/fpdf_parser_decode.cpp @@ -470,20 +470,22 @@ ByteString PDF_EncodeText(const wchar_t* pString, int len) { if (len == -1) len = wcslen(pString); - ByteString result; - char* dest_buf1 = result.GetBuffer(len); int i; - for (i = 0; i < len; ++i) { - int code; - for (code = 0; code < 256; ++code) { - if (PDFDocEncoding[code] == pString[i]) - break; - } + ByteString result; + { + pdfium::span dest_buf = result.GetBuffer(len); + for (i = 0; i < len; ++i) { + int code; + for (code = 0; code < 256; ++code) { + if (PDFDocEncoding[code] == pString[i]) + break; + } - if (code == 256) - break; + if (code == 256) + break; - dest_buf1[i] = code; + dest_buf[i] = code; + } } result.ReleaseBuffer(i); if (i == len) @@ -494,15 +496,18 @@ ByteString PDF_EncodeText(const wchar_t* pString, int len) { return result; } - int encLen = len * 2 + 2; - - uint8_t* dest_buf2 = reinterpret_cast(result.GetBuffer(encLen)); - dest_buf2[0] = 0xfe; - dest_buf2[1] = 0xff; - dest_buf2 += 2; - for (int j = 0; j < len; ++j) { - *dest_buf2++ = pString[j] >> 8; - *dest_buf2++ = static_cast(pString[j]); + size_t dest_index = 0; + size_t encLen = len * 2 + 2; + { + pdfium::span cspan = result.GetBuffer(encLen); + auto dest_buf = pdfium::make_span(reinterpret_cast(cspan.data()), + cspan.size()); + dest_buf[dest_index++] = 0xfe; + dest_buf[dest_index++] = 0xff; + for (int j = 0; j < len; ++j) { + dest_buf[dest_index++] = pString[j] >> 8; + dest_buf[dest_index++] = static_cast(pString[j]); + } } result.ReleaseBuffer(encLen); return result; diff --git a/core/fpdfapi/parser/fpdf_parser_utility.cpp b/core/fpdfapi/parser/fpdf_parser_utility.cpp index c486a76706..2c0b0818f1 100644 --- a/core/fpdfapi/parser/fpdf_parser_utility.cpp +++ b/core/fpdfapi/parser/fpdf_parser_utility.cpp @@ -93,20 +93,23 @@ ByteString PDF_NameDecode(const ByteStringView& bstr) { if (!bstr.Contains('#')) return ByteString(bstr); - int size = bstr.GetLength(); + size_t src_size = bstr.GetLength(); + size_t out_index = 0; ByteString result; - char* pDestStart = result.GetBuffer(size); - char* pDest = pDestStart; - for (int i = 0; i < size; i++) { - if (bstr[i] == '#' && i < size - 2) { - *pDest++ = FXSYS_HexCharToInt(bstr[i + 1]) * 16 + - FXSYS_HexCharToInt(bstr[i + 2]); - i += 2; - } else { - *pDest++ = bstr[i]; + { + // Span's lifetime must end before ReleaseBuffer() below. + pdfium::span pDest = result.GetBuffer(src_size); + for (size_t i = 0; i < src_size; i++) { + if (bstr[i] == '#' && i + 2 < src_size) { + pDest[out_index++] = FXSYS_HexCharToInt(bstr[i + 1]) * 16 + + FXSYS_HexCharToInt(bstr[i + 2]); + i += 2; + } else { + pDest[out_index++] = bstr[i]; + } } } - result.ReleaseBuffer(static_cast(pDest - pDestStart)); + result.ReleaseBuffer(out_index); return result; } @@ -128,21 +131,23 @@ ByteString PDF_NameEncode(const ByteString& orig) { return orig; ByteString res; - char* dest_buf = res.GetBuffer(dest_len); - dest_len = 0; - for (i = 0; i < src_len; i++) { - uint8_t ch = src_buf[i]; - if (ch >= 0x80 || PDFCharIsWhitespace(ch) || ch == '#' || - PDFCharIsDelimiter(ch)) { - dest_buf[dest_len++] = '#'; - FXSYS_IntToTwoHexChars(ch, dest_buf + dest_len); - dest_len += 2; - } else { + { + // Span's lifetime must end before ReleaseBuffer() below. + pdfium::span dest_buf = res.GetBuffer(dest_len); + dest_len = 0; + for (i = 0; i < src_len; i++) { + uint8_t ch = src_buf[i]; + if (ch >= 0x80 || PDFCharIsWhitespace(ch) || ch == '#' || + PDFCharIsDelimiter(ch)) { + dest_buf[dest_len++] = '#'; + FXSYS_IntToTwoHexChars(ch, &dest_buf[dest_len]); + dest_len += 2; + continue; + } dest_buf[dest_len++] = ch; } } - dest_buf[dest_len] = 0; - res.ReleaseBuffer(res.GetStringLength()); + res.ReleaseBuffer(dest_len); return res; } 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; @@ -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 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 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 ByteString::GetBuffer(size_t nMinBufLength) { if (!m_pData) { if (nMinBufLength == 0) - return nullptr; + return pdfium::span(); 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(m_pData->m_String, m_pData->m_nAllocLength); } if (m_pData->CanOperateInPlace(nMinBufLength)) - return m_pData->m_String; + return pdfium::span(m_pData->m_String, m_pData->m_nAllocLength); nMinBufLength = std::max(nMinBufLength, m_pData->m_nDataLength); if (nMinBufLength == 0) - return nullptr; + return pdfium::span(); RetainPtr 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(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 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 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 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 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; } diff --git a/core/fxge/cfx_folderfontinfo.cpp b/core/fxge/cfx_folderfontinfo.cpp index 532824d248..b39c57637f 100644 --- a/core/fxge/cfx_folderfontinfo.cpp +++ b/core/fxge/cfx_folderfontinfo.cpp @@ -44,11 +44,15 @@ const struct { }; ByteString FPDF_ReadStringFromFile(FILE* pFile, uint32_t size) { - ByteString buffer; - if (!fread(buffer.GetBuffer(size), size, 1, pFile)) - return ByteString(); - buffer.ReleaseBuffer(size); - return buffer; + ByteString result; + { + // Span's lifetime must end before ReleaseBuffer() below. + pdfium::span buffer = result.GetBuffer(size); + if (!fread(buffer.data(), size, 1, pFile)) + return ByteString(); + } + result.ReleaseBuffer(size); + return result; } ByteString FPDF_LoadTableFromTT(FILE* pFile, diff --git a/core/fxge/win32/fx_win32_dib.cpp b/core/fxge/win32/fx_win32_dib.cpp index 452d033f64..b6bed7a987 100644 --- a/core/fxge/win32/fx_win32_dib.cpp +++ b/core/fxge/win32/fx_win32_dib.cpp @@ -13,39 +13,43 @@ ByteString CFX_WindowsDIB::GetBitmapInfo( const RetainPtr& pBitmap) { - ByteString result; int len = sizeof(BITMAPINFOHEADER); - if (pBitmap->GetBPP() == 1 || pBitmap->GetBPP() == 8) { + if (pBitmap->GetBPP() == 1 || pBitmap->GetBPP() == 8) len += sizeof(DWORD) * (int)(1 << pBitmap->GetBPP()); - } - BITMAPINFOHEADER* pbmih = (BITMAPINFOHEADER*)result.GetBuffer(len); - memset(pbmih, 0, sizeof(BITMAPINFOHEADER)); - pbmih->biSize = sizeof(BITMAPINFOHEADER); - pbmih->biBitCount = pBitmap->GetBPP(); - pbmih->biCompression = BI_RGB; - pbmih->biHeight = -(int)pBitmap->GetHeight(); - pbmih->biPlanes = 1; - pbmih->biWidth = pBitmap->GetWidth(); - if (pBitmap->GetBPP() == 8) { - uint32_t* pPalette = (uint32_t*)(pbmih + 1); - if (pBitmap->GetPalette()) { - for (int i = 0; i < 256; i++) { - pPalette[i] = pBitmap->GetPalette()[i]; - } - } else { - for (int i = 0; i < 256; i++) { - pPalette[i] = i * 0x010101; + + ByteString result; + { + // Span's lifetime must end before ReleaseBuffer() below. + pdfium::span cspan = result.GetBuffer(len); + BITMAPINFOHEADER* pbmih = reinterpret_cast(cspan.data()); + memset(pbmih, 0, sizeof(BITMAPINFOHEADER)); + pbmih->biSize = sizeof(BITMAPINFOHEADER); + pbmih->biBitCount = pBitmap->GetBPP(); + pbmih->biCompression = BI_RGB; + pbmih->biHeight = -(int)pBitmap->GetHeight(); + pbmih->biPlanes = 1; + pbmih->biWidth = pBitmap->GetWidth(); + if (pBitmap->GetBPP() == 8) { + uint32_t* pPalette = (uint32_t*)(pbmih + 1); + if (pBitmap->GetPalette()) { + for (int i = 0; i < 256; i++) { + pPalette[i] = pBitmap->GetPalette()[i]; + } + } else { + for (int i = 0; i < 256; i++) { + pPalette[i] = i * 0x010101; + } } } - } - if (pBitmap->GetBPP() == 1) { - uint32_t* pPalette = (uint32_t*)(pbmih + 1); - if (pBitmap->GetPalette()) { - pPalette[0] = pBitmap->GetPalette()[0]; - pPalette[1] = pBitmap->GetPalette()[1]; - } else { - pPalette[0] = 0; - pPalette[1] = 0xffffff; + if (pBitmap->GetBPP() == 1) { + uint32_t* pPalette = (uint32_t*)(pbmih + 1); + if (pBitmap->GetPalette()) { + pPalette[0] = pBitmap->GetPalette()[0]; + pPalette[1] = pBitmap->GetPalette()[1]; + } else { + pPalette[0] = 0; + pPalette[1] = 0xffffff; + } } } result.ReleaseBuffer(len); diff --git a/fpdfsdk/cpdfsdk_formfillenvironment.cpp b/fpdfsdk/cpdfsdk_formfillenvironment.cpp index c1846449cd..262a56e2d1 100644 --- a/fpdfsdk/cpdfsdk_formfillenvironment.cpp +++ b/fpdfsdk/cpdfsdk_formfillenvironment.cpp @@ -25,7 +25,7 @@ FPDF_WIDESTRING AsFPDFWideString(ByteString* bsUTF16LE) { // to the embedder. Should the embedder modify it by accident, it won't // corrupt other shares of the string beyond |bsUTF16LE|. return reinterpret_cast( - bsUTF16LE->GetBuffer(bsUTF16LE->GetLength())); + bsUTF16LE->GetBuffer(bsUTF16LE->GetLength()).data()); } CPDFSDK_FormFillEnvironment::CPDFSDK_FormFillEnvironment( diff --git a/fxjs/cfxjse_formcalc_context.cpp b/fxjs/cfxjse_formcalc_context.cpp index 85d0ef8d57..a6d151b468 100644 --- a/fxjs/cfxjse_formcalc_context.cpp +++ b/fxjs/cfxjse_formcalc_context.cpp @@ -509,12 +509,16 @@ ByteString GUIDString(bool bSeparator) { data[6] = (data[6] & 0x0F) | 0x40; ByteString bsStr; - char* pBuf = bsStr.GetBuffer(40); - for (int32_t i = 0; i < 16; ++i, pBuf += 2) { - if (bSeparator && (i == 4 || i == 6 || i == 8 || i == 10)) - *pBuf++ = L'-'; - - FXSYS_IntToTwoHexChars(data[i], pBuf); + { + // Span's lifetime must end before ReleaseBuffer() below. + pdfium::span pBuf = bsStr.GetBuffer(40); + size_t out_index = 0; + for (size_t i = 0; i < 16; ++i, out_index += 2) { + if (bSeparator && (i == 4 || i == 6 || i == 8 || i == 10)) + pBuf[out_index++] = L'-'; + + FXSYS_IntToTwoHexChars(data[i], &pBuf[out_index]); + } } bsStr.ReleaseBuffer(bSeparator ? 36 : 32); return bsStr; diff --git a/fxjs/cjs_publicmethods.cpp b/fxjs/cjs_publicmethods.cpp index 375eb6fd88..8dc69c4a00 100644 --- a/fxjs/cjs_publicmethods.cpp +++ b/fxjs/cjs_publicmethods.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -1094,10 +1095,12 @@ CJS_Return CJS_PublicMethods::AFPercent_Format( if (iDec2 < 0) { ByteString zeros; - char* zeros_ptr = zeros.GetBuffer(abs(iDec2)); - memset(zeros_ptr, '0', abs(iDec2)); + { + pdfium::span zeros_ptr = zeros.GetBuffer(abs(iDec2)); + std::fill(std::begin(zeros_ptr), std::end(zeros_ptr), '0'); + } + zeros.ReleaseBuffer(abs(iDec2)); strValue = zeros + strValue; - iDec2 = 0; } int iMax = strValue.GetLength(); -- cgit v1.2.3