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