Add unit tests for FPDF_BSTR.
Then fix the bug uncovered by tests when length == 0.
- Update comments to explain that "Byte String" isn't exactly correct.
- Rename variables to be saner.
Change-Id: I8a232b221dc6a7342bf838a66dfea3cc69d6de2f
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/62911
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/fpdfsdk/BUILD.gn b/fpdfsdk/BUILD.gn
index 73117e1..3c1b39c 100644
--- a/fpdfsdk/BUILD.gn
+++ b/fpdfsdk/BUILD.gn
@@ -111,6 +111,7 @@
"fpdf_doc_unittest.cpp",
"fpdf_edit_unittest.cpp",
"fpdf_editimg_unittest.cpp",
+ "fpdf_view_unittest.cpp",
]
deps = [
":fpdfsdk",
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index a4a70f4..db6f407 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -1074,49 +1074,49 @@
#endif // PDF_ENABLE_V8
#ifdef PDF_ENABLE_XFA
-FPDF_EXPORT FPDF_RESULT FPDF_CALLCONV FPDF_BStr_Init(FPDF_BSTR* str) {
- if (!str)
+FPDF_EXPORT FPDF_RESULT FPDF_CALLCONV FPDF_BStr_Init(FPDF_BSTR* bstr) {
+ if (!bstr)
return -1;
- memset(str, 0, sizeof(FPDF_BSTR));
+ bstr->str = nullptr;
+ bstr->len = 0;
return 0;
}
-FPDF_EXPORT FPDF_RESULT FPDF_CALLCONV FPDF_BStr_Set(FPDF_BSTR* str,
- FPDF_LPCSTR bstr,
+FPDF_EXPORT FPDF_RESULT FPDF_CALLCONV FPDF_BStr_Set(FPDF_BSTR* bstr,
+ FPDF_LPCSTR cstr,
int length) {
- if (!str || !bstr || !length)
+ if (!bstr || !cstr)
return -1;
if (length == -1)
- length = strlen(bstr);
+ length = strlen(cstr);
if (length == 0) {
- FPDF_BStr_Clear(str);
+ FPDF_BStr_Clear(bstr);
return 0;
}
- if (str->str && str->len < length)
- str->str = FX_Realloc(char, str->str, length + 1);
- else if (!str->str)
- str->str = FX_Alloc(char, length + 1);
+ if (bstr->str && bstr->len < length)
+ bstr->str = FX_Realloc(char, bstr->str, length + 1);
+ else if (!bstr->str)
+ bstr->str = FX_Alloc(char, length + 1);
- str->str[length] = 0;
- memcpy(str->str, bstr, length);
- str->len = length;
-
+ bstr->str[length] = 0;
+ memcpy(bstr->str, cstr, length);
+ bstr->len = length;
return 0;
}
-FPDF_EXPORT FPDF_RESULT FPDF_CALLCONV FPDF_BStr_Clear(FPDF_BSTR* str) {
- if (!str)
+FPDF_EXPORT FPDF_RESULT FPDF_CALLCONV FPDF_BStr_Clear(FPDF_BSTR* bstr) {
+ if (!bstr)
return -1;
- if (str->str) {
- FX_Free(str->str);
- str->str = nullptr;
+ if (bstr->str) {
+ FX_Free(bstr->str);
+ bstr->str = nullptr;
}
- str->len = 0;
+ bstr->len = 0;
return 0;
}
#endif // PDF_ENABLE_XFA
diff --git a/fpdfsdk/fpdf_view_unittest.cpp b/fpdfsdk/fpdf_view_unittest.cpp
new file mode 100644
index 0000000..c08c49b
--- /dev/null
+++ b/fpdfsdk/fpdf_view_unittest.cpp
@@ -0,0 +1,56 @@
+// Copyright 2019 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 "public/fpdfview.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+#ifdef PDF_ENABLE_XFA
+TEST(FPDFView, BstrBadArgs) {
+ EXPECT_EQ(-1, FPDF_BStr_Init(nullptr));
+ EXPECT_EQ(-1, FPDF_BStr_Set(nullptr, "clams", -1));
+ EXPECT_EQ(-1, FPDF_BStr_Clear(nullptr));
+}
+
+TEST(FPDFView, BstrEmpty) {
+ FPDF_BSTR bst;
+ EXPECT_EQ(0, FPDF_BStr_Init(&bst));
+ EXPECT_FALSE(bst.str);
+ EXPECT_FALSE(bst.len);
+ EXPECT_EQ(0, FPDF_BStr_Clear(&bst));
+}
+
+TEST(FPDFView, BstrNormal) {
+ FPDF_BSTR bst;
+ EXPECT_EQ(0, FPDF_BStr_Init(&bst));
+ EXPECT_EQ(0, FPDF_BStr_Set(&bst, "clams", -1));
+ EXPECT_STREQ("clams", bst.str);
+ EXPECT_EQ(5, bst.len);
+
+ EXPECT_EQ(0, FPDF_BStr_Clear(&bst));
+ EXPECT_FALSE(bst.str);
+ EXPECT_FALSE(bst.len);
+}
+
+TEST(FPDFView, BstrReassign) {
+ FPDF_BSTR bst;
+ EXPECT_EQ(0, FPDF_BStr_Init(&bst));
+ EXPECT_EQ(0, FPDF_BStr_Set(&bst, "clams", 3));
+ EXPECT_STREQ("cla", bst.str);
+ EXPECT_EQ(3, bst.len);
+
+ EXPECT_EQ(0, FPDF_BStr_Set(&bst, "clams", 5));
+ EXPECT_STREQ("clams", bst.str);
+ EXPECT_EQ(5, bst.len);
+
+ EXPECT_EQ(0, FPDF_BStr_Set(&bst, "clams", 1));
+ EXPECT_STREQ("c", bst.str);
+ EXPECT_EQ(1, bst.len);
+
+ EXPECT_EQ(0, FPDF_BStr_Set(&bst, "clams", 0));
+ EXPECT_FALSE(bst.str);
+ EXPECT_EQ(0, bst.len);
+
+ EXPECT_EQ(0, FPDF_BStr_Clear(&bst));
+}
+#endif // PDF_ENABLE_XFA
diff --git a/public/fpdfview.h b/public/fpdfview.h
index 2b84f07..f7b99f4 100644
--- a/public/fpdfview.h
+++ b/public/fpdfview.h
@@ -107,13 +107,12 @@
typedef const unsigned short* FPDF_WIDESTRING;
#ifdef PDF_ENABLE_XFA
-// Structure for a byte string.
-// Note, a byte string commonly means a UTF-16LE formated string.
+// Structure for persisting a string beyond the duration of a callback.
+// Note: although represented as a char*, string may be interpreted as
+// a UTF-16LE formated string.
typedef struct _FPDF_BSTR {
- // String buffer.
- char* str;
- // Length of the string, in bytes.
- int len;
+ char* str; // String buffer, manipulate only with FPDF_BStr_* methods.
+ int len; // Length of the string, in bytes.
} FPDF_BSTR;
#endif // PDF_ENABLE_XFA
@@ -1193,18 +1192,18 @@
#ifdef PDF_ENABLE_XFA
// Function: FPDF_BStr_Init
-// Helper function to initialize a byte string.
-FPDF_EXPORT FPDF_RESULT FPDF_CALLCONV FPDF_BStr_Init(FPDF_BSTR* str);
+// Helper function to initialize a FPDF_BSTR.
+FPDF_EXPORT FPDF_RESULT FPDF_CALLCONV FPDF_BStr_Init(FPDF_BSTR* bstr);
// Function: FPDF_BStr_Set
-// Helper function to set string data.
-FPDF_EXPORT FPDF_RESULT FPDF_CALLCONV FPDF_BStr_Set(FPDF_BSTR* str,
- FPDF_LPCSTR bstr,
+// Helper function to copy string data into the FPDF_BSTR.
+FPDF_EXPORT FPDF_RESULT FPDF_CALLCONV FPDF_BStr_Set(FPDF_BSTR* bstr,
+ FPDF_LPCSTR cstr,
int length);
// Function: FPDF_BStr_Clear
-// Helper function to clear a byte string.
-FPDF_EXPORT FPDF_RESULT FPDF_CALLCONV FPDF_BStr_Clear(FPDF_BSTR* str);
+// Helper function to clear a FPDF_BSTR.
+FPDF_EXPORT FPDF_RESULT FPDF_CALLCONV FPDF_BStr_Clear(FPDF_BSTR* bstr);
#endif // PDF_ENABLE_XFA
#ifdef __cplusplus
