#63712 - upgrading xmlsec causes junit tests to fail

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1875392 13f79535-47bb-0310-9956-ffa450edef68

.classpath

@@ -33,7 +33,7 @@
 	<classpathentry kind="lib" path="ooxml-testlib/reflections.jar"/>
 	<classpathentry kind="lib" path="ooxml-testlib/guava.jar"/>
 	<classpathentry kind="lib" path="ooxml-testlib/javassist.jar"/>
-	<classpathentry exported="true" kind="lib" path="compile-lib/xmlsec-2.1.2.jar"/>
+	<classpathentry exported="true" kind="lib" path="compile-lib/xmlsec-2.1.5.jar"/>
 	<classpathentry exported="true" kind="lib" path="lib/commons-codec-1.14.jar"/>
 	<classpathentry exported="true" kind="lib" path="lib/commons-logging-1.2.jar"/>
 	<classpathentry exported="true" kind="lib" path="lib/commons-collections4-4.4.jar"/>

build.gradle

@@ -241,7 +241,7 @@ project('ooxml') {
         compile 'org.apache.commons:commons-collections4:4.4'
         compile "org.apache.commons:commons-math3:${commonsMathVersion}"
         compile "org.apache.commons:commons-compress:${commonsCompressVersion}"
-        compile 'org.apache.santuario:xmlsec:2.1.2'
+        compile 'org.apache.santuario:xmlsec:2.1.5'
         compile "org.bouncycastle:bcpkix-jdk15on:${bouncyCastleVersion}"
         compile 'com.github.virtuald:curvesapi:1.06'
         compile 'com.zaxxer:SparseBitSet:1.2'

build.xml

@@ -223,8 +223,8 @@ under the License.
               value="${repository.m2}/maven2/com/zaxxer/SparseBitSet/1.2/SparseBitSet-1.2.jar"/>
 
     <!-- xml signature libs -->
-    <property name="dsig.xmlsec.jar" location="${compile.lib}/xmlsec-2.1.2.jar"/>
-    <property name="dsig.xmlsec.url" value="${repository.m2}/maven2/org/apache/santuario/xmlsec/2.1.2/xmlsec-2.1.2.jar"/>
+    <property name="dsig.xmlsec.jar" location="${compile.lib}/xmlsec-2.1.5.jar"/>
+    <property name="dsig.xmlsec.url" value="${repository.m2}/maven2/org/apache/santuario/xmlsec/2.1.5/xmlsec-2.1.5.jar"/>
     <property name="dsig.bouncycastle-prov.jar" location="${compile.lib}/bcprov-ext-jdk15on-1.64.jar"/>
     <property name="dsig.bouncycastle-prov.url" value="${repository.m2}/maven2/org/bouncycastle/bcprov-ext-jdk15on/1.64/bcprov-ext-jdk15on-1.64.jar"/>
     <property name="dsig.bouncycastle-pkix.jar" location="${compile.lib}/bcpkix-jdk15on-1.64.jar"/>
@@ -715,6 +715,10 @@ under the License.
                 <include name="xercesImpl-*.jar"/>
                 <include name="xmlsec-2.0*.jar"/>
                 <include name="xmlsec-2.1.0.jar"/>
+                <include name="xmlsec-2.1.1.jar"/>
+                <include name="xmlsec-2.1.2.jar"/>
+                <include name="xmlsec-2.1.3.jar"/>
+                <include name="xmlsec-2.1.4.jar"/>
                 <include name="bc*jdk15on-1.5*.jar"/>
                 <include name="bc*jdk15on-1.60*.jar"/>
                 <include name="bc*jdk15on-1.61*.jar"/>

sonar/ooxml/pom.xml

@@ -84,7 +84,7 @@
                         </fileset>
                     </filesets>
                 </configuration>
-            </plugin>			
+            </plugin>
 
 
             <!-- set jvm parameters for surefire plugin -->
@@ -147,7 +147,7 @@
         <dependency>
             <groupId>org.apache.santuario</groupId>
             <artifactId>xmlsec</artifactId>
-            <version>2.1.2</version>
+            <version>2.1.5</version>
         </dependency>
         <dependency>
             <groupId>org.apache.commons</groupId>
@@ -159,7 +159,7 @@
             <artifactId>curvesapi</artifactId>
             <version>1.06</version>
         </dependency>
-  
+
         <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>

src/ooxml/java/org/apache/poi/poifs/crypt/dsig/SignatureInfo.java

@@ -153,7 +153,7 @@ import org.w3c.dom.events.MutationEvent;
  * in the classpath:</p>
  * <ul>
  * <li>BouncyCastle bcpkix and bcprov (tested against 1.64)</li>
- * <li>Apache Santuario "xmlsec" (tested against 2.1.2)</li>
+ * <li>Apache Santuario "xmlsec" (tested against 2.1.5)</li>
  * <li>and slf4j-api (tested against 1.7.30)</li>
  * </ul>
  */
@@ -461,27 +461,22 @@ public class SignatureInfo {
             return;
         }
 
-        EventTarget target = (EventTarget)document;
-
         final EventListener[] el = { null };
-        el[0] = (e) -> {
-            if (!(e instanceof MutationEvent)) {
-                return;
-            }
+        final EventTarget eventTarget = (EventTarget)document;
+        final String eventType = "DOMSubtreeModified";
+        final boolean DONT_USE_CAPTURE = false;
 
-            MutationEvent mutEvt = (MutationEvent) e;
-            EventTarget et = mutEvt.getTarget();
-            if (!(et instanceof Element)) {
-                return;
+        el[0] = (e) -> {
+            if (e instanceof MutationEvent && e.getTarget() instanceof Document) {
+                eventTarget.removeEventListener(eventType, el[0], DONT_USE_CAPTURE);
+                sml.handleElement(this, document, eventTarget, el[0]);
+                eventTarget.addEventListener(eventType, el[0], DONT_USE_CAPTURE);
             }
-
-            sml.handleElement(this, (Element) et, target, el[0]);
         };
 
-        SignatureMarshalListener.setListener(target, el[0], true);
+        eventTarget.addEventListener(eventType, el[0], DONT_USE_CAPTURE);
     }
 
-
     /**
      * Helper method for adding informations after the signing.
      * Normally {@link #confirmSignature()} is sufficient to be used.

src/ooxml/java/org/apache/poi/poifs/crypt/dsig/SignatureMarshalDefaultListener.java

@@ -17,47 +17,76 @@
 
 package org.apache.poi.poifs.crypt.dsig;
 
-import static org.apache.poi.poifs.crypt.dsig.SignatureMarshalListener.setListener;
-import static org.apache.poi.poifs.crypt.dsig.facets.SignatureFacet.OO_DIGSIG_NS;
+import static org.apache.poi.poifs.crypt.dsig.facets.SignatureFacet.XML_DIGSIG_NS;
 import static org.apache.poi.poifs.crypt.dsig.facets.SignatureFacet.XML_NS;
 
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import org.w3c.dom.Document;
 import org.w3c.dom.Element;
+import org.w3c.dom.NamedNodeMap;
 import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
 import org.w3c.dom.events.EventListener;
 import org.w3c.dom.events.EventTarget;
+import org.w3c.dom.traversal.DocumentTraversal;
+import org.w3c.dom.traversal.NodeFilter;
+import org.w3c.dom.traversal.NodeIterator;
 
 /**
  * This listener class is used, to modify the to be digested xml document,
  * e.g. to register id attributes or set prefixes for registered namespaces
  */
 public class SignatureMarshalDefaultListener implements SignatureMarshalListener {
+    private final Set<String> IGNORE_NS = new HashSet<>(Arrays.asList(null, XML_NS, XML_DIGSIG_NS));
+    private final String OBJECT_TAG = "Object";
+
     @Override
-    public void handleElement(SignatureInfo signatureInfo, Element el, EventTarget target, EventListener parentListener) {
-        if (el.hasAttribute("Id")) {
-            el.setIdAttribute("Id", true);
-        }
+    public void handleElement(SignatureInfo signatureInfo, Document doc, EventTarget target, EventListener parentListener) {
+        // see POI #63712 : because of Santuario change r1853805 in XmlSec 2.1.3,
+        // we have to deal with the whole document now
 
-        setListener(target, parentListener, false);
-        if (OO_DIGSIG_NS.equals(el.getNamespaceURI())) {
-            String parentNS = el.getParentNode().getNamespaceURI();
-            if (!OO_DIGSIG_NS.equals(parentNS) && !el.hasAttributeNS(XML_NS, "mdssi")) {
-                el.setAttributeNS(XML_NS, "xmlns:mdssi", OO_DIGSIG_NS);
-            }
+        final DocumentTraversal traversal = (DocumentTraversal) doc;
+        final Map<String, String> prefixCfg = signatureInfo.getSignatureConfig().getNamespacePrefixes();
+
+        final Map<String, String> prefixUsed = new HashMap<>();
+
+        NodeList nl = doc.getElementsByTagName(OBJECT_TAG);
+        final int objLen = nl.getLength();
+        for (int i=0; i<objLen; i++) {
+            final Element objNode = (Element)nl.item(i);
+            getAllNamespaces(traversal, objNode, prefixCfg, prefixUsed);
+            prefixUsed.forEach((ns, prefix) -> objNode.setAttributeNS(XML_NS, "xmlns:"+prefix, ns));
         }
-        setPrefix(signatureInfo, el);
-        setListener(target, parentListener, true);
     }
 
-    protected static void setPrefix(SignatureInfo signatureInfo, Node el) {
-        String prefix = signatureInfo.getSignatureConfig().getNamespacePrefixes().get(el.getNamespaceURI());
-        if (prefix != null && el.getPrefix() == null) {
-            el.setPrefix(prefix);
+    private void getAllNamespaces(DocumentTraversal traversal, Element objNode, Map<String, String> prefixCfg, Map<String, String> prefixUsed) {
+        prefixUsed.clear();
+        final NodeIterator iter = traversal.createNodeIterator(objNode, NodeFilter.SHOW_ELEMENT, null, false);
+        try {
+            for (Element node; (node = (Element)iter.nextNode()) != null; ) {
+                setPrefix(node, prefixCfg, prefixUsed);
+                NamedNodeMap nnm = node.getAttributes();
+                final int nnmLen = nnm.getLength();
+                for (int j=0; j<nnmLen; j++) {
+                    setPrefix(nnm.item(j), prefixCfg, prefixUsed);
+                }
+            }
+        } finally {
+            iter.detach();
         }
+    }
 
-        NodeList nl = el.getChildNodes();
-        for (int i=0; i<nl.getLength(); i++) {
-            setPrefix(signatureInfo, nl.item(i));
+    private void setPrefix(Node node, Map<String,String> prefixCfg, Map<String,String> prefixUsed) {
+        String ns = node.getNamespaceURI();
+        String prefix = prefixCfg.get(ns);
+        if (!IGNORE_NS.contains(prefix)) {
+            node.setPrefix(prefix);
+            prefixUsed.put(ns, prefix);
         }
     }
 }

src/ooxml/java/org/apache/poi/poifs/crypt/dsig/SignatureMarshalListener.java

@@ -17,7 +17,7 @@
 
 package org.apache.poi.poifs.crypt.dsig;
 
-import org.w3c.dom.Element;
+import org.w3c.dom.Document;
 import org.w3c.dom.events.EventListener;
 import org.w3c.dom.events.EventTarget;
 
@@ -26,16 +26,5 @@ import org.w3c.dom.events.EventTarget;
  * e.g. to register id attributes or set prefixes for registered namespaces
  */
 public interface SignatureMarshalListener {
-    void handleElement(SignatureInfo signatureInfo, Element el, EventTarget target, EventListener parentListener);
-
-    // helper method to keep it in one place
-    static void setListener(EventTarget target, EventListener listener, boolean enabled) {
-        final String type = "DOMSubtreeModified";
-        final boolean DONT_USE_CAPTURE = false;
-        if (enabled) {
-            target.addEventListener(type, listener, DONT_USE_CAPTURE);
-        } else {
-            target.removeEventListener(type, listener, DONT_USE_CAPTURE);
-        }
-    }
+    void handleElement(SignatureInfo signatureInfo, Document doc, EventTarget target, EventListener parentListener);
 }

src/ooxml/java/org/apache/poi/poifs/crypt/dsig/facets/XAdESSignatureFacet.java

@@ -31,8 +31,8 @@ import static org.apache.poi.poifs.crypt.dsig.facets.SignatureFacetHelper.newTra
 import java.security.MessageDigest;
 import java.security.cert.CertificateEncodingException;
 import java.security.cert.X509Certificate;
-import java.util.Arrays;
 import java.util.Calendar;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
@@ -74,7 +74,9 @@ import org.etsi.uri.x01903.v13.SignerRoleType;
 import org.w3.x2000.x09.xmldsig.DigestMethodType;
 import org.w3.x2000.x09.xmldsig.X509IssuerSerialType;
 import org.w3c.dom.Document;
+import org.w3c.dom.Element;
 import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
 
 /**
  * XAdES Signature Facet. Implements XAdES v1.4.1 which is compatible with XAdES
@@ -233,9 +235,15 @@ public class XAdESSignatureFacet implements SignatureFacet {
 
     private XMLObject addXadesObject(SignatureInfo signatureInfo, Document document, QualifyingPropertiesType qualifyingProperties) {
         Node qualDocElSrc = qualifyingProperties.getDomNode();
-        Node qualDocEl = document.importNode(qualDocElSrc, true);
-        List<XMLStructure> xadesObjectContent = Arrays.asList(new DOMStructure(qualDocEl));
-        return signatureInfo.getSignatureFactory().newXMLObject(xadesObjectContent, null, null, null);
+        Element qualDocEl = (Element)document.importNode(qualDocElSrc, true);
+
+        NodeList nl = qualDocEl.getElementsByTagNameNS(SignatureFacet.XADES_132_NS, "SignedProperties");
+        assert(nl.getLength() == 1);
+        ((Element)nl.item(0)).setIdAttribute("Id", true);
+
+        List<XMLStructure> xadesObjectContent = Collections.singletonList(new DOMStructure(qualDocEl));
+        XMLObject xo = signatureInfo.getSignatureFactory().newXMLObject(xadesObjectContent, null, null, null);
+        return xo;
     }
 
     private Reference addXadesReference(SignatureInfo signatureInfo) throws XMLSignatureException {