summaryrefslogtreecommitdiff
path: root/core
diff options
context:
space:
mode:
authorRyan Harrison <rharrison@chromium.org>2017-08-02 16:16:18 -0400
committerChromium commit bot <commit-bot@chromium.org>2017-08-02 20:37:10 +0000
commit0811da801dd72e2e0af2d7b9d1e866162df2cee1 (patch)
tree5f73ca36f6d804571e6b44360f2940a2a1509db5 /core
parentb4fee4d5d471475ada1d0d9110e1a534b49477ba (diff)
downloadpdfium-0811da801dd72e2e0af2d7b9d1e866162df2cee1.tar.xz
Remove support for out of bounds params in Delete
The existing implementation of Delete on the string classes handles some cases where the range being deleted is out of bounds by clipping it to the valid range. This behaviour can lead to programming problems in the calling code being masked by the fact the Delete method still does something. The new version of these methods does an early return if the parameters are invalid. This change also effectively removes support for negative string sizes from the Delete method, so converting FX_STRSIZE to be unsigned will be easier. BUG=pdfium:828 Change-Id: Idbb4a62f70a75eba06e7809e011b25da2d7404c4 Reviewed-on: https://pdfium-review.googlesource.com/9890 Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Diffstat (limited to 'core')
-rw-r--r--core/fxcrt/cfx_bytestring.cpp29
-rw-r--r--core/fxcrt/cfx_bytestring_unittest.cpp18
-rw-r--r--core/fxcrt/cfx_widestring.cpp29
-rw-r--r--core/fxcrt/cfx_widestring_unittest.cpp18
4 files changed, 48 insertions, 46 deletions
diff --git a/core/fxcrt/cfx_bytestring.cpp b/core/fxcrt/cfx_bytestring.cpp
index e031c87d2d..6cbd15e693 100644
--- a/core/fxcrt/cfx_bytestring.cpp
+++ b/core/fxcrt/cfx_bytestring.cpp
@@ -411,26 +411,23 @@ char* CFX_ByteString::GetBuffer(FX_STRSIZE nMinBufLength) {
return m_pData->m_String;
}
-FX_STRSIZE CFX_ByteString::Delete(FX_STRSIZE nIndex, FX_STRSIZE nCount) {
+FX_STRSIZE CFX_ByteString::Delete(FX_STRSIZE index, FX_STRSIZE count) {
if (!m_pData)
return 0;
- if (nIndex < 0)
- nIndex = 0;
+ FX_STRSIZE old_length = m_pData->m_nDataLength;
+ if (count <= 0 || index != pdfium::clamp(index, 0, old_length))
+ return old_length;
- FX_STRSIZE nOldLength = m_pData->m_nDataLength;
- if (nCount > 0 && nIndex < nOldLength) {
- FX_STRSIZE mLength = nIndex + nCount;
- if (mLength >= nOldLength) {
- m_pData->m_nDataLength = nIndex;
- return m_pData->m_nDataLength;
- }
- ReallocBeforeWrite(nOldLength);
- int nCharsToCopy = nOldLength - mLength + 1;
- memmove(m_pData->m_String + nIndex, m_pData->m_String + mLength,
- nCharsToCopy);
- m_pData->m_nDataLength = nOldLength - nCount;
- }
+ FX_STRSIZE removal_length = index + count;
+ if (removal_length > old_length)
+ return old_length;
+
+ ReallocBeforeWrite(old_length);
+ int chars_to_copy = old_length - removal_length + 1;
+ memmove(m_pData->m_String + index, m_pData->m_String + removal_length,
+ chars_to_copy);
+ m_pData->m_nDataLength = old_length - count;
return m_pData->m_nDataLength;
}
diff --git a/core/fxcrt/cfx_bytestring_unittest.cpp b/core/fxcrt/cfx_bytestring_unittest.cpp
index 89c8ac6060..d843afef97 100644
--- a/core/fxcrt/cfx_bytestring_unittest.cpp
+++ b/core/fxcrt/cfx_bytestring_unittest.cpp
@@ -454,21 +454,25 @@ TEST(fxcrt, ByteStringInsertAtFrontAndInsertAtBack) {
TEST(fxcrt, ByteStringDelete) {
CFX_ByteString fred("FRED");
- fred.Delete(0, 2);
+ EXPECT_EQ(4, fred.Delete(0, 0));
+ EXPECT_EQ("FRED", fred);
+ EXPECT_EQ(2, fred.Delete(0, 2));
EXPECT_EQ("ED", fred);
- fred.Delete(1);
+ EXPECT_EQ(1, fred.Delete(1));
+ EXPECT_EQ("E", fred);
+ EXPECT_EQ(1, fred.Delete(-1));
EXPECT_EQ("E", fred);
- fred.Delete(-1);
+ EXPECT_EQ(0, fred.Delete(0));
EXPECT_EQ("", fred);
- fred.Delete(1);
+ EXPECT_EQ(0, fred.Delete(0));
EXPECT_EQ("", fred);
CFX_ByteString empty;
- empty.Delete(0);
+ EXPECT_EQ(0, empty.Delete(0));
EXPECT_EQ("", empty);
- empty.Delete(-1);
+ EXPECT_EQ(0, empty.Delete(-1));
EXPECT_EQ("", empty);
- empty.Delete(1);
+ EXPECT_EQ(0, empty.Delete(1));
EXPECT_EQ("", empty);
}
diff --git a/core/fxcrt/cfx_widestring.cpp b/core/fxcrt/cfx_widestring.cpp
index 6c079b354e..a1fa7ec096 100644
--- a/core/fxcrt/cfx_widestring.cpp
+++ b/core/fxcrt/cfx_widestring.cpp
@@ -538,26 +538,23 @@ wchar_t* CFX_WideString::GetBuffer(FX_STRSIZE nMinBufLength) {
return m_pData->m_String;
}
-FX_STRSIZE CFX_WideString::Delete(FX_STRSIZE nIndex, FX_STRSIZE nCount) {
+FX_STRSIZE CFX_WideString::Delete(FX_STRSIZE index, FX_STRSIZE count) {
if (!m_pData)
return 0;
- if (nIndex < 0)
- nIndex = 0;
+ FX_STRSIZE old_length = m_pData->m_nDataLength;
+ if (count <= 0 || index != pdfium::clamp(index, 0, old_length))
+ return old_length;
- FX_STRSIZE nOldLength = m_pData->m_nDataLength;
- if (nCount > 0 && nIndex < nOldLength) {
- FX_STRSIZE mLength = nIndex + nCount;
- if (mLength >= nOldLength) {
- m_pData->m_nDataLength = nIndex;
- return m_pData->m_nDataLength;
- }
- ReallocBeforeWrite(nOldLength);
- int nCharsToCopy = nOldLength - mLength + 1;
- wmemmove(m_pData->m_String + nIndex, m_pData->m_String + mLength,
- nCharsToCopy);
- m_pData->m_nDataLength = nOldLength - nCount;
- }
+ FX_STRSIZE removal_length = index + count;
+ if (removal_length > old_length)
+ return old_length;
+
+ ReallocBeforeWrite(old_length);
+ int chars_to_copy = old_length - removal_length + 1;
+ wmemmove(m_pData->m_String + index, m_pData->m_String + removal_length,
+ chars_to_copy);
+ m_pData->m_nDataLength = old_length - count;
return m_pData->m_nDataLength;
}
diff --git a/core/fxcrt/cfx_widestring_unittest.cpp b/core/fxcrt/cfx_widestring_unittest.cpp
index a763f8abbd..6e7b63c5fa 100644
--- a/core/fxcrt/cfx_widestring_unittest.cpp
+++ b/core/fxcrt/cfx_widestring_unittest.cpp
@@ -414,21 +414,25 @@ TEST(fxcrt, WideStringInsertAtFrontAndInsertAtBack) {
TEST(fxcrt, WideStringDelete) {
CFX_WideString fred(L"FRED");
- fred.Delete(0, 2);
+ EXPECT_EQ(4, fred.Delete(0, 0));
+ EXPECT_EQ(L"FRED", fred);
+ EXPECT_EQ(2, fred.Delete(0, 2));
EXPECT_EQ(L"ED", fred);
- fred.Delete(1);
+ EXPECT_EQ(1, fred.Delete(1));
+ EXPECT_EQ(L"E", fred);
+ EXPECT_EQ(1, fred.Delete(-1));
EXPECT_EQ(L"E", fred);
- fred.Delete(-1);
+ EXPECT_EQ(0, fred.Delete(0));
EXPECT_EQ(L"", fred);
- fred.Delete(1);
+ EXPECT_EQ(0, fred.Delete(0));
EXPECT_EQ(L"", fred);
CFX_WideString empty;
- empty.Delete(0);
+ EXPECT_EQ(0, empty.Delete(0));
EXPECT_EQ(L"", empty);
- empty.Delete(-1);
+ EXPECT_EQ(0, empty.Delete(-1));
EXPECT_EQ(L"", empty);
- empty.Delete(1);
+ EXPECT_EQ(0, empty.Delete(1));
EXPECT_EQ(L"", empty);
}