Mark two-arg WideString() constructor as UNSAFE_BUFFER_USAGE. Follow-on from https://pdfium-review.googlesource.com/c/pdfium/+/119677 but uses the same technique on WideString. Change-Id: I474807734eb40959e598a58fb2cdda4a36370ad4 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/119693 Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: Thomas Sepez <tsepez@google.com> Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfdoc/cpdf_bookmark.cpp b/core/fpdfdoc/cpdf_bookmark.cpp index f4176b9..4b245e5 100644 --- a/core/fpdfdoc/cpdf_bookmark.cpp +++ b/core/fpdfdoc/cpdf_bookmark.cpp
@@ -42,7 +42,7 @@ wchar_t w = title[i]; buf[i] = w > 0x20 ? w : 0x20; } - return WideString(buf.data(), len); + return UNSAFE_TODO(WideString::Create(buf.data(), len)); } CPDF_Dest CPDF_Bookmark::GetDest(CPDF_Document* pDocument) const {
diff --git a/core/fpdftext/cpdf_textpagefind.cpp b/core/fpdftext/cpdf_textpagefind.cpp index 5b77686..4bdbf1e 100644 --- a/core/fpdftext/cpdf_textpagefind.cpp +++ b/core/fpdftext/cpdf_textpagefind.cpp
@@ -12,6 +12,7 @@ #include "core/fpdftext/cpdf_textpage.h" #include "core/fxcrt/check.h" +#include "core/fxcrt/compiler_specific.h" #include "core/fxcrt/fx_extension.h" #include "core/fxcrt/fx_string.h" #include "core/fxcrt/fx_system.h" @@ -114,7 +115,7 @@ return std::nullopt; } - return WideString(lpszFullString, static_cast<size_t>(nLen)); + return WideString::Create(lpszFullString, static_cast<size_t>(nLen)); }); }
diff --git a/core/fxcrt/fx_extension.cpp b/core/fxcrt/fx_extension.cpp index 64dcc11..f4fa200 100644 --- a/core/fxcrt/fx_extension.cpp +++ b/core/fxcrt/fx_extension.cpp
@@ -9,6 +9,7 @@ #include <wchar.h> #include "core/fxcrt/check.h" +#include "core/fxcrt/compiler_specific.h" #include "core/fxcrt/fx_system.h" #include "core/fxcrt/utf16.h" #include "core/fxcrt/widestring.h" @@ -29,7 +30,7 @@ } // namespace float FXSYS_wcstof(const wchar_t* pwsStr, size_t nLength, size_t* pUsedLen) { - WideString copied(pwsStr, nLength); + auto copied = UNSAFE_TODO(WideString::Create(pwsStr, nLength)); wchar_t* endptr = nullptr; float result = wcstof(copied.c_str(), &endptr); if (result != result) {
diff --git a/core/fxcrt/fx_string_unittest.cpp b/core/fxcrt/fx_string_unittest.cpp index 18a6e12..830182f 100644 --- a/core/fxcrt/fx_string_unittest.cpp +++ b/core/fxcrt/fx_string_unittest.cpp
@@ -450,7 +450,7 @@ TEST(fxstring, WideStringSplitEfficiency) { std::vector<wchar_t> commas(50000, L','); - WideString input(commas.data(), commas.size()); + auto input = WideString(WideStringView(commas)); std::vector<WideString> result; result = fxcrt::Split(input, ','); ASSERT_EQ(commas.size() + 1, result.size());
diff --git a/core/fxcrt/widestring.h b/core/fxcrt/widestring.h index 38be8ea..82832cb 100644 --- a/core/fxcrt/widestring.h +++ b/core/fxcrt/widestring.h
@@ -14,6 +14,7 @@ #include <iosfwd> #include <utility> +#include "core/fxcrt/compiler_specific.h" #include "core/fxcrt/span.h" #include "core/fxcrt/string_template.h" @@ -31,6 +32,12 @@ [[nodiscard]] static WideString FormatV(const wchar_t* lpszFormat, va_list argList); + // Remove when UNSAFE_BUFFER_USAGE enforced on ctors. + UNSAFE_BUFFER_USAGE static WideString Create(const wchar_t* pStr, + size_t len) { + return UNSAFE_BUFFERS(WideString(pStr, len)); + } + WideString() = default; WideString(const WideString& other) = default; @@ -50,8 +57,6 @@ // NOLINTNEXTLINE(runtime/explicit) WideString(char) = delete; - WideString(const wchar_t* pStr, size_t len); - explicit WideString(WideStringView str); WideString(WideStringView str1, WideStringView str2); WideString(const std::initializer_list<WideStringView>& list); @@ -129,6 +134,9 @@ WideString EncodeEntities() const; protected: + // Make public UNSAFE_BUFFER_USAGE enforced on ctors. + UNSAFE_BUFFER_USAGE WideString(const wchar_t* pStr, size_t len); + intptr_t ReferenceCountForTesting() const; friend class WideString_Assign_Test;
diff --git a/core/fxcrt/widestring_unittest.cpp b/core/fxcrt/widestring_unittest.cpp index bd696dc..b6b50c2 100644 --- a/core/fxcrt/widestring_unittest.cpp +++ b/core/fxcrt/widestring_unittest.cpp
@@ -1252,7 +1252,7 @@ {"", L""}, {UNSAFE_BUFFERS(ByteString::Create("\0a\0b\0c", 6)), L"abc"}, {UNSAFE_BUFFERS(ByteString::Create("\0a\0b\0c\0\0\0d\0e\0f", 14)), - WideString(L"abc\0def", 7)}, + UNSAFE_BUFFERS(WideString::Create(L"abc\0def", 7))}, {UNSAFE_BUFFERS(ByteString::Create(" &", 2)), L"…"}, {UNSAFE_BUFFERS(ByteString::Create("\xD8\x3C\xDF\xA8", 4)), L"🎨"}, }; @@ -1275,7 +1275,7 @@ {"", L""}, {UNSAFE_BUFFERS(ByteString::Create("a\0b\0c\0", 6)), L"abc"}, {UNSAFE_BUFFERS(ByteString::Create("a\0b\0c\0\0\0d\0e\0f\0", 14)), - WideString(L"abc\0def", 7)}, + UNSAFE_BUFFERS(WideString::Create(L"abc\0def", 7))}, {UNSAFE_BUFFERS(ByteString::Create("& ", 2)), L"…"}, {UNSAFE_BUFFERS(ByteString::Create("\x3C\xD8\xA8\xDF", 4)), L"🎨"}, }; @@ -2016,7 +2016,8 @@ // Writing a WideString with nulls but specifying its length treats it as // a C++-style string. - str = WideString(stringWithNulls, 4); + // SAFETY: known fixed-length string. + str = UNSAFE_BUFFERS(WideString::Create(stringWithNulls, 4)); EXPECT_EQ(4u, str.GetLength()); stream.str(""); stream << str; @@ -2070,7 +2071,7 @@ // Writing a WideString with nulls but specifying its length treats it as // a C++-style string. - str = WideString(stringWithNulls, 4); + str = UNSAFE_BUFFERS(WideString::Create(stringWithNulls, 4)); EXPECT_EQ(4u, str.GetLength()); stream.str(L""); stream << str;
diff --git a/fxbarcode/datamatrix/BC_C40Encoder.cpp b/fxbarcode/datamatrix/BC_C40Encoder.cpp index ade8764..184bc54 100644 --- a/fxbarcode/datamatrix/BC_C40Encoder.cpp +++ b/fxbarcode/datamatrix/BC_C40Encoder.cpp
@@ -25,6 +25,7 @@ #include <iterator> #include "core/fxcrt/check.h" +#include "core/fxcrt/compiler_specific.h" #include "core/fxcrt/fx_extension.h" #include "fxbarcode/common/BC_CommonBitMatrix.h" #include "fxbarcode/datamatrix/BC_Encoder.h" @@ -42,7 +43,7 @@ wchar_t cw[2]; cw[0] = static_cast<wchar_t>(v / 256); cw[1] = static_cast<wchar_t>(v % 256); - return WideString(cw, std::size(cw)); + return UNSAFE_TODO(WideString::Create(cw, std::size(cw))); } } // namespace
diff --git a/fxbarcode/datamatrix/BC_EdifactEncoder.cpp b/fxbarcode/datamatrix/BC_EdifactEncoder.cpp index 1b98f67..b6036a8 100644 --- a/fxbarcode/datamatrix/BC_EdifactEncoder.cpp +++ b/fxbarcode/datamatrix/BC_EdifactEncoder.cpp
@@ -47,7 +47,8 @@ cw[0] = static_cast<wchar_t>((v >> 16) & 255); cw[1] = static_cast<wchar_t>((v >> 8) & 255); cw[2] = static_cast<wchar_t>(v & 255); - return WideString(cw, std::min(len, kBuflen)); + // TODO(tsepez): stop putting binary data in strings. + return UNSAFE_TODO(WideString::Create(cw, std::min(len, kBuflen))); } bool HandleEOD(CBC_EncoderContext* context, const WideString& buffer) {
diff --git a/fxbarcode/pdf417/BC_PDF417HighLevelEncoder_unittest.cpp b/fxbarcode/pdf417/BC_PDF417HighLevelEncoder_unittest.cpp index 8efb1cc..256b169 100644 --- a/fxbarcode/pdf417/BC_PDF417HighLevelEncoder_unittest.cpp +++ b/fxbarcode/pdf417/BC_PDF417HighLevelEncoder_unittest.cpp
@@ -47,7 +47,8 @@ for (size_t i = 0; i < std::size(kEncodeHighLevelCases); ++i) { const EncodeHighLevelCase& testcase = kEncodeHighLevelCases[i]; WideStringView input(testcase.input); - WideString expected(testcase.expected, testcase.expected_length); + auto expected = UNSAFE_TODO( + WideString::Create(testcase.expected, testcase.expected_length)); std::optional<WideString> result = CBC_PDF417HighLevelEncoder::EncodeHighLevel(input); ASSERT_TRUE(result.has_value()); @@ -96,7 +97,8 @@ for (size_t j = 0; j < input_length; ++j) { input_array[j] = testcase.input[j]; } - WideString expected(testcase.expected, testcase.expected_length); + auto expected = UNSAFE_TODO( + WideString::Create(testcase.expected, testcase.expected_length)); WideString result; CBC_PDF417HighLevelEncoder::EncodeBinary(input_array, testcase.offset, testcase.count, testcase.startmode, @@ -158,7 +160,8 @@ for (size_t i = 0; i < std::size(kEncodeNumericCases); ++i) { const EncodeNumericCase& testcase = kEncodeNumericCases[i]; WideString input(testcase.input); - WideString expected(testcase.expected, testcase.expected_length); + auto expected = UNSAFE_TODO( + WideString::Create(testcase.expected, testcase.expected_length)); WideString result; CBC_PDF417HighLevelEncoder::EncodeNumeric(input, testcase.offset, testcase.count, &result);
diff --git a/xfa/fgas/crt/cfgas_stringformatter.cpp b/xfa/fgas/crt/cfgas_stringformatter.cpp index 6585825..2c2771b 100644 --- a/xfa/fgas/crt/cfgas_stringformatter.cpp +++ b/xfa/fgas/crt/cfgas_stringformatter.cpp
@@ -884,13 +884,13 @@ if (spFormatString[index] == '\'') { bQuote = !bQuote; } else if (spFormatString[index] == L'|' && !bQuote) { - UNSAFE_TODO(wsPatterns.emplace_back(spFormatString.data() + token, - index - token)); + wsPatterns.push_back(UNSAFE_TODO( + WideString::Create(spFormatString.data() + token, index - token))); token = index + 1; } } - UNSAFE_TODO( - wsPatterns.emplace_back(spFormatString.data() + token, index - token)); + wsPatterns.push_back(UNSAFE_TODO( + WideString::Create(spFormatString.data() + token, index - token))); return wsPatterns; }