include/xmloff/xmlimppr.hxx | 1 - sax/source/expatwrap/saxwriter.cxx | 3 +++ sw/source/filter/xml/xmlimpit.cxx | 10 +++++++++- xmloff/source/core/attrlist.cxx | 9 ++++++--- xmloff/source/core/xmlcnimp.cxx | 16 ++++++++++++++++ xmloff/source/style/xmlimppr.cxx | 34 ++++++++++++++++++---------------- 6 files changed, 52 insertions(+), 21 deletions(-)
New commits: commit 12a7a3d57d3dcf222b13fa9143c37736f8ea3d0b Author: Noel Grandin <noel.gran...@collabora.co.uk> AuthorDate: Thu Sep 3 13:42:40 2020 +0200 Commit: Noel Grandin <noel.gran...@collabora.co.uk> CommitDate: Thu Sep 3 15:33:36 2020 +0200 Fix crashtest fdo77855.odt regression from commit commit 9814c1f2edf56ecc0f31001db9234ef335488879 use fastparser in SvXMLPropertySetContext subclasses and add some asserts to find the problems earlier. Change-Id: Ief64f813f2ef7ec005f682713dadb1be47cbcd15 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/101998 Tested-by: Jenkins Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk> diff --git a/include/xmloff/xmlimppr.hxx b/include/xmloff/xmlimppr.hxx index a8c91a2ba5b4..c5e32d064998 100644 --- a/include/xmloff/xmlimppr.hxx +++ b/include/xmloff/xmlimppr.hxx @@ -185,7 +185,6 @@ private: sal_Int32 nStartIdx, sal_Int32 nEndIdx, css::uno::Reference< css::container::XNameContainer >& xAttrContainer, - const OUString& aPrefix, const OUString& sAttrName, const OUString& aNamespaceURI, const OUString& sValue) const; diff --git a/sax/source/expatwrap/saxwriter.cxx b/sax/source/expatwrap/saxwriter.cxx index 2fe579648ce5..8566cab2b7e6 100644 --- a/sax/source/expatwrap/saxwriter.cxx +++ b/sax/source/expatwrap/saxwriter.cxx @@ -38,6 +38,7 @@ #include <osl/diagnose.h> #include <rtl/character.hxx> +#include <sal/log.hxx> using namespace ::std; using namespace ::osl; @@ -579,6 +580,7 @@ void CheckValidName(OUString const& rName) if (c == ':') { // see https://www.w3.org/TR/REC-xml-names/#ns-qualnames + SAL_WARN_IF(hasColon, "sax", "only one colon allowed: " << rName); assert(!hasColon && "only one colon allowed"); hasColon = true; } @@ -593,6 +595,7 @@ void CheckValidName(OUString const& rName) { // https://www.w3.org/TR/xml11/#NT-NameChar // (currently we don't warn about invalid start chars) + SAL_WARN("sax", "unexpected character in attribute name: " << rName); assert(!"unexpected character in attribute name"); } } diff --git a/sw/source/filter/xml/xmlimpit.cxx b/sw/source/filter/xml/xmlimpit.cxx index 023eae8dc25b..be7ffaa95b8f 100644 --- a/sw/source/filter/xml/xmlimpit.cxx +++ b/sw/source/filter/xml/xmlimpit.cxx @@ -218,7 +218,15 @@ void SvXMLImportItemMapper::importXMLUnknownAttributes( SfxItemSet& rSet, pUnknownItem->AddAttr( rAttribute.Name, rAttribute.Value ); else { - pUnknownItem->AddAttr( rAttribute.Name, rAttribute.NamespaceURL, rAttribute.Name, + OUString sPrefix; + OUString sName = rAttribute.Name; + int i = sName.indexOf(':'); + if (i != -1) + { + sPrefix = sName.copy(0, i-1); + sName = sName.copy(i+1); + } + pUnknownItem->AddAttr( sPrefix, rAttribute.NamespaceURL, sName, rAttribute.Value ); } } diff --git a/xmloff/source/core/attrlist.cxx b/xmloff/source/core/attrlist.cxx index c65b9a3cb4b1..7a673bcdaad2 100644 --- a/xmloff/source/core/attrlist.cxx +++ b/xmloff/source/core/attrlist.cxx @@ -143,6 +143,8 @@ SvXMLAttributeList::~SvXMLAttributeList() void SvXMLAttributeList::AddAttribute( const OUString &sName , const OUString &sValue ) { + assert( !sName.isEmpty() && "empty attribute name is invalid"); + assert( std::count(sName.getStr(), sName.getStr() + sName.getLength(), u':') <= 1 && "too many colons"); m_pImpl->vecAttribute.emplace_back( sName , sValue ); } @@ -172,9 +174,10 @@ void SvXMLAttributeList::AppendAttributeList( const uno::Reference< css::xml::sa m_pImpl->vecAttribute.reserve( nTotalSize ); for( sal_Int16 i = 0 ; i < nMax ; ++i ) { - m_pImpl->vecAttribute.emplace_back( - r->getNameByIndex( i ) , - r->getValueByIndex( i )); + OUString sName = r->getNameByIndex( i ); + assert( !sName.isEmpty() && "empty attribute name is invalid"); + assert( std::count(sName.getStr(), sName.getStr() + sName.getLength(), u':') <= 1 && "too many colons"); + m_pImpl->vecAttribute.emplace_back(sName, r->getValueByIndex( i )); } OSL_ASSERT( nTotalSize == static_cast<SvXMLAttributeList_Impl::size_type>(getLength())); diff --git a/xmloff/source/core/xmlcnimp.cxx b/xmloff/source/core/xmlcnimp.cxx index f9cc065320a6..b38752e2d213 100644 --- a/xmloff/source/core/xmlcnimp.cxx +++ b/xmloff/source/core/xmlcnimp.cxx @@ -44,6 +44,8 @@ bool SvXMLAttrContainerData::operator ==( const SvXMLAttrContainerData& rCmp ) c bool SvXMLAttrContainerData::AddAttr( const OUString& rLName, const OUString& rValue ) { + assert( !rLName.isEmpty() && "empty attribute name is invalid"); + assert( rLName.indexOf(':') == -1 && "colon in name?"); return pimpl->AddAttr(rLName, rValue); } @@ -52,6 +54,9 @@ bool SvXMLAttrContainerData::AddAttr( const OUString& rPrefix, const OUString& rLName, const OUString& rValue ) { + assert( !rLName.isEmpty() && "empty attribute name is invalid"); + assert( rPrefix.indexOf(':') == -1 && "colon in prefix?"); + assert( rLName.indexOf(':') == -1 && "colon in name?"); return pimpl->AddAttr(rPrefix, rNamespace, rLName, rValue); } @@ -59,6 +64,9 @@ bool SvXMLAttrContainerData::AddAttr( const OUString& rPrefix, const OUString& rLName, const OUString& rValue ) { + assert( !rLName.isEmpty() && "empty attribute name is invalid"); + assert( rPrefix.indexOf(':') == -1 && "colon in prefix?"); + assert( rLName.indexOf(':') == -1 && "colon in name?"); return pimpl->AddAttr(rPrefix, rLName, rValue); } @@ -66,6 +74,8 @@ bool SvXMLAttrContainerData::SetAt( size_t i, const OUString& rLName, const OUString& rValue ) { + assert( !rLName.isEmpty() && "empty attribute name is invalid"); + assert( rLName.indexOf(':') == -1 && "colon in name?"); return pimpl->SetAt(i, rLName, rValue); } @@ -75,6 +85,9 @@ bool SvXMLAttrContainerData::SetAt( size_t i, const OUString& rLName, const OUString& rValue ) { + assert( !rLName.isEmpty() && "empty attribute name is invalid"); + assert( rPrefix.indexOf(':') == -1 && "colon in prefix?"); + assert( rLName.indexOf(':') == -1 && "colon in name?"); return pimpl->SetAt(i, rPrefix, rNamespace, rLName, rValue); } @@ -83,6 +96,9 @@ bool SvXMLAttrContainerData::SetAt( size_t i, const OUString& rLName, const OUString& rValue ) { + assert( !rLName.isEmpty() && "empty attribute name is invalid"); + assert( rPrefix.indexOf(':') == -1 && "colon in prefix?"); + assert( rLName.indexOf(':') == -1 && "colon in name?"); return pimpl->SetAt(i, rPrefix, rLName, rValue); } diff --git a/xmloff/source/style/xmlimppr.cxx b/xmloff/source/style/xmlimppr.cxx index 8607e9c9fa2d..cf81dfcd1e1c 100644 --- a/xmloff/source/style/xmlimppr.cxx +++ b/xmloff/source/style/xmlimppr.cxx @@ -136,7 +136,7 @@ void SvXMLImportPropertyMapper::importXML( importXMLAttribute(rProperties, rUnitConverter, rNamespaceMap, nPropType, nStartIdx, nEndIdx, xAttrContainer, - aPrefix, sAttrName, aNamespaceURI, sValue); + sAttrName, aNamespaceURI, sValue); } const css::uno::Sequence< css::xml::Attribute > unknownAttribs = xAttrList->getUnknownAttributes(); @@ -145,11 +145,16 @@ void SvXMLImportPropertyMapper::importXML( OUString aPrefix; int nSepIndex = rAttribute.Name.indexOf(SvXMLImport::aNamespaceSeparator); if (nSepIndex != -1) + { + // If it's an unknown attribute in a known namespace, ignore it. aPrefix = rAttribute.Name.copy(0, nSepIndex); + if (rNamespaceMap.GetKeyByPrefix(aPrefix) != USHRT_MAX) + continue; + } importXMLAttribute(rProperties, rUnitConverter, rNamespaceMap, nPropType, nStartIdx, nEndIdx, xAttrContainer, - aPrefix, rAttribute.Name, rAttribute.NamespaceURL, rAttribute.Value); + rAttribute.Name, rAttribute.NamespaceURL, rAttribute.Value); } finished( rProperties, nStartIdx, nEndIdx ); @@ -163,13 +168,13 @@ void SvXMLImportPropertyMapper::importXMLAttribute( const sal_Int32 nStartIdx, const sal_Int32 nEndIdx, Reference< XNameContainer >& xAttrContainer, - const OUString& aPrefix, - const OUString& sAttrName, + const OUString& rAttrName, const OUString& aNamespaceURI, const OUString& sValue) const { - OUString aLocalName; - sal_uInt16 nPrefix = GetImport().GetNamespaceMap().GetKeyByAttrName( sAttrName, &aLocalName ); + OUString aLocalName, aPrefix, aNamespace; + sal_uInt16 nPrefix = rNamespaceMap.GetKeyByAttrName( rAttrName, &aPrefix, + &aLocalName, &aNamespace ); // index of actual property map entry // This looks very strange, but it works well: @@ -262,7 +267,7 @@ void SvXMLImportPropertyMapper::importXMLAttribute( ((nFlags & MID_FLAG_MULTI_PROPERTY) == 0) ) { Sequence<OUString> aSeq(2); - aSeq[0] = sAttrName; + aSeq[0] = rAttrName; aSeq[1] = sValue; rImport.SetError( XMLERROR_FLAG_WARNING | XMLERROR_STYLE_ATTR_VALUE, @@ -279,7 +284,7 @@ void SvXMLImportPropertyMapper::importXMLAttribute( SAL_INFO_IF((XML_NAMESPACE_NONE != nPrefix) && !(XML_NAMESPACE_UNKNOWN_FLAG & nPrefix), "xmloff.style", - "unknown attribute: \"" << sAttrName << "\""); + "unknown attribute: \"" << rAttrName << "\""); if( (XML_NAMESPACE_UNKNOWN_FLAG & nPrefix) || (XML_NAMESPACE_NONE == nPrefix) ) { if( !xAttrContainer.is() ) @@ -325,18 +330,15 @@ void SvXMLImportPropertyMapper::importXMLAttribute( AttributeData aData; aData.Type = GetXMLToken( XML_CDATA ); aData.Value = sValue; - - OUStringBuffer sName; + OUString sName; if( XML_NAMESPACE_NONE != nPrefix ) { - sName.append( aPrefix ); - sName.append( ':' ); + sName = rAttrName; aData.Namespace = aNamespaceURI; } - - sName.append( aLocalName ); - - xAttrContainer->insertByName( sName.makeStringAndClear(), Any(aData) ); + else + sName = aLocalName; + xAttrContainer->insertByName( sName, Any(aData) ); } } } _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits