From 8d5779bef3cb36f0db6f61b6a9f711796d487d9d Mon Sep 17 00:00:00 2001 From: Arkadiy Illarionov Date: Tue, 11 Sep 2018 23:18:20 +0300 Subject: Simplify containers iterations in xmlsecurity Use range-based loop or replace with functions from std algorithm. Change-Id: I0146186b7c42405076dfce7de7805be4228cc6d3 Reviewed-on: https://gerrit.libreoffice.org/60360 Tested-by: Jenkins Reviewed-by: Noel Grandin --- xmlsecurity/source/framework/buffernode.cxx | 80 ++++++---------------- .../source/framework/saxeventkeeperimpl.cxx | 70 ++++++++----------- xmlsecurity/source/framework/signatureengine.cxx | 14 ++-- .../source/helper/documentsignaturehelper.cxx | 67 ++++++------------ .../mscrypt/securityenvironment_mscryptimpl.cxx | 8 +-- .../xmlsec/nss/securityenvironment_nssimpl.cxx | 45 +++++------- 6 files changed, 99 insertions(+), 185 deletions(-) (limited to 'xmlsecurity/source') diff --git a/xmlsecurity/source/framework/buffernode.cxx b/xmlsecurity/source/framework/buffernode.cxx index fb58e7b6232a..d1cd8d17ed44 100644 --- a/xmlsecurity/source/framework/buffernode.cxx +++ b/xmlsecurity/source/framework/buffernode.cxx @@ -803,35 +803,18 @@ bool BufferNode::isECInSubTreeIncluded(sal_Int32 nIgnoredSecurityId) const * bExist - true if a match found, false otherwise. ******************************************************************************/ { - bool rc = false; - - std::vector< const ElementCollector* >::const_iterator jj = m_vElementCollectors.begin(); - - for( ; jj != m_vElementCollectors.end() ; ++jj ) - { - ElementCollector* pElementCollector = const_cast(*jj); - if (nIgnoredSecurityId == cssxc::sax::ConstOfSecurityId::UNDEFINEDSECURITYID || - pElementCollector->getSecurityId() != nIgnoredSecurityId) - { - rc = true; - break; - } - } + bool rc = std::any_of(m_vElementCollectors.begin(), m_vElementCollectors.end(), + [nIgnoredSecurityId](const ElementCollector* pElementCollector) { + return nIgnoredSecurityId == cssxc::sax::ConstOfSecurityId::UNDEFINEDSECURITYID || + pElementCollector->getSecurityId() != nIgnoredSecurityId; + }); if ( !rc ) { - std::vector< const BufferNode* >::const_iterator ii = m_vChildren.begin(); - - for( ; ii != m_vChildren.end() ; ++ii ) - { - BufferNode* pBufferNode = const_cast(*ii); - - if ( pBufferNode->isECInSubTreeIncluded(nIgnoredSecurityId)) - { - rc = true; - break; - } - } + rc = std::any_of(m_vChildren.begin(), m_vChildren.end(), + [nIgnoredSecurityId](const BufferNode* pBufferNode) { + return pBufferNode->isECInSubTreeIncluded(nIgnoredSecurityId); + }); } return rc; @@ -904,31 +887,14 @@ bool BufferNode::isBlockerInSubTreeIncluded(sal_Int32 nIgnoredSecurityId) const * bExist - true if a match found, false otherwise. ******************************************************************************/ { - bool rc = false; - - std::vector< const BufferNode* >::const_iterator ii = m_vChildren.begin(); - - for( ; ii != m_vChildren.end() ; ++ii ) - { - BufferNode* pBufferNode = const_cast(*ii); - ElementMark* pBlocker = pBufferNode->getBlocker(); - - if (pBlocker != nullptr && - (nIgnoredSecurityId == cssxc::sax::ConstOfSecurityId::UNDEFINEDSECURITYID || - pBlocker->getSecurityId() != nIgnoredSecurityId )) - { - rc = true; - break; - } - - if (rc || pBufferNode->isBlockerInSubTreeIncluded(nIgnoredSecurityId)) - { - rc = true; - break; - } - } - - return rc; + return std::any_of(m_vChildren.begin(), m_vChildren.end(), + [nIgnoredSecurityId](const BufferNode* pBufferNode) { + ElementMark* pBlocker = pBufferNode->getBlocker(); + return (pBlocker != nullptr && + (nIgnoredSecurityId == cssxc::sax::ConstOfSecurityId::UNDEFINEDSECURITYID || + pBlocker->getSecurityId() != nIgnoredSecurityId )) || + pBufferNode->isBlockerInSubTreeIncluded(nIgnoredSecurityId); + }); } const BufferNode* BufferNode::getNextChild(const BufferNode* pChild) const @@ -954,16 +920,15 @@ const BufferNode* BufferNode::getNextChild(const BufferNode* pChild) const BufferNode* rc = nullptr; bool bChildFound = false; - std::vector< const BufferNode* >::const_iterator ii = m_vChildren.begin(); - for( ; ii != m_vChildren.end() ; ++ii ) + for( const BufferNode* i : m_vChildren ) { if (bChildFound) { - rc = const_cast(*ii); + rc = const_cast(i); break; } - if( *ii == pChild ) + if( i == pChild ) { bChildFound = true; } @@ -992,10 +957,9 @@ void BufferNode::freeAllChildren() * empty ******************************************************************************/ { - std::vector< const BufferNode* >::const_iterator ii = m_vChildren.begin(); - for( ; ii != m_vChildren.end() ; ++ii ) + for( const BufferNode* i : m_vChildren ) { - BufferNode *pChild = const_cast(*ii); + BufferNode *pChild = const_cast(i); pChild->freeAllChildren(); delete pChild; } diff --git a/xmlsecurity/source/framework/saxeventkeeperimpl.cxx b/xmlsecurity/source/framework/saxeventkeeperimpl.cxx index 5fea1e76fe3b..2c60c950f605 100644 --- a/xmlsecurity/source/framework/saxeventkeeperimpl.cxx +++ b/xmlsecurity/source/framework/saxeventkeeperimpl.cxx @@ -176,10 +176,9 @@ BufferNode* SAXEventKeeperImpl::addNewElementMarkBuffers() if (!m_vNewElementCollectors.empty()) { - for( auto ii = m_vNewElementCollectors.begin(); - ii != m_vNewElementCollectors.end(); ++ii ) + for( const auto& i : m_vNewElementCollectors ) { - pBufferNode->addElementCollector(*ii); + pBufferNode->addElementCollector(i); } m_vNewElementCollectors.clear(); @@ -240,36 +239,30 @@ void SAXEventKeeperImpl::removeElementMarkBuffer(sal_Int32 nId) * nId - the Id of the ElementMark to be removed. ******************************************************************************/ { - for( auto ii = m_vElementMarkBuffers.begin(); - ii != m_vElementMarkBuffers.end(); ++ii ) - { - if ( nId == (*ii)->getBufferId()) - { - /* - * checks whether this ElementMark still in the new ElementCollect array - */ - for( auto jj = m_vNewElementCollectors.begin(); - jj != m_vNewElementCollectors.end(); ++jj ) - { - if (ii->get() == (*jj)) - { - m_vNewElementCollectors.erase(jj); - break; - } - } + auto ii = std::find_if(m_vElementMarkBuffers.begin(), m_vElementMarkBuffers.end(), + [nId](std::unique_ptr& rElementMark) { return nId == rElementMark->getBufferId(); } + ); + if (ii == m_vElementMarkBuffers.end()) + return; - /* - * checks whether this ElementMark is the new Blocker - */ - if (ii->get() == m_pNewBlocker) - { - m_pNewBlocker = nullptr; - } + /* + * checks whether this ElementMark still in the new ElementCollect array + */ + auto jj = std::find_if(m_vNewElementCollectors.begin(), m_vNewElementCollectors.end(), + [&ii](const ElementCollector* pElementCollector) { return ii->get() == pElementCollector; } + ); + if (jj != m_vNewElementCollectors.end()) + m_vNewElementCollectors.erase(jj); - m_vElementMarkBuffers.erase( ii ); - break; - } + /* + * checks whether this ElementMark is the new Blocker + */ + if (ii->get() == m_pNewBlocker) + { + m_pNewBlocker = nullptr; } + + m_vElementMarkBuffers.erase( ii ); } OUString SAXEventKeeperImpl::printBufferNode( @@ -338,10 +331,9 @@ OUString SAXEventKeeperImpl::printBufferNode( rc.append("\n"); std::vector< const BufferNode* >* vChildren = pBufferNode->getChildren(); - for( auto jj = vChildren->begin(); - jj != vChildren->end(); ++jj ) + for( const auto& jj : *vChildren ) { - rc.append(printBufferNode(*jj, nIndent+4)); + rc.append(printBufferNode(jj, nIndent+4)); } delete vChildren; @@ -373,10 +365,9 @@ cssu::Sequence< cssu::Reference< cssxw::XXMLElementWrapper > > cssxw::XXMLElementWrapper > > aChildrenCollection ( vChildren->size()); sal_Int32 nIndex = 0; - for( auto ii = vChildren->begin(); - ii != vChildren->end(); ++ii ) + for( const auto& i : *vChildren ) { - aChildrenCollection[nIndex] = (*ii)->getXMLElement(); + aChildrenCollection[nIndex] = i->getXMLElement(); nIndex++; } @@ -515,11 +506,10 @@ void SAXEventKeeperImpl::smashBufferNode( pParent->removeChild(pBufferNode); pBufferNode->setParent(nullptr); - for( auto ii = vChildren->begin(); - ii != vChildren->end(); ++ii ) + for( const auto& i : *vChildren ) { - const_cast(*ii)->setParent(pParent); - pParent->addChild(*ii, nIndex); + const_cast(i)->setParent(pParent); + pParent->addChild(i, nIndex); nIndex++; } diff --git a/xmlsecurity/source/framework/signatureengine.cxx b/xmlsecurity/source/framework/signatureengine.cxx index bffcb68bad51..b5a872deb168 100644 --- a/xmlsecurity/source/framework/signatureengine.cxx +++ b/xmlsecurity/source/framework/signatureengine.cxx @@ -98,11 +98,9 @@ void SignatureEngine::tryToPerform( ) xSignatureTemplate->setTemplate(xXMLElement); - std::vector< sal_Int32 >::const_iterator ii = m_vReferenceIds.begin(); - - for( ; ii != m_vReferenceIds.end() ; ++ii ) + for( const auto i : m_vReferenceIds ) { - xXMLElement = m_xSAXEventKeeper->getElement( *ii ); + xXMLElement = m_xSAXEventKeeper->getElement( i ); xSignatureTemplate->setTarget(xXMLElement); } @@ -145,14 +143,12 @@ void SignatureEngine::clearUp( ) const m_xSAXEventKeeper->removeElementCollector(m_nIdOfTemplateEC); - std::vector< sal_Int32 >::const_iterator ii = m_vReferenceIds.begin(); - - for( ; ii != m_vReferenceIds.end() ; ++ii ) + for( const auto& i : m_vReferenceIds ) { xReferenceResolvedBroadcaster->removeReferenceResolvedListener( - *ii, + i, static_cast >(static_cast(const_cast(this)))); - m_xSAXEventKeeper->removeElementCollector(*ii); + m_xSAXEventKeeper->removeElementCollector(i); } if (m_nIdOfKeyEC != 0 && m_nIdOfKeyEC != -1) diff --git a/xmlsecurity/source/helper/documentsignaturehelper.cxx b/xmlsecurity/source/helper/documentsignaturehelper.cxx index db05059b31df..e4961377099a 100644 --- a/xmlsecurity/source/helper/documentsignaturehelper.cxx +++ b/xmlsecurity/source/helper/documentsignaturehelper.cxx @@ -21,6 +21,7 @@ #include #include +#include #include #include @@ -424,40 +425,25 @@ bool DocumentSignatureHelper::checkIfAllFilesAreSigned( { // Can only be valid if ALL streams are signed, which means real stream count == signed stream count unsigned int nRealCount = 0; + std::function fEncode = [](const OUString& rStr) { return rStr; }; + if (alg == DocumentSignatureAlgorithm::OOo2) + //Comparing URIs is a difficult. Therefore we kind of normalize + //it before comparing. We assume that our URI do not have a leading "./" + //and fragments at the end (...#...) + fEncode = [](const OUString& rStr) { + return rtl::Uri::encode(rStr, rtl_UriCharClassPchar, rtl_UriEncodeCheckEscapes, RTL_TEXTENCODING_UTF8); + }; + for ( int i = sigInfo.vSignatureReferenceInfors.size(); i; ) { const SignatureReferenceInformation& rInf = sigInfo.vSignatureReferenceInfors[--i]; // There is also an extra entry of type SignatureReferenceType::SAMEDOCUMENT because of signature date. if ( ( rInf.nType == SignatureReferenceType::BINARYSTREAM ) || ( rInf.nType == SignatureReferenceType::XMLSTREAM ) ) { - OUString sReferenceURI = rInf.ouURI; - if (alg == DocumentSignatureAlgorithm::OOo2) - { - //Comparing URIs is a difficult. Therefore we kind of normalize - //it before comparing. We assume that our URI do not have a leading "./" - //and fragments at the end (...#...) - sReferenceURI = ::rtl::Uri::encode( - sReferenceURI, rtl_UriCharClassPchar, - rtl_UriEncodeCheckEscapes, RTL_TEXTENCODING_UTF8); - } - //find the file in the element list - for (auto aIter = sElementList.cbegin(); aIter != sElementList.cend(); ++aIter) - { - OUString sElementListURI = *aIter; - if (alg == DocumentSignatureAlgorithm::OOo2) - { - sElementListURI = - ::rtl::Uri::encode( - sElementListURI, rtl_UriCharClassPchar, - rtl_UriEncodeCheckEscapes, RTL_TEXTENCODING_UTF8); - } - if (sElementListURI == sReferenceURI) - { - nRealCount++; - break; - } - } + if (std::any_of(sElementList.cbegin(), sElementList.cend(), + [&fEncode, &rInf](const OUString& rElement) { return fEncode(rElement) == fEncode(rInf.ouURI); })) + nRealCount++; } } return sElementList.size() == nRealCount; @@ -470,7 +456,6 @@ bool DocumentSignatureHelper::checkIfAllFilesAreSigned( bool DocumentSignatureHelper::equalsReferenceUriManifestPath( const OUString & rUri, const OUString & rPath) { - bool retVal = false; //split up the uri and path into segments. Both are separated by '/' std::vector vUriSegments; sal_Int32 nIndex = 0; @@ -490,25 +475,17 @@ bool DocumentSignatureHelper::equalsReferenceUriManifestPath( } while (nIndex >= 0); + if (vUriSegments.size() != vPathSegments.size()) + return false; + //Now compare each segment of the uri with its counterpart from the path - if (vUriSegments.size() == vPathSegments.size()) - { - retVal = true; - for (auto i = vUriSegments.cbegin(), j = vPathSegments.cbegin(); - i != vUriSegments.cend(); ++i, ++j) - { + return std::equal( + vUriSegments.cbegin(), vUriSegments.cend(), vPathSegments.cbegin(), + [](const OUString& rUriSegment, const OUString& rPathSegment) { //Decode the uri segment, so that %20 becomes ' ', etc. - OUString sDecUri = ::rtl::Uri::decode( - *i, rtl_UriDecodeWithCharset, RTL_TEXTENCODING_UTF8); - if (sDecUri != *j) - { - retVal = false; - break; - } - } - } - - return retVal; + OUString sDecUri = rtl::Uri::decode(rUriSegment, rtl_UriDecodeWithCharset, RTL_TEXTENCODING_UTF8); + return sDecUri == rPathSegment; + }); } OUString DocumentSignatureHelper::GetDocumentContentSignatureDefaultStreamName() diff --git a/xmlsecurity/source/xmlsec/mscrypt/securityenvironment_mscryptimpl.cxx b/xmlsecurity/source/xmlsec/mscrypt/securityenvironment_mscryptimpl.cxx index 81617fc845c7..cc1ea218d844 100644 --- a/xmlsecurity/source/xmlsec/mscrypt/securityenvironment_mscryptimpl.cxx +++ b/xmlsecurity/source/xmlsec/mscrypt/securityenvironment_mscryptimpl.cxx @@ -384,12 +384,12 @@ uno::Sequence< uno::Reference < XCertificate > > SecurityEnvironment_MSCryptImpl length = certsList.size() ; if( length != 0 ) { - int i ; - std::list< X509Certificate_MSCryptImpl* >::iterator xcertIt ; + int i = 0; uno::Sequence< uno::Reference< XCertificate > > certSeq( length ) ; - for( i = 0, xcertIt = certsList.begin(); xcertIt != certsList.end(); ++xcertIt, ++i ) { - certSeq[i] = *xcertIt ; + for( const auto& rXCert : certsList ) { + certSeq[i] = rXCert ; + ++i; } return certSeq ; diff --git a/xmlsecurity/source/xmlsec/nss/securityenvironment_nssimpl.cxx b/xmlsecurity/source/xmlsec/nss/securityenvironment_nssimpl.cxx index 7e19c995ae88..7ccc70c85955 100644 --- a/xmlsecurity/source/xmlsec/nss/securityenvironment_nssimpl.cxx +++ b/xmlsecurity/source/xmlsec/nss/securityenvironment_nssimpl.cxx @@ -120,24 +120,18 @@ SecurityEnvironment_NssImpl::~SecurityEnvironment_NssImpl() { } if( !m_tSymKeyList.empty() ) { - std::list< PK11SymKey* >::iterator symKeyIt ; - - for( symKeyIt = m_tSymKeyList.begin() ; symKeyIt != m_tSymKeyList.end() ; ++symKeyIt ) - PK11_FreeSymKey( *symKeyIt ) ; + for( auto& symKey : m_tSymKeyList ) + PK11_FreeSymKey( symKey ) ; } if( !m_tPubKeyList.empty() ) { - std::list< SECKEYPublicKey* >::iterator pubKeyIt ; - - for( pubKeyIt = m_tPubKeyList.begin() ; pubKeyIt != m_tPubKeyList.end() ; ++pubKeyIt ) - SECKEY_DestroyPublicKey( *pubKeyIt ) ; + for( auto& pubKeyIt : m_tPubKeyList ) + SECKEY_DestroyPublicKey( pubKeyIt ) ; } if( !m_tPriKeyList.empty() ) { - std::list< SECKEYPrivateKey* >::iterator priKeyIt ; - - for( priKeyIt = m_tPriKeyList.begin() ; priKeyIt != m_tPriKeyList.end() ; ++priKeyIt ) - SECKEY_DestroyPrivateKey( *priKeyIt ) ; + for( auto& priKey : m_tPriKeyList ) + SECKEY_DestroyPrivateKey( priKey ) ; } } @@ -206,14 +200,10 @@ void SecurityEnvironment_NssImpl::setCertDb( CERTCertDBHandle* aCertDb ) { } void SecurityEnvironment_NssImpl::adoptSymKey( PK11SymKey* aSymKey ) { - std::list< PK11SymKey* >::iterator keyIt ; - if( aSymKey != nullptr ) { //First try to find the key in the list - for( keyIt = m_tSymKeyList.begin() ; keyIt != m_tSymKeyList.end() ; ++keyIt ) { - if( *keyIt == aSymKey ) - return ; - } + if (std::find(m_tSymKeyList.begin(), m_tSymKeyList.end(), aSymKey) != m_tSymKeyList.end()) + return; //If we do not find the key in the list, add a new node PK11SymKey* symkey = PK11_ReferenceSymKey( aSymKey ) ; @@ -325,10 +315,8 @@ SecurityEnvironment_NssImpl::getPersonalCertificates() //secondly, we try to find certificate from registered private keys. if( !m_tPriKeyList.empty() ) { - std::list< SECKEYPrivateKey* >::iterator priKeyIt ; - - for( priKeyIt = m_tPriKeyList.begin() ; priKeyIt != m_tPriKeyList.end() ; ++priKeyIt ) { - xcert = NssPrivKeyToXCert( *priKeyIt ) ; + for( const auto& priKey : m_tPriKeyList ) { + xcert = NssPrivKeyToXCert( priKey ) ; if( xcert != nullptr ) certsList.push_back( xcert ) ; } @@ -336,12 +324,12 @@ SecurityEnvironment_NssImpl::getPersonalCertificates() length = certsList.size() ; if( length != 0 ) { - int i ; - std::list< X509Certificate_NssImpl* >::iterator xcertIt ; + int i = 0; Sequence< Reference< XCertificate > > certSeq( length ) ; - for( i = 0, xcertIt = certsList.begin(); xcertIt != certsList.end(); ++xcertIt, ++i ) { - certSeq[i] = *xcertIt ; + for( const auto& rXCert : certsList ) { + certSeq[i] = rXCert ; + ++i; } return certSeq ; @@ -712,11 +700,10 @@ verifyCertificate( const Reference< csss::XCertificate >& aCert, } //Destroying the temporary certificates - std::vector::const_iterator cert_i; - for (cert_i = vecTmpNSSCertificates.begin(); cert_i != vecTmpNSSCertificates.end(); ++cert_i) + for (auto& tmpCert : vecTmpNSSCertificates) { SAL_INFO("xmlsecurity.xmlsec", "Destroying temporary certificate"); - CERT_DestroyCertificate(*cert_i); + CERT_DestroyCertificate(tmpCert); } return validity ; } -- cgit