From 690d456ad54f021063dcc17fde27c7ba4d910717 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Thu, 18 May 2017 11:42:46 -0700 Subject: Use UnownedPtr to check CFX_*StringC lifetimes Change interform to avoid temp StringC with dangling ptr. Change-Id: I8d8659973bcdf2cdbcaa6efa6012e4acce5f1604 Reviewed-on: https://pdfium-review.googlesource.com/5571 Commit-Queue: Tom Sepez Reviewed-by: Lei Zhang --- core/fpdfdoc/cpdf_filespec.cpp | 8 +++---- core/fpdfdoc/cpdf_filespec.h | 6 ++--- core/fpdfdoc/cpdf_interform.cpp | 4 ++-- core/fpdfdoc/cpdf_interform.h | 4 ++-- core/fxcrt/cfx_string_c_template.h | 46 +++++++++++++++++++++----------------- core/fxcrt/cfx_unowned_ptr.h | 2 +- fpdfsdk/cpdfsdk_interform.cpp | 13 +++++------ xfa/fxfa/fm2js/cxfa_fmlexer.cpp | 2 ++ xfa/fxfa/fm2js/cxfa_fmlexer.h | 1 + 9 files changed, 46 insertions(+), 40 deletions(-) diff --git a/core/fpdfdoc/cpdf_filespec.cpp b/core/fpdfdoc/cpdf_filespec.cpp index 7fe8ec8c03..59915eaa05 100644 --- a/core/fpdfdoc/cpdf_filespec.cpp +++ b/core/fpdfdoc/cpdf_filespec.cpp @@ -50,7 +50,7 @@ CFX_WideString ChangeSlashToPDF(const wchar_t* str) { } // namespace -CFX_WideString CPDF_FileSpec::DecodeFileName(const CFX_WideStringC& filepath) { +CFX_WideString CPDF_FileSpec::DecodeFileName(const CFX_WideString& filepath) { if (filepath.GetLength() <= 1) return CFX_WideString(); @@ -108,7 +108,7 @@ bool CPDF_FileSpec::GetFileName(CFX_WideString* csFileName) const { } else { return false; } - *csFileName = DecodeFileName(csFileName->AsStringC()); + *csFileName = DecodeFileName(*csFileName); return true; } @@ -116,7 +116,7 @@ CPDF_FileSpec::CPDF_FileSpec(CPDF_Object* pObj) : m_pObj(pObj) {} CPDF_FileSpec::~CPDF_FileSpec() {} -CFX_WideString CPDF_FileSpec::EncodeFileName(const CFX_WideStringC& filepath) { +CFX_WideString CPDF_FileSpec::EncodeFileName(const CFX_WideString& filepath) { if (filepath.GetLength() <= 1) return CFX_WideString(); @@ -145,7 +145,7 @@ CFX_WideString CPDF_FileSpec::EncodeFileName(const CFX_WideStringC& filepath) { #endif } -void CPDF_FileSpec::SetFileName(const CFX_WideStringC& wsFileName) { +void CPDF_FileSpec::SetFileName(const CFX_WideString& wsFileName) { if (!m_pObj) return; diff --git a/core/fpdfdoc/cpdf_filespec.h b/core/fpdfdoc/cpdf_filespec.h index fb71b57186..04baf2e63f 100644 --- a/core/fpdfdoc/cpdf_filespec.h +++ b/core/fpdfdoc/cpdf_filespec.h @@ -20,16 +20,16 @@ class CPDF_FileSpec { ~CPDF_FileSpec(); // Convert a platform dependent file name into pdf format. - static CFX_WideString EncodeFileName(const CFX_WideStringC& filepath); + static CFX_WideString EncodeFileName(const CFX_WideString& filepath); // Convert a pdf file name into platform dependent format. - static CFX_WideString DecodeFileName(const CFX_WideStringC& filepath); + static CFX_WideString DecodeFileName(const CFX_WideString& filepath); CPDF_Object* GetObj() const { return m_pObj.Get(); } bool GetFileName(CFX_WideString* wsFileName) const; // Set this file spec to refer to a file name (not a url). - void SetFileName(const CFX_WideStringC& wsFileName); + void SetFileName(const CFX_WideString& wsFileName); private: CFX_UnownedPtr const m_pObj; diff --git a/core/fpdfdoc/cpdf_interform.cpp b/core/fpdfdoc/cpdf_interform.cpp index 766ec6cd4f..88787d2e5b 100644 --- a/core/fpdfdoc/cpdf_interform.cpp +++ b/core/fpdfdoc/cpdf_interform.cpp @@ -1179,7 +1179,7 @@ bool CPDF_InterForm::CheckRequiredFields( } std::unique_ptr CPDF_InterForm::ExportToFDF( - const CFX_WideStringC& pdf_path, + const CFX_WideString& pdf_path, bool bSimpleFileSpec) const { std::vector fields; size_t nCount = m_pFieldTree->m_Root.CountFields(); @@ -1189,7 +1189,7 @@ std::unique_ptr CPDF_InterForm::ExportToFDF( } std::unique_ptr CPDF_InterForm::ExportToFDF( - const CFX_WideStringC& pdf_path, + const CFX_WideString& pdf_path, const std::vector& fields, bool bIncludeOrExclude, bool bSimpleFileSpec) const { diff --git a/core/fpdfdoc/cpdf_interform.h b/core/fpdfdoc/cpdf_interform.h index 5399392734..23530e57b2 100644 --- a/core/fpdfdoc/cpdf_interform.h +++ b/core/fpdfdoc/cpdf_interform.h @@ -71,11 +71,11 @@ class CPDF_InterForm { bool CheckRequiredFields(const std::vector* fields, bool bIncludeOrExclude) const; - std::unique_ptr ExportToFDF(const CFX_WideStringC& pdf_path, + std::unique_ptr ExportToFDF(const CFX_WideString& pdf_path, bool bSimpleFileSpec) const; std::unique_ptr ExportToFDF( - const CFX_WideStringC& pdf_path, + const CFX_WideString& pdf_path, const std::vector& fields, bool bIncludeOrExclude, bool bSimpleFileSpec) const; diff --git a/core/fxcrt/cfx_string_c_template.h b/core/fxcrt/cfx_string_c_template.h index 75c2be0efc..3a0ad74c08 100644 --- a/core/fxcrt/cfx_string_c_template.h +++ b/core/fxcrt/cfx_string_c_template.h @@ -11,6 +11,7 @@ #include #include +#include "core/fxcrt/cfx_unowned_ptr.h" #include "core/fxcrt/fx_system.h" #include "third_party/base/stl_util.h" @@ -75,22 +76,23 @@ class CFX_StringCTemplate { } const_iterator begin() const { - return reinterpret_cast(m_Ptr); + return reinterpret_cast(m_Ptr.Get()); } const_iterator end() const { - return m_Ptr ? reinterpret_cast(m_Ptr) + m_Length + return m_Ptr ? reinterpret_cast(m_Ptr.Get()) + m_Length : nullptr; } bool operator==(const CharType* ptr) const { return FXSYS_len(ptr) == m_Length && - FXSYS_cmp(ptr, reinterpret_cast(m_Ptr), m_Length) == - 0; + FXSYS_cmp(ptr, reinterpret_cast(m_Ptr.Get()), + m_Length) == 0; } bool operator==(const CFX_StringCTemplate& other) const { return other.m_Length == m_Length && - FXSYS_cmp(reinterpret_cast(other.m_Ptr), - reinterpret_cast(m_Ptr), m_Length) == 0; + FXSYS_cmp(reinterpret_cast(other.m_Ptr.Get()), + reinterpret_cast(m_Ptr.Get()), + m_Length) == 0; } bool operator!=(const CharType* ptr) const { return !(*this == ptr); } bool operator!=(const CFX_StringCTemplate& other) const { @@ -104,28 +106,28 @@ class CFX_StringCTemplate { uint32_t strid = 0; FX_STRSIZE size = std::min(4, m_Length - start_pos); for (FX_STRSIZE i = 0; i < size; i++) - strid = strid * 256 + m_Ptr[start_pos + i]; + strid = strid * 256 + m_Ptr.Get()[start_pos + i]; return strid << ((4 - size) * 8); } - const UnsignedType* raw_str() const { return m_Ptr; } + const UnsignedType* raw_str() const { return m_Ptr.Get(); } const CharType* c_str() const { - return reinterpret_cast(m_Ptr); + return reinterpret_cast(m_Ptr.Get()); } FX_STRSIZE GetLength() const { return m_Length; } bool IsEmpty() const { return m_Length == 0; } - UnsignedType GetAt(FX_STRSIZE index) const { return m_Ptr[index]; } + UnsignedType GetAt(FX_STRSIZE index) const { return m_Ptr.Get()[index]; } CharType CharAt(FX_STRSIZE index) const { - return static_cast(m_Ptr[index]); + return static_cast(m_Ptr.Get()[index]); } FX_STRSIZE Find(CharType ch) const { - const UnsignedType* found = reinterpret_cast( - FXSYS_chr(reinterpret_cast(m_Ptr), ch, m_Length)); - return found ? found - m_Ptr : -1; + const UnsignedType* found = reinterpret_cast(FXSYS_chr( + reinterpret_cast(m_Ptr.Get()), ch, m_Length)); + return found ? found - m_Ptr.Get() : -1; } CFX_StringCTemplate Mid(FX_STRSIZE index, FX_STRSIZE count = -1) const { @@ -136,14 +138,14 @@ class CFX_StringCTemplate { if (count < 0 || count > m_Length - index) count = m_Length - index; - return CFX_StringCTemplate(m_Ptr + index, count); + return CFX_StringCTemplate(m_Ptr.Get() + index, count); } CFX_StringCTemplate Left(FX_STRSIZE count) const { if (count <= 0) return CFX_StringCTemplate(); - return CFX_StringCTemplate(m_Ptr, std::min(count, m_Length)); + return CFX_StringCTemplate(m_Ptr.Get(), std::min(count, m_Length)); } CFX_StringCTemplate Right(FX_STRSIZE count) const { @@ -151,20 +153,22 @@ class CFX_StringCTemplate { return CFX_StringCTemplate(); count = std::min(count, m_Length); - return CFX_StringCTemplate(m_Ptr + m_Length - count, count); + return CFX_StringCTemplate(m_Ptr.Get() + m_Length - count, count); } - const UnsignedType& operator[](size_t index) const { return m_Ptr[index]; } + const UnsignedType& operator[](size_t index) const { + return m_Ptr.Get()[index]; + } bool operator<(const CFX_StringCTemplate& that) const { - int result = FXSYS_cmp(reinterpret_cast(m_Ptr), - reinterpret_cast(that.m_Ptr), + int result = FXSYS_cmp(reinterpret_cast(m_Ptr.Get()), + reinterpret_cast(that.m_Ptr.Get()), std::min(m_Length, that.m_Length)); return result < 0 || (result == 0 && m_Length < that.m_Length); } protected: - const UnsignedType* m_Ptr; + CFX_UnownedPtr m_Ptr; FX_STRSIZE m_Length; private: diff --git a/core/fxcrt/cfx_unowned_ptr.h b/core/fxcrt/cfx_unowned_ptr.h index bc58144335..d91833abd4 100644 --- a/core/fxcrt/cfx_unowned_ptr.h +++ b/core/fxcrt/cfx_unowned_ptr.h @@ -64,7 +64,7 @@ class CFX_UnownedPtr { #if defined(MEMORY_TOOL_REPLACES_ALLOCATOR) void Probe() { if (m_pObj) - reinterpret_cast(m_pObj)[0]; + reinterpret_cast(m_pObj)[0]; } #endif diff --git a/fpdfsdk/cpdfsdk_interform.cpp b/fpdfsdk/cpdfsdk_interform.cpp index 2d02a7ab93..76cb6fdabf 100644 --- a/fpdfsdk/cpdfsdk_interform.cpp +++ b/fpdfsdk/cpdfsdk_interform.cpp @@ -502,9 +502,8 @@ bool CPDFSDK_InterForm::ExportFieldsToFDFTextBuf( const std::vector& fields, bool bIncludeOrExclude, CFX_ByteTextBuf& textBuf) { - std::unique_ptr pFDF = - m_pInterForm->ExportToFDF(m_pFormFillEnv->JS_docGetFilePath().AsStringC(), - fields, bIncludeOrExclude, false); + std::unique_ptr pFDF = m_pInterForm->ExportToFDF( + m_pFormFillEnv->JS_docGetFilePath(), fields, bIncludeOrExclude, false); return pFDF ? pFDF->WriteBuf(textBuf) : false; } @@ -521,8 +520,8 @@ bool CPDFSDK_InterForm::SubmitForm(const CFX_WideString& sDestination, if (!m_pFormFillEnv || !m_pInterForm) return false; - std::unique_ptr pFDFDoc = m_pInterForm->ExportToFDF( - m_pFormFillEnv->JS_docGetFilePath().AsStringC(), false); + std::unique_ptr pFDFDoc = + m_pInterForm->ExportToFDF(m_pFormFillEnv->JS_docGetFilePath(), false); if (!pFDFDoc) return false; @@ -543,8 +542,8 @@ bool CPDFSDK_InterForm::SubmitForm(const CFX_WideString& sDestination, } bool CPDFSDK_InterForm::ExportFormToFDFTextBuf(CFX_ByteTextBuf& textBuf) { - std::unique_ptr pFDF = m_pInterForm->ExportToFDF( - m_pFormFillEnv->JS_docGetFilePath().AsStringC(), false); + std::unique_ptr pFDF = + m_pInterForm->ExportToFDF(m_pFormFillEnv->JS_docGetFilePath(), false); return pFDF && pFDF->WriteBuf(textBuf); } diff --git a/xfa/fxfa/fm2js/cxfa_fmlexer.cpp b/xfa/fxfa/fm2js/cxfa_fmlexer.cpp index db7cc956e8..433839a88c 100644 --- a/xfa/fxfa/fm2js/cxfa_fmlexer.cpp +++ b/xfa/fxfa/fm2js/cxfa_fmlexer.cpp @@ -94,6 +94,8 @@ CXFA_FMToken::CXFA_FMToken() : m_type(TOKreserver), m_uLinenum(1) {} CXFA_FMToken::CXFA_FMToken(uint32_t uLineNum) : m_type(TOKreserver), m_uLinenum(uLineNum) {} +CXFA_FMToken::~CXFA_FMToken() {} + CXFA_FMLexer::CXFA_FMLexer(const CFX_WideStringC& wsFormCalc) : m_ptr(wsFormCalc.c_str()), m_end(m_ptr + wsFormCalc.GetLength() - 1), diff --git a/xfa/fxfa/fm2js/cxfa_fmlexer.h b/xfa/fxfa/fm2js/cxfa_fmlexer.h index f4f8a68d8c..3b5b3727e7 100644 --- a/xfa/fxfa/fm2js/cxfa_fmlexer.h +++ b/xfa/fxfa/fm2js/cxfa_fmlexer.h @@ -95,6 +95,7 @@ class CXFA_FMToken { public: CXFA_FMToken(); explicit CXFA_FMToken(uint32_t uLineNum); + ~CXFA_FMToken(); CFX_WideStringC m_wstring; XFA_FM_TOKEN m_type; -- cgit v1.2.3