From 65b3655fc3f858180122399654bf29dc5d11a4ba Mon Sep 17 00:00:00 2001 From: Wei Li Date: Thu, 18 Feb 2016 14:04:57 -0800 Subject: Clean up CPDF_FileSpec and add unit tests Change implicit conversion to accessor function, remove unused function and unused parameter, add a couple checks for null pointers, and some cleanup. BUG=pdfium:247 R=thestig@chromium.org Review URL: https://codereview.chromium.org/1703183002 . --- core/include/fpdfdoc/fpdf_doc.h | 16 +-- core/src/fpdfdoc/doc_action.cpp | 2 +- core/src/fpdfdoc/doc_basic.cpp | 108 ++++++++------------ core/src/fpdfdoc/doc_basic_unittest.cpp | 168 ++++++++++++++++++++++++++++++++ core/src/fpdfdoc/doc_form.cpp | 4 +- 5 files changed, 221 insertions(+), 77 deletions(-) create mode 100644 core/src/fpdfdoc/doc_basic_unittest.cpp (limited to 'core') diff --git a/core/include/fpdfdoc/fpdf_doc.h b/core/include/fpdfdoc/fpdf_doc.h index 17f2c824c6..15616be4d5 100644 --- a/core/include/fpdfdoc/fpdf_doc.h +++ b/core/include/fpdfdoc/fpdf_doc.h @@ -317,21 +317,23 @@ class CPDF_DocJSActions { protected: CPDF_Document* const m_pDocument; }; + class CPDF_FileSpec { public: CPDF_FileSpec(); - explicit CPDF_FileSpec(CPDF_Object* pObj) { m_pObj = pObj; } - operator CPDF_Object*() const { return m_pObj; } - - FX_BOOL IsURL() const; + // Convert a platform dependent file name into pdf format. + static CFX_WideString EncodeFileName(const CFX_WideStringC& filepath); - FX_BOOL GetFileName(CFX_WideString& wsFileName) const; + // Convert a pdf file name into platform dependent format. + static CFX_WideString DecodeFileName(const CFX_WideStringC& filepath); - CPDF_Stream* GetFileStream() const; + CPDF_Object* GetObj() const { return m_pObj; } + bool GetFileName(CFX_WideString* wsFileName) const; - void SetFileName(const CFX_WideStringC& wsFileName, FX_BOOL bURL = FALSE); + // Set this file spec to refer to a file name (not a url). + void SetFileName(const CFX_WideStringC& wsFileName); protected: CPDF_Object* m_pObj; diff --git a/core/src/fpdfdoc/doc_action.cpp b/core/src/fpdfdoc/doc_action.cpp index 97ca046bb9..94235d2aee 100644 --- a/core/src/fpdfdoc/doc_action.cpp +++ b/core/src/fpdfdoc/doc_action.cpp @@ -66,7 +66,7 @@ CFX_WideString CPDF_Action::GetFilePath() const { return path; } CPDF_FileSpec filespec(pFile); - filespec.GetFileName(path); + filespec.GetFileName(&path); return path; } CFX_ByteString CPDF_Action::GetURI(CPDF_Document* pDoc) const { diff --git a/core/src/fpdfdoc/doc_basic.cpp b/core/src/fpdfdoc/doc_basic.cpp index 85a6c0f7c8..bc88ddc828 100644 --- a/core/src/fpdfdoc/doc_basic.cpp +++ b/core/src/fpdfdoc/doc_basic.cpp @@ -237,6 +237,7 @@ CPDF_Array* CPDF_NameTree::LookupNamedDest(CPDF_Document* pDoc, return pDict->GetArrayBy("D"); return nullptr; } + #if _FXM_PLATFORM_ == _FXM_PLATFORM_APPLE_ || \ _FXM_PLATFORM_ == _FXM_PLATFORM_WINDOWS_ static CFX_WideString ChangeSlashToPlatform(const FX_WCHAR* str) { @@ -255,6 +256,7 @@ static CFX_WideString ChangeSlashToPlatform(const FX_WCHAR* str) { } return result; } + static CFX_WideString ChangeSlashToPDF(const FX_WCHAR* str) { CFX_WideString result; while (*str) { @@ -267,23 +269,21 @@ static CFX_WideString ChangeSlashToPDF(const FX_WCHAR* str) { } return result; } -#endif -static CFX_WideString FILESPEC_DecodeFileName(const CFX_WideStringC& filepath) { - if (filepath.GetLength() <= 1) { +#endif // _FXM_PLATFORM_APPLE_ || _FXM_PLATFORM_WINDOWS_ + +CFX_WideString CPDF_FileSpec::DecodeFileName(const CFX_WideStringC& filepath) { + if (filepath.GetLength() <= 1) return CFX_WideString(); - } + #if _FXM_PLATFORM_ == _FXM_PLATFORM_APPLE_ - if (filepath.Left(sizeof("/Mac") - 1) == CFX_WideStringC(L"/Mac")) { + if (filepath.Left(sizeof("/Mac") - 1) == CFX_WideStringC(L"/Mac")) return ChangeSlashToPlatform(filepath.GetPtr() + 1); - } return ChangeSlashToPlatform(filepath.GetPtr()); #elif _FXM_PLATFORM_ == _FXM_PLATFORM_WINDOWS_ - if (filepath.GetAt(0) != '/') { + if (filepath.GetAt(0) != '/') return ChangeSlashToPlatform(filepath.GetPtr()); - } - if (filepath.GetAt(1) == '/') { + if (filepath.GetAt(1) == '/') return ChangeSlashToPlatform(filepath.GetPtr() + 1); - } if (filepath.GetAt(2) == '/') { CFX_WideString result; result += filepath.GetAt(1); @@ -299,48 +299,41 @@ static CFX_WideString FILESPEC_DecodeFileName(const CFX_WideStringC& filepath) { return filepath; #endif } -FX_BOOL CPDF_FileSpec::GetFileName(CFX_WideString& csFileName) const { - if (!m_pObj) { - return FALSE; - } + +bool CPDF_FileSpec::GetFileName(CFX_WideString* csFileName) const { if (CPDF_Dictionary* pDict = m_pObj->AsDictionary()) { - csFileName = pDict->GetUnicodeTextBy("UF"); - if (csFileName.IsEmpty()) { - csFileName = CFX_WideString::FromLocal(pDict->GetStringBy("F")); + *csFileName = pDict->GetUnicodeTextBy("UF"); + if (csFileName->IsEmpty()) { + *csFileName = CFX_WideString::FromLocal(pDict->GetStringBy("F")); } - if (pDict->GetStringBy("FS") == "URL") { - return TRUE; - } - if (csFileName.IsEmpty()) { + if (pDict->GetStringBy("FS") == "URL") + return true; + if (csFileName->IsEmpty()) { if (pDict->KeyExist("DOS")) { - csFileName = CFX_WideString::FromLocal(pDict->GetStringBy("DOS")); + *csFileName = CFX_WideString::FromLocal(pDict->GetStringBy("DOS")); } else if (pDict->KeyExist("Mac")) { - csFileName = CFX_WideString::FromLocal(pDict->GetStringBy("Mac")); + *csFileName = CFX_WideString::FromLocal(pDict->GetStringBy("Mac")); } else if (pDict->KeyExist("Unix")) { - csFileName = CFX_WideString::FromLocal(pDict->GetStringBy("Unix")); + *csFileName = CFX_WideString::FromLocal(pDict->GetStringBy("Unix")); } else { - return FALSE; + return false; } } + } else if (m_pObj->IsString()) { + *csFileName = CFX_WideString::FromLocal(m_pObj->GetString()); } else { - csFileName = CFX_WideString::FromLocal(m_pObj->GetString()); + return false; } - csFileName = FILESPEC_DecodeFileName(csFileName); - return TRUE; + *csFileName = DecodeFileName(*csFileName); + return true; } + CPDF_FileSpec::CPDF_FileSpec() { m_pObj = new CPDF_Dictionary; - if (CPDF_Dictionary* pDict = ToDictionary(m_pObj)) { - pDict->SetAtName("Type", "Filespec"); - } + m_pObj->AsDictionary()->SetAtName("Type", "Filespec"); } -FX_BOOL CPDF_FileSpec::IsURL() const { - if (CPDF_Dictionary* pDict = ToDictionary(m_pObj)) { - return pDict->GetStringBy("FS") == "URL"; - } - return FALSE; -} -CFX_WideString FILESPEC_EncodeFileName(const CFX_WideStringC& filepath) { + +CFX_WideString CPDF_FileSpec::EncodeFileName(const CFX_WideStringC& filepath) { if (filepath.GetLength() <= 1) { return CFX_WideString(); } @@ -377,40 +370,20 @@ CFX_WideString FILESPEC_EncodeFileName(const CFX_WideStringC& filepath) { return filepath; #endif } -CPDF_Stream* CPDF_FileSpec::GetFileStream() const { + +void CPDF_FileSpec::SetFileName(const CFX_WideStringC& wsFileName) { if (!m_pObj) - return nullptr; - if (CPDF_Stream* pStream = m_pObj->AsStream()) - return pStream; - if (CPDF_Dictionary* pEF = m_pObj->AsDictionary()->GetDictBy("EF")) - return pEF->GetStreamBy("F"); - return nullptr; -} -static void FPDFDOC_FILESPEC_SetFileName(CPDF_Object* pObj, - const CFX_WideStringC& wsFileName, - FX_BOOL bURL) { - CFX_WideString wsStr; - if (bURL) { - wsStr = wsFileName; - } else { - wsStr = FILESPEC_EncodeFileName(wsFileName); - } - if (pObj->IsString()) { - pObj->SetString(CFX_ByteString::FromUnicode(wsStr)); - } else if (CPDF_Dictionary* pDict = pObj->AsDictionary()) { + return; + + CFX_WideString wsStr = EncodeFileName(wsFileName); + if (m_pObj->IsString()) { + m_pObj->SetString(CFX_ByteString::FromUnicode(wsStr)); + } else if (CPDF_Dictionary* pDict = m_pObj->AsDictionary()) { pDict->SetAtString("F", CFX_ByteString::FromUnicode(wsStr)); pDict->SetAtString("UF", PDF_EncodeText(wsStr)); } } -void CPDF_FileSpec::SetFileName(const CFX_WideStringC& wsFileName, - FX_BOOL bURL) { - if (bURL) { - if (CPDF_Dictionary* pDict = m_pObj->AsDictionary()) { - pDict->SetAtName("FS", "URL"); - } - } - FPDFDOC_FILESPEC_SetFileName(m_pObj, wsFileName, bURL); -} + static CFX_WideString _MakeRoman(int num) { const int arabic[] = {1000, 900, 500, 400, 100, 90, 50, 40, 10, 9, 5, 4, 1}; const CFX_WideString roman[] = {L"m", L"cm", L"d", L"cd", L"c", @@ -429,6 +402,7 @@ static CFX_WideString _MakeRoman(int num) { } return wsRomanNumber; } + static CFX_WideString _MakeLetters(int num) { if (num == 0) { return CFX_WideString(); diff --git a/core/src/fpdfdoc/doc_basic_unittest.cpp b/core/src/fpdfdoc/doc_basic_unittest.cpp new file mode 100644 index 0000000000..ecc603aa9e --- /dev/null +++ b/core/src/fpdfdoc/doc_basic_unittest.cpp @@ -0,0 +1,168 @@ +// Copyright 2016 PDFium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/include/fpdfdoc/fpdf_doc.h" + +#include +#include + +#include "testing/gtest/include/gtest/gtest.h" +#include "testing/test_support.h" + +namespace { + +using ScopedObj = std::unique_ptr>; +using ScopedDict = + std::unique_ptr>; +} + +TEST(doc_basic_filespec, EncodeDecodeFileName) { + std::vector test_data = { + // Empty src string. + {L"", L""}, + // only file name. + {L"test.pdf", L"test.pdf"}, +#if _FXM_PLATFORM_ == _FXM_PLATFORM_WINDOWS_ + // With drive identifier. + {L"r:\\pdfdocs\\spec.pdf", L"/r/pdfdocs/spec.pdf"}, + // Relative path. + {L"My Document\\test.pdf", L"My Document/test.pdf"}, + // Absolute path without drive identifier. + {L"\\pdfdocs\\spec.pdf", L"//pdfdocs/spec.pdf"}, + // Absolute path with double backslashes. + {L"\\\\pdfdocs\\spec.pdf", L"/pdfdocs/spec.pdf"}, + // Network resource name. It is not supported yet. + // {L"pclib/eng:\\pdfdocs\\spec.pdf", L"/pclib/eng/pdfdocs/spec.pdf"}, +#elif _FXM_PLATFORM_ == _FXM_PLATFORM_APPLE_ + // Absolute path with colon separator. + {L"Mac HD:PDFDocs:spec.pdf", L"/Mac HD/PDFDocs/spec.pdf"}, + // Relative path with colon separator. + {L"PDFDocs:spec.pdf", L"PDFDocs/spec.pdf"}, +#else + // Relative path. + {L"./docs/test.pdf", L"./docs/test.pdf"}, + // Relative path with parent dir. + {L"../test_docs/test.pdf", L"../test_docs/test.pdf"}, + // Absolute path. + {L"/usr/local/home/test.pdf", L"/usr/local/home/test.pdf"}, +#endif + }; + for (const auto& data : test_data) { + CFX_WideString encoded_str = CPDF_FileSpec::EncodeFileName(data.input); + EXPECT_TRUE(encoded_str.Equal(data.expected)); + // DecodeFileName is the reverse procedure of EncodeFileName. + CFX_WideString decoded_str = CPDF_FileSpec::DecodeFileName(data.expected); + EXPECT_TRUE(decoded_str.Equal(data.input)); + } +} + +TEST(doc_basic_filespec, GetFileName) { + { + // String object. + pdfium::NullTermWstrFuncTestData test_data = { +#if _FXM_PLATFORM_ == _FXM_PLATFORM_WINDOWS_ + L"/C/docs/test.pdf", + L"C:\\docs\\test.pdf" +#elif _FXM_PLATFORM_ == _FXM_PLATFORM_APPLE_ + L"/Mac HD/docs/test.pdf", + L"Mac HD:docs:test.pdf" +#else + L"/docs/test.pdf", + L"/docs/test.pdf" +#endif + }; + ScopedObj str_obj(new CPDF_String(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.Equal(test_data.expected)); + } + { + // Dictionary object. + pdfium::NullTermWstrFuncTestData test_data[5] = { +#if _FXM_PLATFORM_ == _FXM_PLATFORM_WINDOWS_ + {L"/C/docs/test.pdf", L"C:\\docs\\test.pdf"}, + {L"/D/docs/test.pdf", L"D:\\docs\\test.pdf"}, + {L"/E/docs/test.pdf", L"E:\\docs\\test.pdf"}, + {L"/F/docs/test.pdf", L"F:\\docs\\test.pdf"}, + {L"/G/docs/test.pdf", L"G:\\docs\\test.pdf"}, +#elif _FXM_PLATFORM_ == _FXM_PLATFORM_APPLE_ + {L"/Mac HD/docs1/test.pdf", L"Mac HD:docs1:test.pdf"}, + {L"/Mac HD/docs2/test.pdf", L"Mac HD:docs2:test.pdf"}, + {L"/Mac HD/docs3/test.pdf", L"Mac HD:docs3:test.pdf"}, + {L"/Mac HD/docs4/test.pdf", L"Mac HD:docs4:test.pdf"}, + {L"/Mac HD/docs5/test.pdf", L"Mac HD:docs5:test.pdf"}, +#else + {L"/docs/a/test.pdf", L"/docs/a/test.pdf"}, + {L"/docs/b/test.pdf", L"/docs/b/test.pdf"}, + {L"/docs/c/test.pdf", L"/docs/c/test.pdf"}, + {L"/docs/d/test.pdf", L"/docs/d/test.pdf"}, + {L"/docs/e/test.pdf", L"/docs/e/test.pdf"}, +#endif + }; + // Keyword fields in reverse order of precedence to retrieve the file name. + const char* const keywords[5] = {"Unix", "Mac", "DOS", "F", "UF"}; + ScopedDict dict_obj(new CPDF_Dictionary); + CPDF_FileSpec file_spec(dict_obj.get()); + CFX_WideString file_name; + for (int i = 0; i < 5; ++i) { + dict_obj->SetAt(keywords[i], new CPDF_String(test_data[i].input)); + EXPECT_TRUE(file_spec.GetFileName(&file_name)); + EXPECT_TRUE(file_name.Equal(test_data[i].expected)); + } + + // With all the former fields and 'FS' field suggests 'URL' type. + dict_obj->SetAtString("FS", "URL"); + EXPECT_TRUE(file_spec.GetFileName(&file_name)); + // Url string is not decoded. + EXPECT_TRUE(file_name.Equal(test_data[4].input)); + } + { + // Invalid object. + ScopedObj name_obj(new CPDF_Name("test.pdf")); + CPDF_FileSpec file_spec(name_obj.get()); + CFX_WideString file_name; + EXPECT_FALSE(file_spec.GetFileName(&file_name)); + } +} + +TEST(doc_basic_filespec, SetFileName) { + pdfium::NullTermWstrFuncTestData test_data = { +#if _FXM_PLATFORM_ == _FXM_PLATFORM_WINDOWS_ + L"C:\\docs\\test.pdf", + L"/C/docs/test.pdf" +#elif _FXM_PLATFORM_ == _FXM_PLATFORM_APPLE_ + L"Mac HD:docs:test.pdf", + L"/Mac HD/docs/test.pdf" +#else + L"/docs/test.pdf", + L"/docs/test.pdf" +#endif + }; + // String object. + ScopedObj str_obj(new CPDF_String(L"babababa")); + CPDF_FileSpec file_spec1(str_obj.get()); + file_spec1.SetFileName(test_data.input); + // Check internal object value. + CFX_ByteString str; + str.ConvertFrom(test_data.expected); + EXPECT_TRUE(str == str_obj->GetString()); + // Check we can get the file name back. + CFX_WideString file_name; + EXPECT_TRUE(file_spec1.GetFileName(&file_name)); + EXPECT_TRUE(file_name.Equal(test_data.input)); + + // Dictionary object. + ScopedDict dict_obj(new CPDF_Dictionary); + CPDF_FileSpec file_spec2(dict_obj.get()); + file_spec2.SetFileName(test_data.input); + // Check internal object value. + file_name = dict_obj->GetUnicodeTextBy("F"); + EXPECT_TRUE(file_name.Equal(test_data.expected)); + file_name = dict_obj->GetUnicodeTextBy("UF"); + EXPECT_TRUE(file_name.Equal(test_data.expected)); + // Check we can get the file name back. + EXPECT_TRUE(file_spec2.GetFileName(&file_name)); + EXPECT_TRUE(file_name.Equal(test_data.input)); +} diff --git a/core/src/fpdfdoc/doc_form.cpp b/core/src/fpdfdoc/doc_form.cpp index aeddb28e96..460a44a7df 100644 --- a/core/src/fpdfdoc/doc_form.cpp +++ b/core/src/fpdfdoc/doc_form.cpp @@ -1041,13 +1041,13 @@ CFDF_Document* CPDF_InterForm::ExportToFDF( CPDF_Dictionary* pMainDict = pDoc->GetRoot()->GetDictBy("FDF"); if (!pdf_path.IsEmpty()) { if (bSimpleFileSpec) { - CFX_WideString wsFilePath = FILESPEC_EncodeFileName(pdf_path); + CFX_WideString wsFilePath = CPDF_FileSpec::EncodeFileName(pdf_path); pMainDict->SetAtString("F", CFX_ByteString::FromUnicode(wsFilePath)); pMainDict->SetAtString("UF", PDF_EncodeText(wsFilePath)); } else { CPDF_FileSpec filespec; filespec.SetFileName(pdf_path); - pMainDict->SetAt("F", static_cast(filespec)); + pMainDict->SetAt("F", filespec.GetObj()); } } CPDF_Array* pFields = new CPDF_Array; -- cgit v1.2.3