Changed CPDF_FileSpec::GetFileName() to directly return CFX_WideString

Changed CPDF_FileSpec::GetFileName() to directly return CFX_WideString,
instead of taking in a CFX_WideString* and returning a bool. Also fixed
test calls in the unit test.

Bug=pdfium:808

Change-Id: Ie081fc9f4f1c15d23c86d7222d8480ab9cb56056
Reviewed-on: https://pdfium-review.googlesource.com/7671
Commit-Queue: Jane Liu <janeliulwq@google.com>
Commit-Queue: dsinclair <dsinclair@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

5 files changed, 43 insertions, 67 deletions

diff --git a/core/fpdfdoc/cpdf_action.cpp b/core/fpdfdoc/cpdf_action.cpp
index b40fdcd225..88d0781e80 100644
--- a/core/fpdfdoc/cpdf_action.cpp
+++ b/core/fpdfdoc/cpdf_action.cpp
@@ -73,21 +73,16 @@ CFX_WideString CPDF_Action::GetFilePath() const {
   }
 
   CPDF_Object* pFile = m_pDict->GetDirectObjectFor("F");
-  CFX_WideString path;
-  if (!pFile) {
-    if (type == "Launch") {
-      CPDF_Dictionary* pWinDict = m_pDict->GetDictFor("Win");
-      if (pWinDict) {
-        return CFX_WideString::FromLocal(
-            pWinDict->GetStringFor("F").AsStringC());
-      }
+  if (pFile)
+    return CPDF_FileSpec(pFile).GetFileName();
+
+  if (type == "Launch") {
+    CPDF_Dictionary* pWinDict = m_pDict->GetDictFor("Win");
+    if (pWinDict) {
+      return CFX_WideString::FromLocal(pWinDict->GetStringFor("F").AsStringC());
     }
-    return path;
   }
-
-  CPDF_FileSpec filespec(pFile);
-  filespec.GetFileName(&path);
-  return path;
+  return CFX_WideString();
 }
 
 CFX_ByteString CPDF_Action::GetURI(CPDF_Document* pDoc) const {
diff --git a/core/fpdfdoc/cpdf_filespec.cpp b/core/fpdfdoc/cpdf_filespec.cpp
index 7d1f0e68b9..5ef6f466c5 100644
--- a/core/fpdfdoc/cpdf_filespec.cpp
+++ b/core/fpdfdoc/cpdf_filespec.cpp
@@ -89,36 +89,31 @@ CFX_WideString CPDF_FileSpec::DecodeFileName(const CFX_WideString& filepath) {
 #endif
 }
 
-bool CPDF_FileSpec::GetFileName(CFX_WideString* csFileName) const {
+CFX_WideString CPDF_FileSpec::GetFileName() const {
+  CFX_WideString csFileName;
   if (CPDF_Dictionary* pDict = m_pObj->AsDictionary()) {
-    *csFileName = pDict->GetUnicodeTextFor("UF");
-    if (csFileName->IsEmpty()) {
-      *csFileName =
+    csFileName = pDict->GetUnicodeTextFor("UF");
+    if (csFileName.IsEmpty()) {
+      csFileName =
           CFX_WideString::FromLocal(pDict->GetStringFor("F").AsStringC());
     }
     if (pDict->GetStringFor("FS") == "URL")
-      return true;
-    if (csFileName->IsEmpty()) {
-      if (pDict->KeyExist("DOS")) {
-        *csFileName =
-            CFX_WideString::FromLocal(pDict->GetStringFor("DOS").AsStringC());
-      } else if (pDict->KeyExist("Mac")) {
-        *csFileName =
-            CFX_WideString::FromLocal(pDict->GetStringFor("Mac").AsStringC());
-      } else if (pDict->KeyExist("Unix")) {
-        *csFileName =
-            CFX_WideString::FromLocal(pDict->GetStringFor("Unix").AsStringC());
-      } else {
-        return false;
+      return csFileName;
+
+    if (csFileName.IsEmpty()) {
+      constexpr const char* keys[] = {"DOS", "Mac", "Unix"};
+      for (const auto* key : keys) {
+        if (pDict->KeyExist(key)) {
+          csFileName =
+              CFX_WideString::FromLocal(pDict->GetStringFor(key).AsStringC());
+          break;
+        }
       }
     }
   } else if (m_pObj->IsString()) {
-    *csFileName = CFX_WideString::FromLocal(m_pObj->GetString().AsStringC());
-  } else {
-    return false;
+    csFileName = CFX_WideString::FromLocal(m_pObj->GetString().AsStringC());
   }
-  *csFileName = DecodeFileName(*csFileName);
-  return true;
+  return DecodeFileName(csFileName);
 }
 
 CPDF_Stream* CPDF_FileSpec::GetFileStream() const {
diff --git a/core/fpdfdoc/cpdf_filespec.h b/core/fpdfdoc/cpdf_filespec.h
index 2cef20b48a..8640b2e4c6 100644
--- a/core/fpdfdoc/cpdf_filespec.h
+++ b/core/fpdfdoc/cpdf_filespec.h
@@ -28,7 +28,7 @@ class CPDF_FileSpec {
   static CFX_WideString DecodeFileName(const CFX_WideString& filepath);
 
   CPDF_Object* GetObj() const { return m_pObj.Get(); }
-  bool GetFileName(CFX_WideString* wsFileName) const;
+  CFX_WideString GetFileName() const;
   CPDF_Stream* GetFileStream() const;
   CPDF_Dictionary* GetParamsDict() const;
 
diff --git a/core/fpdfdoc/cpdf_filespec_unittest.cpp b/core/fpdfdoc/cpdf_filespec_unittest.cpp
index 7e2975c5f0..8d151c9dd8 100644
--- a/core/fpdfdoc/cpdf_filespec_unittest.cpp
+++ b/core/fpdfdoc/cpdf_filespec_unittest.cpp
@@ -48,11 +48,11 @@ TEST(cpdf_filespec, EncodeDecodeFileName) {
 #endif
   };
   for (const auto& data : test_data) {
-    CFX_WideString encoded_str = CPDF_FileSpec::EncodeFileName(data.input);
-    EXPECT_TRUE(encoded_str == data.expected);
+    EXPECT_STREQ(data.expected,
+                 CPDF_FileSpec::EncodeFileName(data.input).c_str());
     // DecodeFileName is the reverse procedure of EncodeFileName.
-    CFX_WideString decoded_str = CPDF_FileSpec::DecodeFileName(data.expected);
-    EXPECT_TRUE(decoded_str == data.input);
+    EXPECT_STREQ(data.input,
+                 CPDF_FileSpec::DecodeFileName(data.expected).c_str());
   }
 }
 
@@ -73,9 +73,7 @@ TEST(cpdf_filespec, GetFileName) {
     };
     auto str_obj = pdfium::MakeUnique<CPDF_String>(nullptr, test_data.input);
     CPDF_FileSpec file_spec(str_obj.get());
-    CFX_WideString file_name;
-    EXPECT_TRUE(file_spec.GetFileName(&file_name));
-    EXPECT_TRUE(file_name == test_data.expected);
+    EXPECT_STREQ(test_data.expected, file_spec.GetFileName().c_str());
   }
   {
     // Dictionary object.
@@ -104,25 +102,22 @@ TEST(cpdf_filespec, GetFileName) {
     const char* const keywords[5] = {"Unix", "Mac", "DOS", "F", "UF"};
     auto dict_obj = pdfium::MakeUnique<CPDF_Dictionary>();
     CPDF_FileSpec file_spec(dict_obj.get());
-    CFX_WideString file_name;
+    EXPECT_TRUE(file_spec.GetFileName().IsEmpty());
     for (int i = 0; i < 5; ++i) {
       dict_obj->SetNewFor<CPDF_String>(keywords[i], test_data[i].input);
-      EXPECT_TRUE(file_spec.GetFileName(&file_name));
-      EXPECT_TRUE(file_name == test_data[i].expected);
+      EXPECT_STREQ(test_data[i].expected, file_spec.GetFileName().c_str());
     }
 
     // With all the former fields and 'FS' field suggests 'URL' type.
     dict_obj->SetNewFor<CPDF_String>("FS", "URL", false);
-    EXPECT_TRUE(file_spec.GetFileName(&file_name));
     // Url string is not decoded.
-    EXPECT_TRUE(file_name == test_data[4].input);
+    EXPECT_STREQ(test_data[4].input, file_spec.GetFileName().c_str());
   }
   {
     // Invalid object.
     auto name_obj = pdfium::MakeUnique<CPDF_Name>(nullptr, "test.pdf");
     CPDF_FileSpec file_spec(name_obj.get());
-    CFX_WideString file_name;
-    EXPECT_FALSE(file_spec.GetFileName(&file_name));
+    EXPECT_TRUE(file_spec.GetFileName().IsEmpty());
   }
 }
 
@@ -144,25 +139,19 @@ TEST(cpdf_filespec, SetFileName) {
   CPDF_FileSpec file_spec1(str_obj.get());
   file_spec1.SetFileName(test_data.input);
   // Check internal object value.
-  CFX_ByteString str = CFX_ByteString::FromUnicode(test_data.expected);
-  EXPECT_TRUE(str == str_obj->GetString());
+  EXPECT_STREQ(test_data.expected, str_obj->GetUnicodeText().c_str());
   // Check we can get the file name back.
-  CFX_WideString file_name;
-  EXPECT_TRUE(file_spec1.GetFileName(&file_name));
-  EXPECT_TRUE(file_name == test_data.input);
+  EXPECT_STREQ(test_data.input, file_spec1.GetFileName().c_str());
 
   // Dictionary object.
   auto dict_obj = pdfium::MakeUnique<CPDF_Dictionary>();
   CPDF_FileSpec file_spec2(dict_obj.get());
   file_spec2.SetFileName(test_data.input);
   // Check internal object value.
-  file_name = dict_obj->GetUnicodeTextFor("F");
-  EXPECT_TRUE(file_name == test_data.expected);
-  file_name = dict_obj->GetUnicodeTextFor("UF");
-  EXPECT_TRUE(file_name == test_data.expected);
+  EXPECT_STREQ(test_data.expected, dict_obj->GetUnicodeTextFor("F").c_str());
+  EXPECT_STREQ(test_data.expected, dict_obj->GetUnicodeTextFor("UF").c_str());
   // Check we can get the file name back.
-  EXPECT_TRUE(file_spec2.GetFileName(&file_name));
-  EXPECT_TRUE(file_name == test_data.input);
+  EXPECT_STREQ(test_data.input, file_spec2.GetFileName().c_str());
 }
 
 TEST(cpdf_filespec, GetFileStream) {
diff --git a/fpdfsdk/fpdfattachment.cpp b/fpdfsdk/fpdfattachment.cpp
index d8713bb2e1..e07d15b0c8 100644
--- a/fpdfsdk/fpdfattachment.cpp
+++ b/fpdfsdk/fpdfattachment.cpp
@@ -35,13 +35,10 @@ FPDFDoc_GetAttachmentName(FPDF_DOCUMENT document,
   if (!pFile)
     return 0;
 
-  CFX_WideString name;
-  CPDF_FileSpec filespec(pFile);
-  filespec.GetFileName(&name);
-  CFX_ByteString encodedName = name.UTF16LE_Encode();
-  unsigned long len = encodedName.GetLength();
+  CFX_ByteString name = CPDF_FileSpec(pFile).GetFileName().UTF16LE_Encode();
+  unsigned long len = name.GetLength();
   if (buffer && buflen >= len)
-    memcpy(buffer, encodedName.c_str(), len);
+    memcpy(buffer, name.c_str(), len);
 
   return len;
 }