diff options
author | Noel Grandin <noel.grandin@collabora.co.uk> | 2020-09-03 13:42:40 +0200 |
---|---|---|
committer | Noel Grandin <noel.grandin@collabora.co.uk> | 2020-09-03 15:33:36 +0200 |
commit | 12a7a3d57d3dcf222b13fa9143c37736f8ea3d0b (patch) | |
tree | 0cf118576f8e313fd5321e2ee49b73546c79b2ae | |
parent | 8db4b89e6b4a0f6be6e1040aec9fd9c0c0bdd608 (diff) |
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.grandin@collabora.co.uk>
-rw-r--r-- | include/xmloff/xmlimppr.hxx | 1 | ||||
-rw-r--r-- | sax/source/expatwrap/saxwriter.cxx | 3 | ||||
-rw-r--r-- | sw/source/filter/xml/xmlimpit.cxx | 10 | ||||
-rw-r--r-- | xmloff/source/core/attrlist.cxx | 9 | ||||
-rw-r--r-- | xmloff/source/core/xmlcnimp.cxx | 16 | ||||
-rw-r--r-- | xmloff/source/style/xmlimppr.cxx | 34 |
6 files changed, 52 insertions, 21 deletions
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) ); } } } |