[cpp-xmltooling] branch master updated: CPPXT-133 - Eliminate uses of getTextContent in DOM helpers

Scott Cantor cantor.2 at osu.edu
Wed Jul 11 14:17:22 EDT 2018


This is an automated email from the git hooks/post-receive script.

scantor pushed a commit to branch master
in repository cpp-xmltooling.

View the commit online:
http://git.shibboleth.net/view/?p=cpp-xmltooling.git;a=commit;h=c09d73442756783a0afa2fe8de78086f00b05151

The following commit(s) were added to refs/heads/master by this push:
       new  c09d734   CPPXT-133 - Eliminate uses of getTextContent in DOM helpers
c09d734 is described below

commit c09d73442756783a0afa2fe8de78086f00b05151
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Jul 11 14:16:17 2018 -0400

    CPPXT-133 - Eliminate uses of getTextContent in DOM helpers
    
    https://issues.shibboleth.net/jira/browse/CPPXT-133
---
 .../security/impl/AbstractPKIXTrustEngine.cpp      | 27 ++++----
 xmltooling/soap/impl/SOAPImpl.cpp                  |  9 ++-
 xmltooling/util/XMLHelper.cpp                      | 79 +++++++++++++++++++---
 xmltooling/util/XMLHelper.h                        | 18 ++++-
 4 files changed, 105 insertions(+), 28 deletions(-)

diff --git a/xmltooling/security/impl/AbstractPKIXTrustEngine.cpp b/xmltooling/security/impl/AbstractPKIXTrustEngine.cpp
index 52455aa..55db195 100644
--- a/xmltooling/security/impl/AbstractPKIXTrustEngine.cpp
+++ b/xmltooling/security/impl/AbstractPKIXTrustEngine.cpp
@@ -108,7 +108,7 @@ namespace xmltooling {
     static XMLCh policyMappingInhibit[] =   UNICODE_LITERAL_20(p,o,l,i,c,y,M,a,p,p,i,n,g,I,n,h,i,b,i,t);
     static XMLCh anyPolicyInhibit[] =       UNICODE_LITERAL_16(a,n,y,P,o,l,i,c,y,I,n,h,i,b,i,t);
     static XMLCh _PathValidator[] =         UNICODE_LITERAL_13(P,a,t,h,V,a,l,i,d,a,t,o,r);
-    static XMLCh PolicyOID[] =      	    UNICODE_LITERAL_9(P,o,l,i,c,y,O,I,D);
+    static XMLCh PolicyOID[] =              UNICODE_LITERAL_9(P,o,l,i,c,y,O,I,D);
     static XMLCh TrustedName[] =            UNICODE_LITERAL_11(T,r,u,s,t,e,d,N,a,m,e);
     static XMLCh type[] =                   UNICODE_LITERAL_4(t,y,p,e);
 };
@@ -122,10 +122,10 @@ AbstractPKIXTrustEngine::PKIXValidationInfoIterator::~PKIXValidationInfoIterator
 }
 
 AbstractPKIXTrustEngine::AbstractPKIXTrustEngine(const xercesc::DOMElement* e, bool deprecationSupport)
