Fix crashtest fdo77855.odt

regression from commit
    commit 9814c1f2ed
    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>
This commit is contained in:
Noel Grandin
2020-09-03 13:42:40 +02:00
parent 8db4b89e6b
commit 12a7a3d57d
6 changed files with 52 additions and 21 deletions

View File

@@ -185,7 +185,6 @@ private:
sal_Int32 nStartIdx, sal_Int32 nStartIdx,
sal_Int32 nEndIdx, sal_Int32 nEndIdx,
css::uno::Reference< css::container::XNameContainer >& xAttrContainer, css::uno::Reference< css::container::XNameContainer >& xAttrContainer,
const OUString& aPrefix,
const OUString& sAttrName, const OUString& sAttrName,
const OUString& aNamespaceURI, const OUString& aNamespaceURI,
const OUString& sValue) const; const OUString& sValue) const;

View File

@@ -38,6 +38,7 @@
#include <osl/diagnose.h> #include <osl/diagnose.h>
#include <rtl/character.hxx> #include <rtl/character.hxx>
#include <sal/log.hxx>
using namespace ::std; using namespace ::std;
using namespace ::osl; using namespace ::osl;
@@ -579,6 +580,7 @@ void CheckValidName(OUString const& rName)
if (c == ':') if (c == ':')
{ {
// see https://www.w3.org/TR/REC-xml-names/#ns-qualnames // 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"); assert(!hasColon && "only one colon allowed");
hasColon = true; hasColon = true;
} }
@@ -593,6 +595,7 @@ void CheckValidName(OUString const& rName)
{ {
// https://www.w3.org/TR/xml11/#NT-NameChar // https://www.w3.org/TR/xml11/#NT-NameChar
// (currently we don't warn about invalid start chars) // (currently we don't warn about invalid start chars)
SAL_WARN("sax", "unexpected character in attribute name: " << rName);
assert(!"unexpected character in attribute name"); assert(!"unexpected character in attribute name");
} }
} }

View File

@@ -218,7 +218,15 @@ void SvXMLImportItemMapper::importXMLUnknownAttributes( SfxItemSet& rSet,
pUnknownItem->AddAttr( rAttribute.Name, rAttribute.Value ); pUnknownItem->AddAttr( rAttribute.Name, rAttribute.Value );
else 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 ); rAttribute.Value );
} }
} }

View File

@@ -143,6 +143,8 @@ SvXMLAttributeList::~SvXMLAttributeList()
void SvXMLAttributeList::AddAttribute( const OUString &sName , void SvXMLAttributeList::AddAttribute( const OUString &sName ,
const OUString &sValue ) 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 ); 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 ); m_pImpl->vecAttribute.reserve( nTotalSize );
for( sal_Int16 i = 0 ; i < nMax ; ++i ) { for( sal_Int16 i = 0 ; i < nMax ; ++i ) {
m_pImpl->vecAttribute.emplace_back( OUString sName = r->getNameByIndex( i );
r->getNameByIndex( i ) , assert( !sName.isEmpty() && "empty attribute name is invalid");
r->getValueByIndex( i )); 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())); OSL_ASSERT( nTotalSize == static_cast<SvXMLAttributeList_Impl::size_type>(getLength()));

View File

@@ -44,6 +44,8 @@ bool SvXMLAttrContainerData::operator ==( const SvXMLAttrContainerData& rCmp ) c
bool SvXMLAttrContainerData::AddAttr( const OUString& rLName, bool SvXMLAttrContainerData::AddAttr( const OUString& rLName,
const OUString& rValue ) const OUString& rValue )
{ {
assert( !rLName.isEmpty() && "empty attribute name is invalid");
assert( rLName.indexOf(':') == -1 && "colon in name?");
return pimpl->AddAttr(rLName, rValue); return pimpl->AddAttr(rLName, rValue);
} }
@@ -52,6 +54,9 @@ bool SvXMLAttrContainerData::AddAttr( const OUString& rPrefix,
const OUString& rLName, const OUString& rLName,
const OUString& rValue ) 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); return pimpl->AddAttr(rPrefix, rNamespace, rLName, rValue);
} }
@@ -59,6 +64,9 @@ bool SvXMLAttrContainerData::AddAttr( const OUString& rPrefix,
const OUString& rLName, const OUString& rLName,
const OUString& rValue ) 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); return pimpl->AddAttr(rPrefix, rLName, rValue);
} }
@@ -66,6 +74,8 @@ bool SvXMLAttrContainerData::SetAt( size_t i,
const OUString& rLName, const OUString& rLName,
const OUString& rValue ) const OUString& rValue )
{ {
assert( !rLName.isEmpty() && "empty attribute name is invalid");
assert( rLName.indexOf(':') == -1 && "colon in name?");
return pimpl->SetAt(i, rLName, rValue); return pimpl->SetAt(i, rLName, rValue);
} }
@@ -75,6 +85,9 @@ bool SvXMLAttrContainerData::SetAt( size_t i,
const OUString& rLName, const OUString& rLName,
const OUString& rValue ) 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); return pimpl->SetAt(i, rPrefix, rNamespace, rLName, rValue);
} }
@@ -83,6 +96,9 @@ bool SvXMLAttrContainerData::SetAt( size_t i,
const OUString& rLName, const OUString& rLName,
const OUString& rValue ) 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); return pimpl->SetAt(i, rPrefix, rLName, rValue);
} }

View File

@@ -136,7 +136,7 @@ void SvXMLImportPropertyMapper::importXML(
importXMLAttribute(rProperties, rUnitConverter, rNamespaceMap, importXMLAttribute(rProperties, rUnitConverter, rNamespaceMap,
nPropType, nStartIdx, nEndIdx, xAttrContainer, nPropType, nStartIdx, nEndIdx, xAttrContainer,
aPrefix, sAttrName, aNamespaceURI, sValue); sAttrName, aNamespaceURI, sValue);
} }
const css::uno::Sequence< css::xml::Attribute > unknownAttribs = xAttrList->getUnknownAttributes(); const css::uno::Sequence< css::xml::Attribute > unknownAttribs = xAttrList->getUnknownAttributes();
@@ -145,11 +145,16 @@ void SvXMLImportPropertyMapper::importXML(
OUString aPrefix; OUString aPrefix;
int nSepIndex = rAttribute.Name.indexOf(SvXMLImport::aNamespaceSeparator); int nSepIndex = rAttribute.Name.indexOf(SvXMLImport::aNamespaceSeparator);
if (nSepIndex != -1) if (nSepIndex != -1)
{
// If it's an unknown attribute in a known namespace, ignore it.
aPrefix = rAttribute.Name.copy(0, nSepIndex); aPrefix = rAttribute.Name.copy(0, nSepIndex);
if (rNamespaceMap.GetKeyByPrefix(aPrefix) != USHRT_MAX)
continue;
}
importXMLAttribute(rProperties, rUnitConverter, rNamespaceMap, importXMLAttribute(rProperties, rUnitConverter, rNamespaceMap,
nPropType, nStartIdx, nEndIdx, xAttrContainer, nPropType, nStartIdx, nEndIdx, xAttrContainer,
aPrefix, rAttribute.Name, rAttribute.NamespaceURL, rAttribute.Value); rAttribute.Name, rAttribute.NamespaceURL, rAttribute.Value);
} }
finished( rProperties, nStartIdx, nEndIdx ); finished( rProperties, nStartIdx, nEndIdx );
@@ -163,13 +168,13 @@ void SvXMLImportPropertyMapper::importXMLAttribute(
const sal_Int32 nStartIdx, const sal_Int32 nStartIdx,
const sal_Int32 nEndIdx, const sal_Int32 nEndIdx,
Reference< XNameContainer >& xAttrContainer, Reference< XNameContainer >& xAttrContainer,
const OUString& aPrefix, const OUString& rAttrName,
const OUString& sAttrName,
const OUString& aNamespaceURI, const OUString& aNamespaceURI,
const OUString& sValue) const const OUString& sValue) const
{ {
OUString aLocalName; OUString aLocalName, aPrefix, aNamespace;
sal_uInt16 nPrefix = GetImport().GetNamespaceMap().GetKeyByAttrName( sAttrName, &aLocalName ); sal_uInt16 nPrefix = rNamespaceMap.GetKeyByAttrName( rAttrName, &aPrefix,
&aLocalName, &aNamespace );
// index of actual property map entry // index of actual property map entry
// This looks very strange, but it works well: // This looks very strange, but it works well:
@@ -262,7 +267,7 @@ void SvXMLImportPropertyMapper::importXMLAttribute(
((nFlags & MID_FLAG_MULTI_PROPERTY) == 0) ) ((nFlags & MID_FLAG_MULTI_PROPERTY) == 0) )
{ {
Sequence<OUString> aSeq(2); Sequence<OUString> aSeq(2);
aSeq[0] = sAttrName; aSeq[0] = rAttrName;
aSeq[1] = sValue; aSeq[1] = sValue;
rImport.SetError( XMLERROR_FLAG_WARNING | rImport.SetError( XMLERROR_FLAG_WARNING |
XMLERROR_STYLE_ATTR_VALUE, XMLERROR_STYLE_ATTR_VALUE,
@@ -279,7 +284,7 @@ void SvXMLImportPropertyMapper::importXMLAttribute(
SAL_INFO_IF((XML_NAMESPACE_NONE != nPrefix) && SAL_INFO_IF((XML_NAMESPACE_NONE != nPrefix) &&
!(XML_NAMESPACE_UNKNOWN_FLAG & nPrefix), !(XML_NAMESPACE_UNKNOWN_FLAG & nPrefix),
"xmloff.style", "xmloff.style",
"unknown attribute: \"" << sAttrName << "\""); "unknown attribute: \"" << rAttrName << "\"");
if( (XML_NAMESPACE_UNKNOWN_FLAG & nPrefix) || (XML_NAMESPACE_NONE == nPrefix) ) if( (XML_NAMESPACE_UNKNOWN_FLAG & nPrefix) || (XML_NAMESPACE_NONE == nPrefix) )
{ {
if( !xAttrContainer.is() ) if( !xAttrContainer.is() )
@@ -325,18 +330,15 @@ void SvXMLImportPropertyMapper::importXMLAttribute(
AttributeData aData; AttributeData aData;
aData.Type = GetXMLToken( XML_CDATA ); aData.Type = GetXMLToken( XML_CDATA );
aData.Value = sValue; aData.Value = sValue;
OUString sName;
OUStringBuffer sName;
if( XML_NAMESPACE_NONE != nPrefix ) if( XML_NAMESPACE_NONE != nPrefix )
{ {
sName.append( aPrefix ); sName = rAttrName;
sName.append( ':' );
aData.Namespace = aNamespaceURI; aData.Namespace = aNamespaceURI;
} }
else
sName.append( aLocalName ); sName = aLocalName;
xAttrContainer->insertByName( sName, Any(aData) );
xAttrContainer->insertByName( sName.makeStringAndClear(), Any(aData) );
} }
} }
} }