Introduce UNSAFE_TODO() macro.
Improve readability by avoiding distracting boilerplate comments.
-- tidy some other comments while at it.
Bug: pdfium:2154
Change-Id: Idaf43bc85bc17cb2738faa7b3deda1fd66ac67e7
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/119211
Reviewed-by: Thomas Sepez <tsepez@google.com>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/fpdfsdk/cpdfsdk_appstream.cpp b/fpdfsdk/cpdfsdk_appstream.cpp
index 57d36c3..55cc243 100644
--- a/fpdfsdk/cpdfsdk_appstream.cpp
+++ b/fpdfsdk/cpdfsdk_appstream.cpp
@@ -206,17 +206,17 @@
for (size_t i = 0; i < std::size(pts); ++i) {
for (size_t j = 0; j < std::size(pts[0]); ++j) {
- // TODO(crbug.com/pdfium/2155): resolve safety issues.
- UNSAFE_BUFFERS(pts[i][j].x = pts[i][j].x * fWidth + crBBox.left);
- UNSAFE_BUFFERS(pts[i][j].y *= pts[i][j].y * fHeight + crBBox.bottom);
+ UNSAFE_TODO({
+ pts[i][j].x = pts[i][j].x * fWidth + crBBox.left;
+ pts[i][j].y *= pts[i][j].y * fHeight + crBBox.bottom;
+ });
}
}
fxcrt::ostringstream csAP;
WriteMove(csAP, pts[0][0]);
- // TODO(crbug.com/pdfium/2155): resolve safety issues.
- UNSAFE_BUFFERS({
+ UNSAFE_TODO({
for (size_t i = 0; i < std::size(pts); ++i) {
size_t nNext = i < std::size(pts) - 1 ? i + 1 : 0;
const CFX_PointF& pt_next = pts[nNext][0];
@@ -334,8 +334,7 @@
int next = 0;
for (size_t i = 0; i < std::size(points); ++i) {
next = (next + 2) % std::size(points);
- // TODO(crbug.com/pdfium/2155): resolve safety issues.
- WriteLine(csAP, UNSAFE_BUFFERS(points[next]));
+ WriteLine(csAP, UNSAFE_TODO(points[next]));
}
return ByteString(csAP);
diff --git a/fpdfsdk/cpdfsdk_helpers_unittest.cpp b/fpdfsdk/cpdfsdk_helpers_unittest.cpp
index db5c85a..85fbb0a 100644
--- a/fpdfsdk/cpdfsdk_helpers_unittest.cpp
+++ b/fpdfsdk/cpdfsdk_helpers_unittest.cpp
@@ -26,8 +26,8 @@
fxcrt::spanset(pdfium::make_span(buf), 0x42);
ASSERT_EQ(kExpectedToBeCopiedLen + 1,
NulTerminateMaybeCopyAndReturnLength(
- to_be_copied, UNSAFE_BUFFERS(pdfium::make_span(
- buf, kExpectedToBeCopiedLen))));
+ to_be_copied,
+ UNSAFE_TODO(pdfium::make_span(buf, kExpectedToBeCopiedLen))));
for (char c : buf)
EXPECT_EQ(0x42, c);
diff --git a/fpdfsdk/fpdf_annot.cpp b/fpdfsdk/fpdf_annot.cpp
index 14e708d..637f9c7 100644
--- a/fpdfsdk/fpdf_annot.cpp
+++ b/fpdfsdk/fpdf_annot.cpp
@@ -519,9 +519,8 @@
auto ink_coord_list = inklist->AppendNew<CPDF_Array>();
for (size_t i = 0; i < point_count; i++) {
- // TODO(crbug.com/pdfium/2155): investigate safety issues.
- ink_coord_list->AppendNew<CPDF_Number>(UNSAFE_BUFFERS(points[i].x));
- ink_coord_list->AppendNew<CPDF_Number>(UNSAFE_BUFFERS(points[i].y));
+ ink_coord_list->AppendNew<CPDF_Number>(UNSAFE_TODO(points[i].x));
+ ink_coord_list->AppendNew<CPDF_Number>(UNSAFE_TODO(points[i].y));
}
return static_cast<int>(inklist->size() - 1);
}
@@ -878,9 +877,8 @@
fxcrt::CollectionSize<unsigned long>(*vertices) / 2;
if (buffer && length >= points_len) {
for (unsigned long i = 0; i < points_len; ++i) {
- // TODO(crbug.com/pdfium/2155): investigate safety issues.
- UNSAFE_BUFFERS(buffer[i].x) = vertices->GetFloatAt(i * 2);
- UNSAFE_BUFFERS(buffer[i].y) = vertices->GetFloatAt(i * 2 + 1);
+ UNSAFE_TODO(buffer[i].x) = vertices->GetFloatAt(i * 2);
+ UNSAFE_TODO(buffer[i].y) = vertices->GetFloatAt(i * 2 + 1);
}
}
return points_len;
@@ -910,9 +908,8 @@
fxcrt::CollectionSize<unsigned long>(*path) / 2;
if (buffer && length >= points_len) {
for (unsigned long i = 0; i < points_len; ++i) {
- // TODO(crbug.com/pdfium/2155): investigate safety issues.
- UNSAFE_BUFFERS(buffer[i].x) = path->GetFloatAt(i * 2);
- UNSAFE_BUFFERS(buffer[i].y) = path->GetFloatAt(i * 2 + 1);
+ UNSAFE_TODO(buffer[i].x) = path->GetFloatAt(i * 2);
+ UNSAFE_TODO(buffer[i].y) = path->GetFloatAt(i * 2 + 1);
}
}
return points_len;
@@ -1069,8 +1066,7 @@
"length of kModeKeyForMode should be equal to "
"FPDF_ANNOT_APPEARANCEMODE_COUNT");
- // TODO(crbug.com/pdfium/2155): investigate safety issues.
- const char* mode_key = UNSAFE_BUFFERS(kModeKeyForMode[appearanceMode]);
+ const char* mode_key = UNSAFE_TODO(kModeKeyForMode[appearanceMode]);
RetainPtr<CPDF_Dictionary> pApDict =
pAnnotDict->GetMutableDictFor(pdfium::annotation::kAP);
@@ -1391,9 +1387,8 @@
std::vector<CPDF_Annot::Subtype> focusable_annot_types;
focusable_annot_types.reserve(count);
for (size_t i = 0; i < count; ++i) {
- // TODO(crbug.com/pdfium/2155): investigate safety issues.
focusable_annot_types.push_back(
- static_cast<CPDF_Annot::Subtype>(UNSAFE_BUFFERS(subtypes[i])));
+ static_cast<CPDF_Annot::Subtype>(UNSAFE_TODO(subtypes[i])));
}
pFormFillEnv->SetFocusableAnnotSubtypes(focusable_annot_types);
@@ -1431,9 +1426,8 @@
return false;
for (size_t i = 0; i < focusable_annot_types.size(); ++i) {
- // TODO(crbug.com/pdfium/2155): investigate safety issues.
- UNSAFE_BUFFERS(subtypes[i] = static_cast<FPDF_ANNOTATION_SUBTYPE>(
- focusable_annot_types[i]));
+ UNSAFE_TODO(subtypes[i] = static_cast<FPDF_ANNOTATION_SUBTYPE>(
+ focusable_annot_types[i]));
}
return true;
@@ -1483,7 +1477,7 @@
// SAFETY: required from caller.
return Utf16EncodeMaybeCopyAndReturnLength(
pWidget->GetExportValue(),
- UNSAFE_BUFFERS(SpanFromFPDFApiArgs(buffer, buflen)));
+ UNSAFE_TODO(SpanFromFPDFApiArgs(buffer, buflen)));
}
FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFAnnot_SetURI(FPDF_ANNOTATION annot,
diff --git a/fpdfsdk/fpdf_attachment.cpp b/fpdfsdk/fpdf_attachment.cpp
index 6fc9671..85ba9fb 100644
--- a/fpdfsdk/fpdf_attachment.cpp
+++ b/fpdfsdk/fpdf_attachment.cpp
@@ -51,9 +51,7 @@
char buf[32];
for (int i = 0; i < 16; ++i) {
- // TODO(crbug.com/pdfium/2155): resolve safety issues.
- FXSYS_IntToTwoHexChars(UNSAFE_BUFFERS(digest[i]),
- UNSAFE_BUFFERS(&buf[i * 2]));
+ FXSYS_IntToTwoHexChars(UNSAFE_TODO(digest[i]), UNSAFE_TODO(&buf[i * 2]));
}
return ByteString(buf, 32);
}
@@ -262,10 +260,9 @@
// Create the file stream and have the filespec dictionary link to it.
const uint8_t* contents_as_bytes = static_cast<const uint8_t*>(contents);
- // TODO(crbug.com/pdfium/2155): resolve safety issues.
auto pFileStream = pDoc->NewIndirect<CPDF_Stream>(
DataVector<uint8_t>(contents_as_bytes,
- UNSAFE_BUFFERS(contents_as_bytes + len)),
+ UNSAFE_TODO(contents_as_bytes + len)),
std::move(pFileStreamDict));
auto pEFDict = pFile->AsMutableDictionary()->SetNewFor<CPDF_Dictionary>("EF");
diff --git a/fpdfsdk/fpdf_doc.cpp b/fpdfsdk/fpdf_doc.cpp
index 570fa31..b16b9b7 100644
--- a/fpdfsdk/fpdf_doc.cpp
+++ b/fpdfsdk/fpdf_doc.cpp
@@ -273,8 +273,7 @@
DCHECK(nParams <= 4);
*pNumParams = nParams;
for (unsigned long i = 0; i < nParams; ++i) {
- // TODO(crbug.com/pdfium/2155): resolve safety issues.
- UNSAFE_BUFFERS(pParams[i] = destination.GetParam(i));
+ UNSAFE_TODO(pParams[i]) = destination.GetParam(i);
}
return destination.GetZoomMode();
}
diff --git a/fpdfsdk/fpdf_edit_embeddertest.cpp b/fpdfsdk/fpdf_edit_embeddertest.cpp
index 2a3b524..d63b948 100644
--- a/fpdfsdk/fpdf_edit_embeddertest.cpp
+++ b/fpdfsdk/fpdf_edit_embeddertest.cpp
@@ -4054,8 +4054,7 @@
// - blob "BlobKey": "\x01\x02\x03\0BlobValue1\0\0\0BlobValue2\0"
constexpr size_t kBlobLen = 28;
char block_value[kBlobLen];
- // TODO(tsepez): investigate safety.
- UNSAFE_BUFFERS(FXSYS_memcpy(
+ UNSAFE_TODO(FXSYS_memcpy(
block_value, "\x01\x02\x03\0BlobValue1\0\0\0BlobValue2\0", kBlobLen));
EXPECT_EQ(0, FPDFPageObjMark_CountParams(mark));
EXPECT_TRUE(
diff --git a/fpdfsdk/fpdf_editimg.cpp b/fpdfsdk/fpdf_editimg.cpp
index ab75757..479c6eb 100644
--- a/fpdfsdk/fpdf_editimg.cpp
+++ b/fpdfsdk/fpdf_editimg.cpp
@@ -93,8 +93,7 @@
if (pages) {
for (int index = 0; index < count; index++) {
- // TODO(crbug.com/pdfium/2155): resolve safety issues.
- CPDF_Page* pPage = CPDFPageFromFPDFPage(UNSAFE_BUFFERS(pages[index]));
+ CPDF_Page* pPage = CPDFPageFromFPDFPage(UNSAFE_TODO(pages[index]));
if (pPage) {
pImgObj->GetImage()->ResetCache(pPage);
}
@@ -175,8 +174,7 @@
if (pages) {
for (int index = 0; index < count; index++) {
- // TODO(crbug.com/pdfium/2155): resolve safety issues.
- CPDF_Page* pPage = CPDFPageFromFPDFPage(UNSAFE_BUFFERS(pages[index]));
+ CPDF_Page* pPage = CPDFPageFromFPDFPage(UNSAFE_TODO(pages[index]));
if (pPage) {
pImgObj->GetImage()->ResetCache(pPage);
}
diff --git a/fpdfsdk/fpdf_editpage.cpp b/fpdfsdk/fpdf_editpage.cpp
index 7fa4fe1..e6ff79a 100644
--- a/fpdfsdk/fpdf_editpage.cpp
+++ b/fpdfsdk/fpdf_editpage.cpp
@@ -1013,8 +1013,7 @@
std::vector<float> dashes;
if (dash_count > 0) {
dashes.reserve(dash_count);
- // TODO(crbug.com/pdfium/2155): resolve safety issues.
- dashes.assign(dash_array, UNSAFE_BUFFERS(dash_array + dash_count));
+ dashes.assign(dash_array, UNSAFE_TODO(dash_array + dash_count));
}
pPageObj->mutable_graph_state().SetLineDash(dashes, phase, 1.0f);
pPageObj->SetDirty(true);
diff --git a/fpdfsdk/fpdf_edittext.cpp b/fpdfsdk/fpdf_edittext.cpp
index f9d16ea..fe52b92 100644
--- a/fpdfsdk/fpdf_edittext.cpp
+++ b/fpdfsdk/fpdf_edittext.cpp
@@ -618,8 +618,7 @@
ByteString byte_text;
if (charcodes) {
for (size_t i = 0; i < count; ++i) {
- // TODO(crbug.com/pdfium/2155): investigate safety issues.
- pTextObj->GetFont()->AppendChar(&byte_text, UNSAFE_BUFFERS(charcodes[i]));
+ pTextObj->GetFont()->AppendChar(&byte_text, UNSAFE_TODO(charcodes[i]));
}
}
pTextObj->SetText(byte_text);
diff --git a/fpdfsdk/fpdf_javascript_embeddertest.cpp b/fpdfsdk/fpdf_javascript_embeddertest.cpp
index c5fa0d8..8810754 100644
--- a/fpdfsdk/fpdf_javascript_embeddertest.cpp
+++ b/fpdfsdk/fpdf_javascript_embeddertest.cpp
@@ -82,8 +82,7 @@
// The result buffer should be overwritten with an empty string.
std::vector<FPDF_WCHAR> buf = GetFPDFWideStringBuffer(kExpectedLength);
// Write in the buffer to verify it's not overwritten.
- // TODO(tsepez): investigate safety.
- UNSAFE_BUFFERS(FXSYS_memcpy(buf.data(), "abcdefgh", 8));
+ UNSAFE_TODO(FXSYS_memcpy(buf.data(), "abcdefgh", 8));
EXPECT_EQ(kExpectedLength, FPDFJavaScriptAction_GetName(js.get(), buf.data(),
kExpectedLength - 1));
EXPECT_EQ(0, memcmp(buf.data(), "abcdefgh", 8));
@@ -113,8 +112,7 @@
// small. The result buffer should be overwritten with an empty string.
std::vector<FPDF_WCHAR> buf = GetFPDFWideStringBuffer(kExpectedLength);
// Write in the buffer to verify it's not overwritten.
- // TODO(tsepez): investigate safety.
- UNSAFE_BUFFERS(FXSYS_memcpy(buf.data(), "abcdefgh", 8));
+ UNSAFE_TODO(FXSYS_memcpy(buf.data(), "abcdefgh", 8));
EXPECT_EQ(kExpectedLength, FPDFJavaScriptAction_GetScript(
js.get(), buf.data(), kExpectedLength - 1));
EXPECT_EQ(0, memcmp(buf.data(), "abcdefgh", 8));
diff --git a/fpdfsdk/fpdf_ppo.cpp b/fpdfsdk/fpdf_ppo.cpp
index a4faaa4..266d021 100644
--- a/fpdfsdk/fpdf_ppo.cpp
+++ b/fpdfsdk/fpdf_ppo.cpp
@@ -739,8 +739,7 @@
if (length == 0) {
return false;
}
- // TODO(crbug.com/pdfium/2155): investigate safety issues.
- auto page_span = UNSAFE_BUFFERS(pdfium::make_span(
+ auto page_span = UNSAFE_TODO(pdfium::make_span(
reinterpret_cast<const uint32_t*>(page_indices), length));
return exporter.ExportPage(page_span, index);
}
diff --git a/fpdfsdk/fpdf_signature.cpp b/fpdfsdk/fpdf_signature.cpp
index 5061f42..48ecf06 100644
--- a/fpdfsdk/fpdf_signature.cpp
+++ b/fpdfsdk/fpdf_signature.cpp
@@ -114,8 +114,7 @@
fxcrt::CollectionSize<unsigned long>(*byte_range);
if (buffer && length >= byte_range_len) {
for (size_t i = 0; i < byte_range_len; ++i) {
- // TODO(crbug.com/pdfium/2155): resolve safety issue.
- UNSAFE_BUFFERS(buffer[i] = byte_range->GetIntegerAt(i));
+ UNSAFE_TODO(buffer[i]) = byte_range->GetIntegerAt(i);
}
}
return byte_range_len;