-	: TrustEngine(e, deprecationSupport),
-		m_checkRevocation(XMLHelper::getAttrString(e, nullptr, checkRevocation)),
-		m_policyMappingInhibit(XMLHelper::getAttrBool(e, false, policyMappingInhibit)),
-		m_anyPolicyInhibit(XMLHelper::getAttrBool(e, false, anyPolicyInhibit))
+    : TrustEngine(e, deprecationSupport),
+        m_checkRevocation(XMLHelper::getAttrString(e, nullptr, checkRevocation)),
+        m_policyMappingInhibit(XMLHelper::getAttrBool(e, false, policyMappingInhibit)),
+        m_anyPolicyInhibit(XMLHelper::getAttrBool(e, false, anyPolicyInhibit))
 {
     if (m_checkRevocation.empty() && deprecationSupport && XMLHelper::getAttrBool(e, false, fullCRLChain)) {
         Category::getInstance(XMLTOOLING_LOGCAT ".TrustEngine.PKIX").warn(
@@ -136,14 +136,15 @@ AbstractPKIXTrustEngine::AbstractPKIXTrustEngine(const xercesc::DOMElement* e, b
 
     xercesc::DOMElement* c = XMLHelper::getFirstChildElement(e);
     while (c) {
-        if (c->hasChildNodes()) {
-            auto_ptr_char v(c->getTextContent());
-            if (v.get() && *v.get()) {
-                if (XMLString::equals(c->getLocalName(), PolicyOID))
-                    m_policyOIDs.insert(v.get());
-                else if (XMLString::equals(c->getLocalName(), TrustedName))
-                    m_trustedNames.insert(v.get());
-            }
+        if (XMLString::equals(c->getLocalName(), PolicyOID)) {
+            auto_ptr_char v(XMLHelper::getTextContent(c));
+            if (v.get() && *v.get())
+                m_policyOIDs.insert(v.get());
+        }
+        else if (XMLString::equals(c->getLocalName(), TrustedName)) {
+            auto_ptr_char v(XMLHelper::getTextContent(c));
+            if (v.get() && *v.get())
+                m_trustedNames.insert(v.get());
         }
         else if (XMLString::equals(c->getLocalName(), _PathValidator)) {
             try {
diff --git a/xmltooling/soap/impl/SOAPImpl.cpp b/xmltooling/soap/impl/SOAPImpl.cpp
index f6b5bd6..45c7486 100644
--- a/xmltooling/soap/impl/SOAPImpl.cpp
+++ b/xmltooling/soap/impl/SOAPImpl.cpp
@@ -78,8 +78,13 @@ namespace {
         }
         
         const xmltooling::QName* getCode() const {
-            if (!m_qname && getDOM() && getDOM()->getTextContent()) {
-                m_qname = XMLHelper::getNodeValueAsQName(getDOM());
+            if (!m_qname && getDOM() && getDOM()->hasChildNodes()) {
+                try {
+                    m_qname = XMLHelper::getNodeValueAsQName(getDOM());
+                }
+                catch (std::exception&) {
+            	    // unbound prefix here will throw
+                }
             }
             return m_qname;
         }
diff --git a/xmltooling/util/XMLHelper.cpp b/xmltooling/util/XMLHelper.cpp
index 8eef3bf..f86af73 100644
--- a/xmltooling/util/XMLHelper.cpp
+++ b/xmltooling/util/XMLHelper.cpp
@@ -59,7 +59,7 @@ xmltooling::QName* XMLHelper::getXSIType(const DOMElement* e)
 {
     DOMAttr* attribute = e ? e->getAttributeNodeNS(xmlconstants::XSI_NS, type) : nullptr;
     if (attribute) {
-        const XMLCh* attributeValue = attribute->getTextContent();
+        const XMLCh* attributeValue = attribute->getNodeValue();
         if (attributeValue && *attributeValue) {
             int i;
             if ((i=XMLString::indexOf(attributeValue,chColon))>0) {
@@ -185,7 +185,19 @@ xmltooling::QName* XMLHelper::getNodeValueAsQName(const DOMNode* domNode)
     if (!domNode)
         return nullptr;
     
-    const XMLCh* value=domNode->getTextContent();
+    const XMLCh* value = nullptr;
+    XMLCh* ownedValue = nullptr;
+    
+    if (domNode->getNodeType() == DOMNode::ATTRIBUTE_NODE) {
+        value = domNode->getNodeValue();
+    }
+    else if (domNode->getNodeType() == DOMNode::ELEMENT_NODE) {
+        ownedValue = getWholeTextContent(static_cast<const DOMElement*>(domNode));
+        value = ownedValue;
+    }
+
+    ArrayJanitor<XMLCh> jan(ownedValue);
+
     if (!value || !*value)
         return nullptr;
 
@@ -194,22 +206,39 @@ xmltooling::QName* XMLHelper::getNodeValueAsQName(const DOMNode* domNode)
         XMLCh* prefix=new XMLCh[i+1];
         XMLString::subString(prefix,value,0,i);
         prefix[i]=chNull;
-        xmltooling::QName* ret=new xmltooling::QName(domNode->lookupNamespaceURI(prefix), value + i + 1, prefix);
-        delete[] prefix;
-        return ret;
+        ArrayJanitor<XMLCh> jan2(prefix);
+        const XMLCh* ns = domNode->lookupNamespaceURI(prefix);
+        if (!ns) {
+            auto_ptr_char temp(prefix);
+            throw XMLToolingException("Namespace prefix ($1) not declared in document.", params(1, temp.get()));
+        }
+        return new xmltooling::QName(ns, value + i + 1, prefix);
     }
     
     return new xmltooling::QName(domNode->lookupNamespaceURI(nullptr), value);
 }
 
-bool XMLHelper::getNodeValueAsBool(const xercesc::DOMNode* domNode, bool def)
+bool XMLHelper::getNodeValueAsBool(const DOMNode* domNode, bool def)
 {
     if (!domNode)
         return def;
-    const XMLCh* value = domNode->getNodeValue();
+
+    const XMLCh* value = nullptr;
+    XMLCh* ownedValue = nullptr;
+
+    if (domNode->getNodeType() == DOMNode::ATTRIBUTE_NODE) {
+        value = domNode->getNodeValue();
+    }
+    else if (domNode->getNodeType() == DOMNode::ELEMENT_NODE) {
+        ownedValue = getWholeTextContent(static_cast<const DOMElement*>(domNode));
+        value = ownedValue;
+    }
+
+    ArrayJanitor<XMLCh> jan(ownedValue);
+
     if (!value || !*value)
         return def;
-    if (*value == chLatin_t || *value == chDigit_1)
+    else if (*value == chLatin_t || *value == chDigit_1)
         return true;
     else if (*value == chLatin_f || *value == chDigit_0)
         return false;
@@ -227,16 +256,46 @@ DOMElement* XMLHelper::appendChildElement(DOMElement* parentElement, DOMElement*
     return childElement;
 }
 
-bool XMLHelper::isNodeNamed(const xercesc::DOMNode* n, const XMLCh* ns, const XMLCh* local)
+bool XMLHelper::isNodeNamed(const DOMNode* n, const XMLCh* ns, const XMLCh* local)
 {
     return (n && XMLString::equals(local,n->getLocalName()) && XMLString::equals(ns,n->getNamespaceURI()));
 }
 
+XMLCh* XMLHelper::getWholeTextContent(const DOMElement* e)
+{
+    XMLCh* buf = nullptr;
+    const DOMNode* child = e ? e->getFirstChild() : nullptr;
+    while (child) {
+        if (child->getNodeType() == DOMNode::TEXT_NODE || child->getNodeType() == DOMNode::CDATA_SECTION_NODE) {
+            if (child->getNodeValue()) {
+                if (buf) {
+                    XMLSize_t initialLen = buf ? XMLString::stringLen(buf) : 0;
+                    XMLCh* merged = new XMLCh[initialLen + XMLString::stringLen(child->getNodeValue()) + 1];
+                    XMLString::copyString(merged, buf);
+                    XMLString::catString(merged + initialLen, child->getNodeValue());
+                    delete[] buf;
+                    buf = merged;
+                }
+                else {
+                    buf = new XMLCh[XMLString::stringLen(child->getNodeValue()) + 1];
+                    XMLString::copyString(buf, child->getNodeValue());
+                }
+            }
+        }
+        else if (child->getNodeType() != DOMNode::COMMENT_NODE) {
+            break;
+        }
+        child = child->getNextSibling();
+    }
+
+    return buf;
+}
+
 const XMLCh* XMLHelper::getTextContent(const DOMElement* e)
 {
     DOMNode* child = e ? e->getFirstChild() : nullptr;
     while (child) {
-        if (child->getNodeType() == DOMNode::TEXT_NODE)
+        if (child->getNodeType() == DOMNode::TEXT_NODE || child->getNodeType() == DOMNode::CDATA_SECTION_NODE)
             return child->getNodeValue();
         child = child->getNextSibling();
     }
diff --git a/xmltooling/util/XMLHelper.h b/xmltooling/util/XMLHelper.h
index cc38979..1906480 100644
--- a/xmltooling/util/XMLHelper.h
+++ b/xmltooling/util/XMLHelper.h
@@ -271,9 +271,21 @@ namespace xmltooling {
         static xercesc::DOMElement* getPreviousSiblingElement(const xercesc::DOMNode* n, const XMLCh* ns, const XMLCh* localName);
 
         /**
-         * Returns the content of the first Text node found in the element, if any.
-         * This is roughly similar to the DOM getTextContent function, but only
-         * examines the immediate children of the element.
+        * Returns all text content inside an element, regardless of the number of
+        * child nodes involved, up to the first child element encountered if any.
+
+        * Because this may require merging data, the text is returned in a separately
+        * allocated buffer the caller must free using delete[].
+        *
+        * @param e   the element
+        * @return an array allocated with new[] containing the text
+        */
+        static XMLCh* getWholeTextContent(const xercesc::DOMElement* e);
+
+        /**
+         * Returns the content of the first Text node found in the element, if any,
+         * acting on the assumption there can be no embedded comment, CDATA, or other
+         * interfering node types interrupting the text.
          *
          * @param e     element to examine
          * @return the content of the first Text node found, or nullptr

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list