From f83005fccd7177d896b0ce0135f36314cef9cafa Mon Sep 17 00:00:00 2001 From: Daniel Hosseinian Date: Wed, 13 Oct 2021 00:10:39 +0000 Subject: [PATCH] [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 Reviewed-by: Lei Zhang Cr-Commit-Position: refs/heads/main@{#930867} --- pdf/pdfium/pdfium_form_filler.cc | 9 ++++--- pdf/pdfium/pdfium_form_filler_unittest.cc | 30 +++++++++-------------- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/pdf/pdfium/pdfium_form_filler.cc b/pdf/pdfium/pdfium_form_filler.cc index 229cdf85e314c6..5855bc609247a1 100644 --- a/pdf/pdfium/pdfium_form_filler.cc +++ b/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(length)) - memcpy(file_path, rv.c_str(), rv.size()); - return rv.size(); + + // Account for the trailing null. + int necessary_length = rv.size() + 1; + if (file_path && necessary_length <= length) + memcpy(file_path, rv.c_str(), necessary_length); + return necessary_length; } // static diff --git a/pdf/pdfium/pdfium_form_filler_unittest.cc b/pdf/pdfium/pdfium_form_filler_unittest.cc index c8f0df3cec4a82..5d0e5e2c15cf6a 100644 --- a/pdf/pdfium/pdfium_form_filler_unittest.cc +++ b/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 buffer(kTestPathSize, 'X'); EXPECT_EQ(TriggerGetFilePath(engine, buffer.data(), buffer.size()), - kTestPathSize - 1); - - // TODO(dhoss): `buffer.data()` should be null terminated. - EXPECT_STRNE(buffer.data(), kTestPath); + kTestPathSize); + EXPECT_STREQ(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), 0); + EXPECT_EQ(TriggerGetFilePath(engine, /*file_path=*/nullptr, /*length=*/0), 1); 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 - // null terminator). - EXPECT_STRNE(buffer, ""); + // The trailing null should be copied over. + EXPECT_STREQ(buffer, ""); } TEST_F(FormFillerJavaScriptTest, GetFilePathShortBuffer) { @@ -271,14 +265,12 @@ TEST_F(FormFillerJavaScriptTest, GetFilePathShortBuffer) { PDFiumEngine engine(&client, PDFiumFormFiller::ScriptOption::kJavaScript); std::vector 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 - // contain a trailing null. - EXPECT_THAT(buffer, Not(Contains('X').Times(buffer.size()))); + // Nothing should be copied over. The buffer size is too small to contain a + // trailing null. + EXPECT_THAT(buffer, Contains('X').Times(buffer.size())); } #endif // defined(PDF_ENABLE_V8)