From f3024c3b1fbbfe442f93da3459ea79e817d5b8fe Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Mon, 26 Jun 2017 15:28:15 -0700 Subject: Split CFDE_CSSTextBuf in two. For the external buffer use case, use a CFDE_CSSExtTextBuf instead. With the split, both text buffer implementations are simpler now. As a result, it becomes obvious where it never fails. Adjust callers accordingly. Change-Id: I7b53d36593172487b8c939e6a55af2437ea4ee5a Reviewed-on: https://pdfium-review.googlesource.com/6932 Commit-Queue: Lei Zhang Reviewed-by: Henrique Nakashima --- BUILD.gn | 2 ++ testing/libfuzzer/pdf_css_fuzzer.cc | 4 +-- xfa/fde/css/cfde_cssexttextbuf.cpp | 17 +++++++++ xfa/fde/css/cfde_cssexttextbuf.h | 34 ++++++++++++++++++ xfa/fde/css/cfde_cssstyleselector.cpp | 6 ++-- xfa/fde/css/cfde_cssstylesheet.cpp | 8 ++--- xfa/fde/css/cfde_csssyntaxparser.cpp | 41 ++++++++-------------- xfa/fde/css/cfde_csssyntaxparser.h | 15 ++++---- xfa/fde/css/cfde_csstextbuf.cpp | 66 +++++++---------------------------- xfa/fde/css/cfde_csstextbuf.h | 29 +++------------ 10 files changed, 99 insertions(+), 123 deletions(-) create mode 100644 xfa/fde/css/cfde_cssexttextbuf.cpp create mode 100644 xfa/fde/css/cfde_cssexttextbuf.h diff --git a/BUILD.gn b/BUILD.gn index 5b64e0408e..34b032fd89 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1451,6 +1451,8 @@ if (pdf_enable_xfa) { "xfa/fde/css/cfde_cssdeclaration.h", "xfa/fde/css/cfde_cssenumvalue.cpp", "xfa/fde/css/cfde_cssenumvalue.h", + "xfa/fde/css/cfde_cssexttextbuf.cpp", + "xfa/fde/css/cfde_cssexttextbuf.h", "xfa/fde/css/cfde_cssnumbervalue.cpp", "xfa/fde/css/cfde_cssnumbervalue.h", "xfa/fde/css/cfde_csspropertyholder.cpp", diff --git a/testing/libfuzzer/pdf_css_fuzzer.cc b/testing/libfuzzer/pdf_css_fuzzer.cc index 43ce686c22..d3b2e6a996 100644 --- a/testing/libfuzzer/pdf_css_fuzzer.cc +++ b/testing/libfuzzer/pdf_css_fuzzer.cc @@ -18,9 +18,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { if (input.GetLength() == 0) return 0; - CFDE_CSSSyntaxParser parser; - parser.Init(input.c_str(), input.GetLength()); - + CFDE_CSSSyntaxParser parser(input.c_str(), input.GetLength()); FDE_CSSSyntaxStatus status; do { status = parser.DoSyntaxParse(); diff --git a/xfa/fde/css/cfde_cssexttextbuf.cpp b/xfa/fde/css/cfde_cssexttextbuf.cpp new file mode 100644 index 0000000000..86f6776c48 --- /dev/null +++ b/xfa/fde/css/cfde_cssexttextbuf.cpp @@ -0,0 +1,17 @@ +// Copyright 2017 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. + +// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com + +#include "xfa/fde/css/cfde_cssexttextbuf.h" + +CFDE_CSSExtTextBuf::CFDE_CSSExtTextBuf() + : m_pExtBuffer(nullptr), m_iDatLen(0), m_iDatPos(0) {} + +CFDE_CSSExtTextBuf::~CFDE_CSSExtTextBuf() {} + +void CFDE_CSSExtTextBuf::AttachBuffer(const wchar_t* pBuffer, int32_t iBufLen) { + m_pExtBuffer = pBuffer; + m_iDatLen = iBufLen; +} diff --git a/xfa/fde/css/cfde_cssexttextbuf.h b/xfa/fde/css/cfde_cssexttextbuf.h new file mode 100644 index 0000000000..0760182f25 --- /dev/null +++ b/xfa/fde/css/cfde_cssexttextbuf.h @@ -0,0 +1,34 @@ +// Copyright 2017 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. + +// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com + +#ifndef XFA_FDE_CSS_CFDE_CSSEXTTEXTBUF_H_ +#define XFA_FDE_CSS_CFDE_CSSEXTTEXTBUF_H_ + +#include "core/fxcrt/fx_system.h" + +class CFDE_CSSExtTextBuf { + public: + CFDE_CSSExtTextBuf(); + ~CFDE_CSSExtTextBuf(); + + void AttachBuffer(const wchar_t* pBuffer, int32_t iBufLen); + + bool IsEOF() const { return m_iDatPos >= m_iDatLen; } + + wchar_t GetChar() const { return m_pExtBuffer[m_iDatPos]; } + wchar_t GetNextChar() const { + return (m_iDatPos + 1 >= m_iDatLen) ? 0 : m_pExtBuffer[m_iDatPos + 1]; + } + + void MoveNext() { m_iDatPos++; } + + protected: + const wchar_t* m_pExtBuffer; + int32_t m_iDatLen; + int32_t m_iDatPos; +}; + +#endif // XFA_FDE_CSS_CFDE_CSSEXTTEXTBUF_H_ diff --git a/xfa/fde/css/cfde_cssstyleselector.cpp b/xfa/fde/css/cfde_cssstyleselector.cpp index 461192e976..2fe306faec 100644 --- a/xfa/fde/css/cfde_cssstyleselector.cpp +++ b/xfa/fde/css/cfde_cssstyleselector.cpp @@ -142,10 +142,8 @@ void CFDE_CSSStyleSelector::AppendInlineStyle(CFDE_CSSDeclaration* pDecl, const CFX_WideString& style) { ASSERT(pDecl && !style.IsEmpty()); - auto pSyntax = pdfium::MakeUnique(); - if (!pSyntax->Init(style.c_str(), style.GetLength(), 32, true)) - return; - + auto pSyntax = pdfium::MakeUnique( + style.c_str(), style.GetLength(), 32, true); int32_t iLen2 = 0; const FDE_CSSPropertyTable* table = nullptr; CFX_WideString wsName; diff --git a/xfa/fde/css/cfde_cssstylesheet.cpp b/xfa/fde/css/cfde_cssstylesheet.cpp index c16e25156c..369279853e 100644 --- a/xfa/fde/css/cfde_cssstylesheet.cpp +++ b/xfa/fde/css/cfde_cssstylesheet.cpp @@ -35,12 +35,10 @@ CFDE_CSSStyleRule* CFDE_CSSStyleSheet::GetRule(int32_t index) const { } bool CFDE_CSSStyleSheet::LoadBuffer(const wchar_t* pBuffer, int32_t iBufSize) { - ASSERT(pBuffer && iBufSize > 0); - - auto pSyntax = pdfium::MakeUnique(); - if (!pSyntax->Init(pBuffer, iBufSize)) - return false; + ASSERT(pBuffer); + ASSERT(iBufSize > 0); + auto pSyntax = pdfium::MakeUnique(pBuffer, iBufSize); Reset(); FDE_CSSSyntaxStatus eStatus; do { diff --git a/xfa/fde/css/cfde_csssyntaxparser.cpp b/xfa/fde/css/cfde_csssyntaxparser.cpp index b896eb79b8..51d99f1992 100644 --- a/xfa/fde/css/cfde_csssyntaxparser.cpp +++ b/xfa/fde/css/cfde_csssyntaxparser.cpp @@ -22,39 +22,26 @@ bool IsSelectorStart(wchar_t wch) { } // namespace -CFDE_CSSSyntaxParser::CFDE_CSSSyntaxParser() +CFDE_CSSSyntaxParser::CFDE_CSSSyntaxParser(const wchar_t* pBuffer, + int32_t iBufferSize) + : CFDE_CSSSyntaxParser(pBuffer, iBufferSize, 32, false) {} + +CFDE_CSSSyntaxParser::CFDE_CSSSyntaxParser(const wchar_t* pBuffer, + int32_t iBufferSize, + int32_t iTextDatSize, + bool bOnlyDeclaration) : m_iTextDataLen(0), m_dwCheck(0xFFFFFFFF), - m_eMode(FDE_CSSSyntaxMode::RuleSet), - m_eStatus(FDE_CSSSyntaxStatus::None) {} - -CFDE_CSSSyntaxParser::~CFDE_CSSSyntaxParser() { - m_TextData.Reset(); - m_TextPlane.Reset(); -} - -bool CFDE_CSSSyntaxParser::Init(const wchar_t* pBuffer, - int32_t iBufferSize, - int32_t iTextDatSize, - bool bOnlyDeclaration) { + m_eStatus(FDE_CSSSyntaxStatus::None) { ASSERT(pBuffer && iBufferSize > 0 && iTextDatSize > 0); - Reset(bOnlyDeclaration); - if (!m_TextData.EstimateSize(iTextDatSize)) - return false; - m_TextPlane.AttachBuffer(pBuffer, iBufferSize); - return true; -} - -void CFDE_CSSSyntaxParser::Reset(bool bOnlyDeclaration) { - m_TextPlane.Reset(); - m_TextData.Reset(); - m_iTextDataLen = 0; - m_dwCheck = 0xFFFFFFFF; - m_eStatus = FDE_CSSSyntaxStatus::None; m_eMode = bOnlyDeclaration ? FDE_CSSSyntaxMode::PropertyName : FDE_CSSSyntaxMode::RuleSet; + m_TextData.InitWithSize(iTextDatSize); + m_TextPlane.AttachBuffer(pBuffer, iBufferSize); } +CFDE_CSSSyntaxParser::~CFDE_CSSSyntaxParser() {} + FDE_CSSSyntaxStatus CFDE_CSSSyntaxParser::DoSyntaxParse() { while (m_eStatus >= FDE_CSSSyntaxStatus::None) { if (m_TextPlane.IsEOF()) { @@ -171,7 +158,7 @@ FDE_CSSSyntaxStatus CFDE_CSSSyntaxParser::DoSyntaxParse() { break; case FDE_CSSSyntaxMode::Comment: if (wch == '/' && m_TextData.GetLength() > 0 && - m_TextData.GetAt(m_TextData.GetLength() - 1) == '*') { + m_TextData.GetBuffer()[m_TextData.GetLength() - 1] == '*') { RestoreMode(); } else { m_TextData.AppendChar(wch); diff --git a/xfa/fde/css/cfde_csssyntaxparser.h b/xfa/fde/css/cfde_csssyntaxparser.h index a5e79d50e5..c17d84aef1 100644 --- a/xfa/fde/css/cfde_csssyntaxparser.h +++ b/xfa/fde/css/cfde_csssyntaxparser.h @@ -9,6 +9,8 @@ #include +#include "core/fxcrt/fx_string.h" +#include "xfa/fde/css/cfde_cssexttextbuf.h" #include "xfa/fde/css/cfde_csstextbuf.h" #define FDE_CSSSYNTAXCHECK_AllowCharset 1 @@ -37,18 +39,17 @@ enum class FDE_CSSSyntaxStatus : uint8_t { class CFDE_CSSSyntaxParser { public: - CFDE_CSSSyntaxParser(); + CFDE_CSSSyntaxParser(const wchar_t* pBuffer, int32_t iBufferSize); + CFDE_CSSSyntaxParser(const wchar_t* pBuffer, + int32_t iBufferSize, + int32_t iTextDatSize, + bool bOnlyDeclaration); ~CFDE_CSSSyntaxParser(); - bool Init(const wchar_t* pBuffer, - int32_t iBufferSize, - int32_t iTextDatSize = 32, - bool bOnlyDeclaration = false); FDE_CSSSyntaxStatus DoSyntaxParse(); CFX_WideStringC GetCurrentString() const; protected: - void Reset(bool bOnlyDeclaration); void SwitchMode(FDE_CSSSyntaxMode eMode); int32_t SwitchToComment(); @@ -63,7 +64,7 @@ class CFDE_CSSSyntaxParser { void DisableImport() { m_dwCheck = 0; } CFDE_CSSTextBuf m_TextData; - CFDE_CSSTextBuf m_TextPlane; + CFDE_CSSExtTextBuf m_TextPlane; int32_t m_iTextDataLen; uint32_t m_dwCheck; FDE_CSSSyntaxMode m_eMode; diff --git a/xfa/fde/css/cfde_csstextbuf.cpp b/xfa/fde/css/cfde_csstextbuf.cpp index 8ccdca9fed..0d8ba2d6ce 100644 --- a/xfa/fde/css/cfde_csstextbuf.cpp +++ b/xfa/fde/css/cfde_csstextbuf.cpp @@ -6,69 +6,39 @@ #include "xfa/fde/css/cfde_csstextbuf.h" -#include "third_party/base/stl_util.h" +#include "core/fxcrt/fx_memory.h" CFDE_CSSTextBuf::CFDE_CSSTextBuf() - : m_bExtBuf(false), - m_pExtBuffer(nullptr), - m_pBuffer(nullptr), - m_iBufLen(0), - m_iDatLen(0), - m_iDatPos(0) {} + : m_pBuffer(nullptr), m_iBufLen(0), m_iDatLen(0) {} CFDE_CSSTextBuf::~CFDE_CSSTextBuf() { - Reset(); + FX_Free(m_pBuffer); + m_pBuffer = nullptr; + m_iDatLen = m_iBufLen; } -void CFDE_CSSTextBuf::AttachBuffer(const wchar_t* pBuffer, int32_t iBufLen) { - Reset(); - m_pExtBuffer = pBuffer; - m_iDatLen = m_iBufLen = iBufLen; - m_bExtBuf = true; +void CFDE_CSSTextBuf::InitWithSize(int32_t iAllocSize) { + ExpandBuf(iAllocSize); } -bool CFDE_CSSTextBuf::EstimateSize(int32_t iAllocSize) { - ASSERT(iAllocSize > 0); - Clear(); - m_bExtBuf = false; - return ExpandBuf(iAllocSize); -} - -bool CFDE_CSSTextBuf::AppendChar(wchar_t wch) { - if (m_iDatLen >= m_iBufLen && !ExpandBuf(m_iBufLen * 2)) - return false; +void CFDE_CSSTextBuf::AppendChar(wchar_t wch) { + if (m_iDatLen >= m_iBufLen) + ExpandBuf(m_iBufLen * 2); - ASSERT(!m_bExtBuf); m_pBuffer[m_iDatLen++] = wch; - return true; -} - -void CFDE_CSSTextBuf::Reset() { - if (!m_bExtBuf) { - FX_Free(m_pBuffer); - m_pBuffer = nullptr; - } - m_iDatPos = m_iDatLen = m_iBufLen; } int32_t CFDE_CSSTextBuf::TrimEnd() { - ASSERT(!m_bExtBuf); while (m_iDatLen > 0 && m_pBuffer[m_iDatLen - 1] <= ' ') --m_iDatLen; AppendChar(0); return --m_iDatLen; } -const wchar_t* CFDE_CSSTextBuf::GetBuffer() const { - return m_bExtBuf ? m_pExtBuffer : m_pBuffer; -} - -bool CFDE_CSSTextBuf::ExpandBuf(int32_t iDesiredSize) { - if (m_bExtBuf) - return false; - +void CFDE_CSSTextBuf::ExpandBuf(int32_t iDesiredSize) { + ASSERT(iDesiredSize > 0); if (m_pBuffer && m_iBufLen == iDesiredSize) - return true; + return; if (m_pBuffer) m_pBuffer = FX_Realloc(wchar_t, m_pBuffer, iDesiredSize); @@ -76,14 +46,4 @@ bool CFDE_CSSTextBuf::ExpandBuf(int32_t iDesiredSize) { m_pBuffer = FX_Alloc(wchar_t, iDesiredSize); m_iBufLen = iDesiredSize; - return true; -} - -void CFDE_CSSTextBuf::Subtract(int32_t iStart, int32_t iLength) { - ASSERT(iStart >= 0 && iLength >= 0); - ASSERT(!m_bExtBuf); - - iLength = pdfium::clamp(iLength, 0, m_iDatLen - iStart); - memmove(m_pBuffer, m_pBuffer + iStart, iLength * sizeof(wchar_t)); - m_iDatLen = iLength; } diff --git a/xfa/fde/css/cfde_csstextbuf.h b/xfa/fde/css/cfde_csstextbuf.h index be6800fe3a..b2c17c95f8 100644 --- a/xfa/fde/css/cfde_csstextbuf.h +++ b/xfa/fde/css/cfde_csstextbuf.h @@ -7,9 +7,6 @@ #ifndef XFA_FDE_CSS_CFDE_CSSTEXTBUF_H_ #define XFA_FDE_CSS_CFDE_CSSTEXTBUF_H_ -#include "core/fxcrt/cfx_retain_ptr.h" -#include "core/fxcrt/cfx_seekablestreamproxy.h" -#include "core/fxcrt/fx_memory.h" #include "core/fxcrt/fx_system.h" class CFDE_CSSTextBuf { @@ -17,38 +14,22 @@ class CFDE_CSSTextBuf { CFDE_CSSTextBuf(); ~CFDE_CSSTextBuf(); - void AttachBuffer(const wchar_t* pBuffer, int32_t iBufLen); - bool EstimateSize(int32_t iAllocSize); - bool AppendChar(wchar_t wch); + void InitWithSize(int32_t iAllocSize); + void AppendChar(wchar_t wch); - void Clear() { m_iDatPos = m_iDatLen = 0; } - void Reset(); + void Clear() { m_iDatLen = 0; } int32_t TrimEnd(); - void Subtract(int32_t iStart, int32_t iLength); - bool IsEOF() const { return m_iDatPos >= m_iDatLen; } - - wchar_t GetAt(int32_t index) const { return GetBuffer()[index]; } - wchar_t GetChar() const { return GetBuffer()[m_iDatPos]; } - wchar_t GetNextChar() const { - return (m_iDatPos + 1 >= m_iDatLen) ? 0 : GetBuffer()[m_iDatPos + 1]; - } - - void MoveNext() { m_iDatPos++; } - int32_t GetLength() const { return m_iDatLen; } - const wchar_t* GetBuffer() const; + const wchar_t* GetBuffer() const { return m_pBuffer; } protected: - bool ExpandBuf(int32_t iDesiredSize); + void ExpandBuf(int32_t iDesiredSize); - bool m_bExtBuf; - const wchar_t* m_pExtBuffer; wchar_t* m_pBuffer; int32_t m_iBufLen; int32_t m_iDatLen; - int32_t m_iDatPos; }; #endif // XFA_FDE_CSS_CFDE_CSSTEXTBUF_H_ -- cgit v1.2.3