[pdf] Fix PDFiumFormFiller::Form_GetFilePath()

Its return value should account for the trailing null.

Change-Id: I255078c1007c967b6e648362d11eae868d23c501
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3219441
Commit-Queue: Daniel Hosseinian <dhoss@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#930867}

pdf/pdfium/pdfium_form_filler.cc

@@ -650,9 +650,12 @@ int PDFiumFormFiller::Form_GetFilePath(IPDF_JSPLATFORM* param,
   EngineInIsolateScope engine_scope = GetEngineInIsolateScope(param);
   PDFiumEngine* engine = engine_scope.engine();
   std::string rv = engine->client_->GetURL();
-  if (file_path && rv.size() <= static_cast<size_t>(length))
+
-    memcpy(file_path, rv.c_str(), rv.size());
+  // Account for the trailing null.
-  return rv.size();
+  int necessary_length = rv.size() + 1;
+  if (file_path && necessary_length <= length)
+    memcpy(file_path, rv.c_str(), necessary_length);
+  return necessary_length;
 }
 
 // static

pdf/pdfium/pdfium_form_filler_unittest.cc

@@ -28,7 +28,6 @@ namespace {
 
 using ::testing::Contains;
 using ::testing::InSequence;
-using ::testing::Not;
 using ::testing::Return;
 
 class FormFillerTestClient : public TestClient {
@@ -234,16 +233,13 @@ TEST_F(FormFillerJavaScriptTest, GetFilePath) {
   EXPECT_CALL(client, GetURL).Times(2).WillRepeatedly(Return(kTestPath));
   PDFiumEngine engine(&client, PDFiumFormFiller::ScriptOption::kJavaScript);
 
-  // TODO(dhoss): The return value should be `kTestPathSize`.
   EXPECT_EQ(TriggerGetFilePath(engine, /*file_path=*/nullptr, /*length=*/0),
-            kTestPathSize - 1);
+            kTestPathSize);
 
   std::vector<char> buffer(kTestPathSize, 'X');
   EXPECT_EQ(TriggerGetFilePath(engine, buffer.data(), buffer.size()),
-            kTestPathSize - 1);
+            kTestPathSize);
-
+  EXPECT_STREQ(buffer.data(), kTestPath);
-  // TODO(dhoss): `buffer.data()` should be null terminated.
-  EXPECT_STRNE(buffer.data(), kTestPath);
 }
 
 TEST_F(FormFillerJavaScriptTest, GetFilePathEmpty) {
@@ -251,15 +247,13 @@ TEST_F(FormFillerJavaScriptTest, GetFilePathEmpty) {
   EXPECT_CALL(client, GetURL).Times(2).WillRepeatedly(Return(std::string()));
   PDFiumEngine engine(&client, PDFiumFormFiller::ScriptOption::kJavaScript);
 
-  // TODO(dhoss): The return value should be 1.
+  EXPECT_EQ(TriggerGetFilePath(engine, /*file_path=*/nullptr, /*length=*/0), 1);
-  EXPECT_EQ(TriggerGetFilePath(engine, /*file_path=*/nullptr, /*length=*/0), 0);
 
   char buffer[] = "buffer";
-  EXPECT_EQ(TriggerGetFilePath(engine, buffer, /*length=*/1), 0);
+  EXPECT_EQ(TriggerGetFilePath(engine, buffer, /*length=*/1), 1);
 
-  // TODO(dhoss): `buffer` should be "" (i.e., its first character should be a
+  // The trailing null should be copied over.
-  // null terminator).
+  EXPECT_STREQ(buffer, "");
-  EXPECT_STRNE(buffer, "");
 }
 
 TEST_F(FormFillerJavaScriptTest, GetFilePathShortBuffer) {
@@ -271,14 +265,12 @@ TEST_F(FormFillerJavaScriptTest, GetFilePathShortBuffer) {
   PDFiumEngine engine(&client, PDFiumFormFiller::ScriptOption::kJavaScript);
 
   std::vector<char> buffer(kTestPathSize - 1, 'X');
-
-  // TODO(dhoss): The return value should be `kTestPathSize`.
   EXPECT_EQ(TriggerGetFilePath(engine, buffer.data(), buffer.size()),
-            kTestPathSize - 1);
+            kTestPathSize);
 
-  // TODO(dhoss): Nothing should be copied over. The buffer size is too small to
+  // Nothing should be copied over. The buffer size is too small to contain a
-  // contain a trailing null.
+  // trailing null.
-  EXPECT_THAT(buffer, Not(Contains('X').Times(buffer.size())));
+  EXPECT_THAT(buffer, Contains('X').Times(buffer.size()));
 }
 #endif  // defined(PDF_ENABLE_V8)