[java-opensaml] branch main updated: Adjust some annotations and null assumptions.

Scott Cantor cantor.2 at osu.edu
Tue Mar 7 21:19:06 UTC 2023


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

scantor pushed a commit to branch main
in repository java-opensaml.

View the commit online:
http://git.shibboleth.net/view/?p=java-opensaml.git;a=commit;h=247ba19f5ec66e4d8bd8fcaf09cab194d2cea56f

The following commit(s) were added to refs/heads/main by this push:
     new 247ba19f5 Adjust some annotations and null assumptions.
247ba19f5 is described below

commit 247ba19f5ec66e4d8bd8fcaf09cab194d2cea56f
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Mar 7 16:19:02 2023 -0500

    Adjust some annotations and null assumptions.
---
 .../core/xml/schema/impl/XSQNameUnmarshaller.java  | 11 +++--
 .../signature/AbstractSignableXMLObject.java       |  3 +-
 .../xmlsec/signature/impl/SignatureMarshaller.java |  3 +-
 .../signature/impl/SignatureUnmarshaller.java      |  7 ++-
 .../support/impl/SignatureAlgorithmValidator.java  | 51 +++++++++++++++-------
 5 files changed, 52 insertions(+), 23 deletions(-)

diff --git a/opensaml-core-impl/src/main/java/org/opensaml/core/xml/schema/impl/XSQNameUnmarshaller.java b/opensaml-core-impl/src/main/java/org/opensaml/core/xml/schema/impl/XSQNameUnmarshaller.java
index 4c5bcb218..02f1b50c9 100644
--- a/opensaml-core-impl/src/main/java/org/opensaml/core/xml/schema/impl/XSQNameUnmarshaller.java
+++ b/opensaml-core-impl/src/main/java/org/opensaml/core/xml/schema/impl/XSQNameUnmarshaller.java
@@ -21,6 +21,8 @@ import net.shibboleth.shared.primitive.StringSupport;
 import net.shibboleth.shared.xml.ElementSupport;
 import net.shibboleth.shared.xml.QNameSupport;
 
+import javax.annotation.Nonnull;
+
 import org.opensaml.core.xml.XMLObject;
 import org.opensaml.core.xml.io.AbstractXMLObjectUnmarshaller;
 import org.opensaml.core.xml.io.UnmarshallingException;
