Add a helper to copy a ByteString to a buffer
We have a few repetitive patterns similar to:
if (buffer && len <= buflen)
memcpy(buffer, byteString.c_str(), len);
return len;
Replace them with a new helper, NulTerminateMaybeCopyAndReturnLength().
Add tests for the new helper.
Bug: pdfium:1514
Change-Id: I0d35cc775f6963833524c2eab1847aafa8e29a10
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/68695
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Daniel Hosseinian <dhoss@chromium.org>
diff --git a/fpdfsdk/BUILD.gn b/fpdfsdk/BUILD.gn
index 9eb9562..e0a9983 100644
--- a/fpdfsdk/BUILD.gn
+++ b/fpdfsdk/BUILD.gn
@@ -108,6 +108,7 @@
pdfium_unittest_source_set("unittests") {
sources = [
+ "cpdfsdk_helpers_unittest.cpp",
"fpdf_annot_unittest.cpp",
"fpdf_catalog_unittest.cpp",
"fpdf_doc_unittest.cpp",
diff --git a/fpdfsdk/cpdfsdk_helpers.cpp b/fpdfsdk/cpdfsdk_helpers.cpp
index db11927..2d3a95b 100644
--- a/fpdfsdk/cpdfsdk_helpers.cpp
+++ b/fpdfsdk/cpdfsdk_helpers.cpp
@@ -277,6 +277,15 @@
return {matrix.a, matrix.b, matrix.c, matrix.d, matrix.e, matrix.f};
}
+unsigned long NulTerminateMaybeCopyAndReturnLength(const ByteString& text,
+ void* buffer,
+ unsigned long buflen) {
+ unsigned long len = text.GetLength() + 1;
+ if (buffer && len <= buflen)
+ memcpy(buffer, text.c_str(), len);
+ return len;
+}
+
unsigned long Utf16EncodeMaybeCopyAndReturnLength(const WideString& text,
void* buffer,
unsigned long buflen) {
diff --git a/fpdfsdk/cpdfsdk_helpers.h b/fpdfsdk/cpdfsdk_helpers.h
index e32afc2..74ae582 100644
--- a/fpdfsdk/cpdfsdk_helpers.h
+++ b/fpdfsdk/cpdfsdk_helpers.h
@@ -238,6 +238,10 @@
CFX_Matrix CFXMatrixFromFSMatrix(const FS_MATRIX& matrix);
FS_MATRIX FSMatrixFromCFXMatrix(const CFX_Matrix& matrix);
+unsigned long NulTerminateMaybeCopyAndReturnLength(const ByteString& text,
+ void* buffer,
+ unsigned long buflen);
+
unsigned long Utf16EncodeMaybeCopyAndReturnLength(const WideString& text,
void* buffer,
unsigned long buflen);
diff --git a/fpdfsdk/cpdfsdk_helpers_unittest.cpp b/fpdfsdk/cpdfsdk_helpers_unittest.cpp
new file mode 100644
index 0000000..6b65fa8
--- /dev/null
+++ b/fpdfsdk/cpdfsdk_helpers_unittest.cpp
@@ -0,0 +1,40 @@
+// Copyright 2020 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "fpdfsdk/cpdfsdk_helpers.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(CPDFSDK_HelpersTest, NulTerminateMaybeCopyAndReturnLength) {
+ {
+ const ByteString to_be_copied("toBeCopied");
+ constexpr size_t kExpectedToBeCopiedLen = 10;
+ ASSERT_EQ(kExpectedToBeCopiedLen, to_be_copied.GetLength());
+
+ EXPECT_EQ(kExpectedToBeCopiedLen + 1,
+ NulTerminateMaybeCopyAndReturnLength(to_be_copied, nullptr, 0));
+
+ // Buffer should not change if declared length is too short.
+ char buf[kExpectedToBeCopiedLen + 1];
+ memset(buf, 0x42, kExpectedToBeCopiedLen + 1);
+ ASSERT_EQ(kExpectedToBeCopiedLen + 1,
+ NulTerminateMaybeCopyAndReturnLength(to_be_copied, buf,
+ kExpectedToBeCopiedLen));
+ for (char c : buf)
+ EXPECT_EQ(0x42, c);
+
+ // Buffer should copy over if long enough.
+ ASSERT_EQ(kExpectedToBeCopiedLen + 1,
+ NulTerminateMaybeCopyAndReturnLength(to_be_copied, buf,
+ kExpectedToBeCopiedLen + 1));
+ EXPECT_EQ(to_be_copied, ByteString(buf));
+ }
+ {
+ // Empty ByteString should still copy NUL terminator.
+ const ByteString empty;
+ char buf[1];
+ ASSERT_EQ(1u, NulTerminateMaybeCopyAndReturnLength(empty, buf, 1));
+ EXPECT_EQ(empty, ByteString(buf));
+ }
+}
diff --git a/fpdfsdk/fpdf_doc.cpp b/fpdfsdk/fpdf_doc.cpp
index b0a2324..0a5f593 100644
--- a/fpdfsdk/fpdf_doc.cpp
+++ b/fpdfsdk/fpdf_doc.cpp
@@ -192,10 +192,7 @@
CPDF_Action cAction(CPDFDictionaryFromFPDFAction(action));
ByteString path = cAction.GetFilePath().ToUTF8();
- unsigned long len = path.GetLength() + 1;
- if (buffer && len <= buflen)
- memcpy(buffer, path.c_str(), len);
- return len;
+ return NulTerminateMaybeCopyAndReturnLength(path, buffer, buflen);
}
FPDF_EXPORT unsigned long FPDF_CALLCONV
diff --git a/fpdfsdk/fpdf_editimg.cpp b/fpdfsdk/fpdf_editimg.cpp
index ecd11d1..1e0e560 100644
--- a/fpdfsdk/fpdf_editimg.cpp
+++ b/fpdfsdk/fpdf_editimg.cpp
@@ -292,10 +292,7 @@
else
bsFilter = pFilter->AsArray()->GetStringAt(index);
- unsigned long len = bsFilter.GetLength() + 1;
- if (buffer && len <= buflen)
- memcpy(buffer, bsFilter.c_str(), len);
- return len;
+ return NulTerminateMaybeCopyAndReturnLength(bsFilter, buffer, buflen);
}
FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index b405a00..7266301 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -989,10 +989,7 @@
if (!bsVal)
return 0;
- unsigned long dwStringLen = bsVal->GetLength() + 1;
- if (buffer && length >= dwStringLen)
- memcpy(buffer, bsVal->c_str(), dwStringLen);
- return dwStringLen;
+ return NulTerminateMaybeCopyAndReturnLength(*bsVal, buffer, length);
}
FPDF_EXPORT FPDF_DWORD FPDF_CALLCONV
