Convert CFX_XMLParser::current_text_ to WideString Avoid use of UNSAFE_BUFFERS in the future by using our checked string representation rather than just pointers into a DataVector<>. -- Note that character-by-character append to string is not costly. Change-Id: I24034dd01be19d0e4b1bb73490a337f8c41f0a71 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/116735 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Tom Sepez <tsepez@chromium.org> Reviewed-by: Thomas Sepez <tsepez@google.com>
diff --git a/core/fxcrt/string_template.cpp b/core/fxcrt/string_template.cpp index 672f7cb..45339b5 100644 --- a/core/fxcrt/string_template.cpp +++ b/core/fxcrt/string_template.cpp
@@ -357,21 +357,19 @@ if (!pSrcData || nSrcLen == 0) { return; } - if (!m_pData) { m_pData = StringData::Create({pSrcData, nSrcLen}); return; } - if (m_pData->CanOperateInPlace(m_pData->m_nDataLength + nSrcLen)) { m_pData->CopyContentsAt(m_pData->m_nDataLength, {pSrcData, nSrcLen}); m_pData->m_nDataLength += nSrcLen; return; } - - size_t nConcatLen = std::max(m_pData->m_nDataLength / 2, nSrcLen); + // Increase size by at least 50% to amortize repeated concatenations. + size_t nGrowLen = std::max(m_pData->m_nDataLength / 2, nSrcLen); RetainPtr<StringData> pNewData = - StringData::Create(m_pData->m_nDataLength + nConcatLen); + StringData::Create(m_pData->m_nDataLength + nGrowLen); pNewData->CopyContents(*m_pData); pNewData->CopyContentsAt(m_pData->m_nDataLength, {pSrcData, nSrcLen}); pNewData->m_nDataLength = m_pData->m_nDataLength + nSrcLen;
diff --git a/core/fxcrt/xml/cfx_xmlparser.cpp b/core/fxcrt/xml/cfx_xmlparser.cpp index 274c757..07fba43 100644 --- a/core/fxcrt/xml/cfx_xmlparser.cpp +++ b/core/fxcrt/xml/cfx_xmlparser.cpp
@@ -78,7 +78,7 @@ xml_plane_size_ = std::min(xml_plane_size_, pdfium::checked_cast<size_t>(stream_->GetSize())); - current_text_.reserve(kCurrentTextReserve); + current_text_.Reserve(kCurrentTextReserve); } CFX_XMLParser::~CFX_XMLParser() = default; @@ -130,7 +130,7 @@ switch (current_parser_state) { case FDE_XmlSyntaxState::Text: if (ch == L'<') { - if (!current_text_.empty()) { + if (!current_text_.IsEmpty()) { current_node_->AppendLastChild( doc->CreateNode<CFX_XMLText>(GetTextData())); } else { @@ -163,9 +163,10 @@ } break; case FDE_XmlSyntaxState::Target: - if (!IsXMLNameChar(ch, current_text_.empty())) { - if (current_text_.empty()) + if (!IsXMLNameChar(ch, current_text_.IsEmpty())) { + if (current_text_.IsEmpty()) { return false; + } current_parser_state = FDE_XmlSyntaxState::TargetData; @@ -177,14 +178,15 @@ current_node_ = node; } } else { - current_text_.push_back(ch); + current_text_ += ch; current_buffer_idx++; } break; case FDE_XmlSyntaxState::Tag: - if (!IsXMLNameChar(ch, current_text_.empty())) { - if (current_text_.empty()) + if (!IsXMLNameChar(ch, current_text_.IsEmpty())) { + if (current_text_.IsEmpty()) { return false; + } current_parser_state = FDE_XmlSyntaxState::AttriName; @@ -192,17 +194,17 @@ current_node_->AppendLastChild(child); current_node_ = child; } else { - current_text_.push_back(ch); + current_text_ += ch; current_buffer_idx++; } break; case FDE_XmlSyntaxState::AttriName: - if (current_text_.empty() && IsXMLWhiteSpace(ch)) { + if (current_text_.IsEmpty() && IsXMLWhiteSpace(ch)) { current_buffer_idx++; break; } - if (!IsXMLNameChar(ch, current_text_.empty())) { - if (current_text_.empty()) { + if (!IsXMLNameChar(ch, current_text_.IsEmpty())) { + if (current_text_.IsEmpty()) { if (node_type_stack.top() == CFX_XMLNode::Type::kElement) { if (ch == L'>' || ch == L'/') { current_parser_state = FDE_XmlSyntaxState::BreakElement; @@ -230,7 +232,7 @@ current_attribute_name = GetTextData(); } } else { - current_text_.push_back(ch); + current_text_ += ch; current_buffer_idx++; } break; @@ -284,9 +286,9 @@ break; case FDE_XmlSyntaxState::CloseInstruction: if (ch != L'>') { - current_text_.push_back(ch); + current_text_ += ch; current_parser_state = FDE_XmlSyntaxState::TargetData; - } else if (!current_text_.empty()) { + } else if (!current_text_.IsEmpty()) { ProcessTargetData(); } else { current_buffer_idx++; @@ -312,7 +314,7 @@ current_buffer_idx++; break; case FDE_XmlSyntaxState::CloseElement: - if (!IsXMLNameChar(ch, current_text_.empty())) { + if (!IsXMLNameChar(ch, current_text_.IsEmpty())) { if (ch == L'>') { if (node_type_stack.empty()) return false; @@ -335,7 +337,7 @@ return false; } } else { - current_text_.push_back(ch); + current_text_ += ch; } current_buffer_idx++; break; @@ -364,7 +366,7 @@ current_node_->AppendLastChild( doc->CreateNode<CFX_XMLCharData>(GetTextData())); } else { - current_text_.push_back(ch); + current_text_ += ch; current_buffer_idx++; } break; @@ -430,7 +432,7 @@ } case FDE_XmlSyntaxState::TargetData: if (IsXMLWhiteSpace(ch)) { - if (current_text_.empty()) { + if (current_text_.IsEmpty()) { current_buffer_idx++; break; } @@ -455,7 +457,7 @@ return false; } } else { - current_text_.push_back(ch); + current_text_ += ch; current_buffer_idx++; } break; @@ -467,16 +469,18 @@ } void CFX_XMLParser::ProcessTextChar(wchar_t character) { - current_text_.push_back(character); + current_text_ += character; if (entity_start_.has_value() && character == L';') { - // Copy the entity out into a string and remove from the vector. When we - // copy the entity we don't want to copy out the & or the ; so we start + // Copy the entity out into a string and remove from the current text. When + // we copy the entity we don't want to copy out the & or the ; so we start // shifted by one and want to copy 2 less characters in total. - WideString csEntity(current_text_.data() + entity_start_.value() + 1, - current_text_.size() - entity_start_.value() - 2); - current_text_.erase(current_text_.begin() + entity_start_.value(), - current_text_.end()); + WideString csEntity = current_text_.Substr( + entity_start_.value() + 1, + current_text_.GetLength() - entity_start_.value() - 2); + + current_text_.Delete(entity_start_.value(), + current_text_.GetLength() - entity_start_.value()); size_t iLen = csEntity.GetLength(); if (iLen > 0) { @@ -500,24 +504,24 @@ character = static_cast<wchar_t>(ch); if (character != 0) - current_text_.push_back(character); + current_text_ += character; } else { if (csEntity == L"amp") { - current_text_.push_back(L'&'); + current_text_ += L'&'; } else if (csEntity == L"lt") { - current_text_.push_back(L'<'); + current_text_ += L'<'; } else if (csEntity == L"gt") { - current_text_.push_back(L'>'); + current_text_ += L'>'; } else if (csEntity == L"apos") { - current_text_.push_back(L'\''); + current_text_ += L'\''; } else if (csEntity == L"quot") { - current_text_.push_back(L'"'); + current_text_ += L'"'; } } } entity_start_ = std::nullopt; } else if (!entity_start_.has_value() && character == L'&') { - entity_start_ = current_text_.size() - 1; + entity_start_ = current_text_.GetLength() - 1; } } @@ -532,9 +536,8 @@ } WideString CFX_XMLParser::GetTextData() { - WideString ret(current_text_.data(), current_text_.size()); + WideString ret = std::move(current_text_); + current_text_.Reserve(kCurrentTextReserve); entity_start_ = std::nullopt; - current_text_.clear(); - current_text_.reserve(kCurrentTextReserve); return ret; }
diff --git a/core/fxcrt/xml/cfx_xmlparser.h b/core/fxcrt/xml/cfx_xmlparser.h index 34f743b..8915455 100644 --- a/core/fxcrt/xml/cfx_xmlparser.h +++ b/core/fxcrt/xml/cfx_xmlparser.h
@@ -10,7 +10,6 @@ #include <memory> #include <optional> -#include "core/fxcrt/data_vector.h" #include "core/fxcrt/retain_ptr.h" #include "core/fxcrt/unowned_ptr.h" #include "core/fxcrt/widestring.h" @@ -56,7 +55,7 @@ UnownedPtr<CFX_XMLNode> current_node_; RetainPtr<CFX_SeekableStreamProxy> stream_; - DataVector<wchar_t> current_text_; + WideString current_text_; size_t xml_plane_size_ = 1024; std::optional<size_t> entity_start_; };