@@ -28,19 +30,20 @@ import org.opensaml.core.xml.schema.XSQName;
 import org.w3c.dom.Text;
 
 /**
- * A thread-safe unmarshaller for {@link org.opensaml.core.xml.schema.XSQName}s.
+ * A thread-safe unmarshaller for {@link XSQName}s.
  */
 public class XSQNameUnmarshaller extends AbstractXMLObjectUnmarshaller {
 
     /** {@inheritDoc} */
-    protected void processChildElement(final XMLObject parentXMLObject, final XMLObject childXMLObject)
-            throws UnmarshallingException {
+    protected void processChildElement(@Nonnull final XMLObject parentXMLObject,
+            @Nonnull final XMLObject childXMLObject) throws UnmarshallingException {
         // no child elements
         // left this in to bypass the "ignore" logging message since we're not in fact ignoring the content
     }
 
     /** {@inheritDoc} */
-    protected void unmarshallTextContent(final XMLObject xmlObject, final Text content) throws UnmarshallingException {
+    protected void unmarshallTextContent(@Nonnull final XMLObject xmlObject, @Nonnull final Text content)
+            throws UnmarshallingException {
         final String textContent = StringSupport.trimOrNull(content.getData());
         if (textContent != null) {
             final XSQName qname = (XSQName) xmlObject;
diff --git a/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/signature/AbstractSignableXMLObject.java b/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/signature/AbstractSignableXMLObject.java
index 18e79ce5e..1ba02b4f3 100644
--- a/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/signature/AbstractSignableXMLObject.java
+++ b/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/signature/AbstractSignableXMLObject.java
@@ -49,8 +49,9 @@ public abstract class AbstractSignableXMLObject extends AbstractXMLObject implem
 
     /** {@inheritDoc} */
     public boolean isSigned() {
+        final Element root = getDOM();
         
-        Element child = ElementSupport.getFirstChildElement(getDOM());
+        Element child = root != null ? ElementSupport.getFirstChildElement(root) : null;
         while (child != null && !ElementSupport.isElementNamed(child, SignatureConstants.XMLSIG_NS,
                 Signature.DEFAULT_ELEMENT_LOCAL_NAME)) {
             child = ElementSupport.getNextSiblingElement(child);
diff --git a/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/signature/impl/SignatureMarshaller.java b/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/signature/impl/SignatureMarshaller.java
index e751fa8ca..561a9e238 100644
--- a/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/signature/impl/SignatureMarshaller.java
+++ b/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/signature/impl/SignatureMarshaller.java
@@ -17,6 +17,7 @@
 
 package org.opensaml.xmlsec.signature.impl;
 
+import javax.annotation.Nonnull;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
 
@@ -98,7 +99,7 @@ public class SignatureMarshaller implements Marshaller {
      * 
      * @throws MarshallingException thrown if the signature can not be constructed
      */
-    private Element createSignatureElement(final Signature signature, final Document document)
+    @Nonnull private Element createSignatureElement(final Signature signature, final Document document)
             throws MarshallingException {
         log.debug("Starting to marshall {}", signature.getElementQName());
 
diff --git a/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/signature/impl/SignatureUnmarshaller.java b/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/signature/impl/SignatureUnmarshaller.java
index 11783273a..973d49f13 100644
--- a/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/signature/impl/SignatureUnmarshaller.java
+++ b/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/signature/impl/SignatureUnmarshaller.java
@@ -19,6 +19,9 @@ package org.opensaml.xmlsec.signature.impl;
 
 import java.util.List;
 
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
 import net.shibboleth.shared.primitive.StringSupport;
 import net.shibboleth.shared.xml.ElementSupport;
 
@@ -53,7 +56,7 @@ public class SignatureUnmarshaller implements Unmarshaller {
     }
 
     /** {@inheritDoc} */
-    public Signature unmarshall(final Element signatureElement) throws UnmarshallingException {
+    public Signature unmarshall(@Nonnull final Element signatureElement) throws UnmarshallingException {
         log.debug("Starting to unmarshall Apache XML-Security-based SignatureImpl element");
 
         final SignatureImpl signature =
@@ -95,7 +98,7 @@ public class SignatureUnmarshaller implements Unmarshaller {
      * @param signatureMethodElement the ds:SignatureMethod element
      * @return the HMAC output length value, or null if not present
      */
-    private Integer getHMACOutputLengthValue(final Element signatureMethodElement) {
+    private Integer getHMACOutputLengthValue(@Nullable final Element signatureMethodElement) {
         if (signatureMethodElement == null) {
             return null;
         }
diff --git a/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/signature/support/impl/SignatureAlgorithmValidator.java b/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/signature/support/impl/SignatureAlgorithmValidator.java
index 5d35ca2da..0d8467d8e 100644
--- a/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/signature/support/impl/SignatureAlgorithmValidator.java
+++ b/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/signature/support/impl/SignatureAlgorithmValidator.java
@@ -26,6 +26,7 @@ import javax.annotation.Nullable;
 import javax.xml.namespace.QName;
 
 import net.shibboleth.shared.annotation.ParameterName;
+import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.logic.Constraint;
 import net.shibboleth.shared.primitive.StringSupport;
 import net.shibboleth.shared.xml.AttributeSupport;
@@ -52,23 +53,27 @@ import org.w3c.dom.Element;
 public class SignatureAlgorithmValidator {
     
     /** QName of 'ds:SignedInfo' element. */
-    private static final QName ELEMENT_NAME_SIGNED_INFO = new QName(SignatureConstants.XMLSIG_NS, "SignedInfo");
+    @Nonnull private static final QName ELEMENT_NAME_SIGNED_INFO =
+            new QName(SignatureConstants.XMLSIG_NS, "SignedInfo");
     
     /** QName of 'ds:SignatureMethod' element. */
-    private static final QName ELEMENT_NAME_SIGNATURE_METHOD = new QName(SignatureConstants.XMLSIG_NS, 
+    @Nonnull private static final QName ELEMENT_NAME_SIGNATURE_METHOD =
+            new QName(SignatureConstants.XMLSIG_NS, 
             "SignatureMethod");
     
     /** QName of 'ds:Reference' element. */
-    private static final QName ELEMENT_NAME_REFERENCE = new QName(SignatureConstants.XMLSIG_NS, "Reference");
+    @Nonnull private static final QName ELEMENT_NAME_REFERENCE =
+            new QName(SignatureConstants.XMLSIG_NS, "Reference");
     
     /** QName of 'ds:DigestMethod' element. */
-    private static final QName ELEMENT_NAME_DIGEST_METHOD = new QName(SignatureConstants.XMLSIG_NS, "DigestMethod");
+    @Nonnull private static final QName ELEMENT_NAME_DIGEST_METHOD =
+            new QName(SignatureConstants.XMLSIG_NS, "DigestMethod");
     
     /** Local name of 'Algorithm' attribute. */
-    private static final String ATTR_NAME_ALGORTHM = "Algorithm";
+    @Nonnull @NotEmpty private static final String ATTR_NAME_ALGORTHM = "Algorithm";
     
     /** Logger. */
-    private Logger log = LoggerFactory.getLogger(SignatureAlgorithmValidator.class);
+    @Nonnull private Logger log = LoggerFactory.getLogger(SignatureAlgorithmValidator.class);
     
     /** The collection of algorithm URIs which are included. */
     private Collection<String> includedAlgorithmURIs;
@@ -146,17 +151,22 @@ public class SignatureAlgorithmValidator {
     @Nonnull protected String getSignatureAlgorithm(@Nonnull final Signature signatureXMLObject) 
             throws SignatureException {
         final Element signature = signatureXMLObject.getDOM();
-        final Element signedInfo = ElementSupport.getFirstChildElement(signature, ELEMENT_NAME_SIGNED_INFO);
-        final Element signatureMethod = ElementSupport.getFirstChildElement(signedInfo, ELEMENT_NAME_SIGNATURE_METHOD);
-        
-        if (signatureMethod != null) {
-            final String signatureMethodAlgorithm = StringSupport.trimOrNull(
-                    AttributeSupport.getAttributeValue(signatureMethod, null, ATTR_NAME_ALGORTHM));
-            if (signatureMethodAlgorithm != null) {
-                return signatureMethodAlgorithm;
+        if (signature != null) {
+            final Element signedInfo = ElementSupport.getFirstChildElement(signature, ELEMENT_NAME_SIGNED_INFO);
+            if (signedInfo != null) {
+                final Element signatureMethod =
+                        ElementSupport.getFirstChildElement(signedInfo, ELEMENT_NAME_SIGNATURE_METHOD);
+                
+                if (signatureMethod != null) {
+                    final String signatureMethodAlgorithm = StringSupport.trimOrNull(
+                            AttributeSupport.getAttributeValue(signatureMethod, null, ATTR_NAME_ALGORTHM));
+                    if (signatureMethodAlgorithm != null) {
+                        return signatureMethodAlgorithm;
+                    }
+                }
             }
         }
-        throw new SignatureException("SignatureMethod element or Algorithm was null");
+        throw new SignatureException("Signature/SignedInfo/SignatureMethod elements or Algorithm were null");
     }
 
     
@@ -171,9 +181,20 @@ public class SignatureAlgorithmValidator {
             @Nonnull final Signature signatureXMLObject) throws SignatureException {
         final ArrayList<String> digestMethodAlgorithms = new ArrayList<>();
         
+        // TODO: should these null checks throw?
+        // I suspect so necause the getSignatureAlgorithm logic did/does.
+        
         final Element signature = signatureXMLObject.getDOM();
+        if (signature == null) {
+            log.warn("Signature element was null");
+            return digestMethodAlgorithms;
+        }
         
         final Element signedInfo = ElementSupport.getFirstChildElement(signature, ELEMENT_NAME_SIGNED_INFO);
+        if (signedInfo == null) {
+            log.warn("SignedInfo element was absent");
+            return digestMethodAlgorithms;
+        }
         
         for (final Element reference : ElementSupport.getChildElements(signedInfo, ELEMENT_NAME_REFERENCE)) {
             final Element digestMethod = ElementSupport.getFirstChildElement(reference, ELEMENT_NAME_DIGEST_METHOD);

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


More information about the commits mailing list