[java-opensaml] branch master updated: JSPT-79 - Review date and time handling for Java 8

Scott Cantor cantor.2 at osu.edu
Mon Mar 18 21:32:04 EDT 2019


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

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

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

The following commit(s) were added to refs/heads/master by this push:
       new  12ccfaa   JSPT-79 - Review date and time handling for Java 8
12ccfaa is described below

commit 12ccfaae686f315fa38d039e7170ed0789326550
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Mar 18 21:31:59 2019 -0400

    JSPT-79 - Review date and time handling for Java 8
    
    https://issues.shibboleth.net/jira/browse/JSPT-79
    
    More remediation of longs in various APIs.
---
 ...actConditionalLoadXMLObjectLoadSaveManager.java | 18 +++++-----
 .../ConditionalLoadXMLObjectLoadSaveManager.java   | 10 +++---
 .../xml/persist/FilesystemLoadSaveManager.java     |  7 ++--
 .../core/xml/persist/MapLoadSaveManager.java       | 22 ++++++------
 .../opensaml/core/xml/XMLObjectBaseTestCase.java   | 17 ---------
 .../xml/persist/FilesystemLoadSaveManagerTest.java |  5 +--
 .../core/xml/persist/MapLoadSaveManagerTest.java   |  5 +--
 .../opensaml/profile/RequestContextBuilder.java    | 22 ++++++------
 .../saml2/assertion/SAML20AssertionValidator.java  | 42 ++++++++++++++--------
 .../SAML2AssertionValidationParameters.java        |  6 ++--
 .../impl/AddNotOnOrAfterConditionToAssertions.java |  4 +--
 .../impl/AbstractDynamicMetadataResolver.java      | 10 +++---
 .../impl/AbstractSubjectConfirmationValidator.java |  4 +--
 .../impl/OneTimeUseConditionValidator.java         | 29 ++++++++-------
 .../messaging/impl/AddTimestampHandler.java        | 26 +++++++-------
 .../messaging/impl/AddTimestampHandlerTest.java    |  7 ++--
 16 files changed, 125 insertions(+), 109 deletions(-)

diff --git a/opensaml-core/src/main/java/org/opensaml/core/xml/persist/AbstractConditionalLoadXMLObjectLoadSaveManager.java b/opensaml-core/src/main/java/org/opensaml/core/xml/persist/AbstractConditionalLoadXMLObjectLoadSaveManager.java
index 5388678..eed6957 100644
--- a/opensaml-core/src/main/java/org/opensaml/core/xml/persist/AbstractConditionalLoadXMLObjectLoadSaveManager.java
+++ b/opensaml-core/src/main/java/org/opensaml/core/xml/persist/AbstractConditionalLoadXMLObjectLoadSaveManager.java
@@ -18,6 +18,7 @@
 package org.opensaml.core.xml.persist;
 
 import java.io.IOException;
+import java.time.Instant;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -44,7 +45,7 @@ public abstract class AbstractConditionalLoadXMLObjectLoadSaveManager<T extends
     private boolean loadConditionally;
     
     /** Storage for last modified time of requested data. */
-    private Map<String, Long> loadLastModified;
+    private Map<String,Instant> loadLastModified;
     
     /** 
      * Constructor. 
@@ -64,13 +65,13 @@ public abstract class AbstractConditionalLoadXMLObjectLoadSaveManager<T extends
     }
     
     /** {@inheritDoc} */
-    @Nullable public synchronized Long getLoadLastModified(@Nonnull final String key) {
+    @Nullable public synchronized Instant getLoadLastModified(@Nonnull final String key) {
         return loadLastModified.get(key);
     }
 
     /** {@inheritDoc} */
-    @Nullable public synchronized Long clearLoadLastModified(@Nonnull final String key) {
-        final Long prev = loadLastModified.get(key);
+    @Nullable public synchronized Instant clearLoadLastModified(@Nonnull final String key) {
+        final Instant prev = loadLastModified.get(key);
         loadLastModified.remove(key);
         return prev;
     }
@@ -86,8 +87,8 @@ public abstract class AbstractConditionalLoadXMLObjectLoadSaveManager<T extends
      * @param key the target key
      * @return the previously cached modified time, or null if did not exist
      */
-    protected synchronized Long updateLoadLastModified(@Nonnull final String key) {
-        return updateLoadLastModified(key, System.currentTimeMillis());
+    protected synchronized Instant updateLoadLastModified(@Nonnull final String key) {
+        return updateLoadLastModified(key, Instant.now());
     }
     
     /**
@@ -97,11 +98,12 @@ public abstract class AbstractConditionalLoadXMLObjectLoadSaveManager<T extends
      * @param modified the new cached modified time
      * @return the previously cached modified time, or null if did not exist
      */
-    protected synchronized Long updateLoadLastModified(@Nonnull final String key, @Nullable final Long modified) {
+    @Nullable protected synchronized Instant updateLoadLastModified(@Nonnull final String key,
+            @Nullable final Instant modified) {
         if (modified == null) {
             return null;
         }
-        final Long prev = loadLastModified.get(key);
+        final Instant prev = loadLastModified.get(key);
         loadLastModified.put(key, modified);
         return prev;
     }
diff --git a/opensaml-core/src/main/java/org/opensaml/core/xml/persist/ConditionalLoadXMLObjectLoadSaveManager.java b/opensaml-core/src/main/java/org/opensaml/core/xml/persist/ConditionalLoadXMLObjectLoadSaveManager.java
index bbaaebe..3e735ab 100644
--- a/opensaml-core/src/main/java/org/opensaml/core/xml/persist/ConditionalLoadXMLObjectLoadSaveManager.java
+++ b/opensaml-core/src/main/java/org/opensaml/core/xml/persist/ConditionalLoadXMLObjectLoadSaveManager.java
@@ -17,6 +17,8 @@
 
 package org.opensaml.core.xml.persist;
 
+import java.time.Instant;
+
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
@@ -50,17 +52,17 @@ public interface ConditionalLoadXMLObjectLoadSaveManager<T extends XMLObject> ex
      * </p> 
      * 
      * @param key the target key
-     * @return the current cached modified time in milliseconds since the epoch, may be null
+     * @return the current cached modified time, may be null
      */
-    @Nullable public Long getLoadLastModified(@Nonnull final String key);
+    @Nullable public Instant getLoadLastModified(@Nonnull final String key);
     
     /**
      * Clear the cached modified time for the last load of the specified key.
      * 
      * @param key the target key
-     * @return the previously cached modified time in milliseconds since the epoch, or null if did not exist
+     * @return the previously cached modified time, or null if did not exist
      */
-    @Nullable public Long clearLoadLastModified(@Nonnull final String key);
+    @Nullable public Instant clearLoadLastModified(@Nonnull final String key);
 
     /**
      * Clear the cached modified times for the last load for all keys.
diff --git a/opensaml-core/src/main/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManager.java b/opensaml-core/src/main/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManager.java
index b6154a5..7fb41ad 100644
--- a/opensaml-core/src/main/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManager.java
+++ b/opensaml-core/src/main/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManager.java
@@ -23,6 +23,7 @@ import java.io.FileFilter;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.time.Instant;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
@@ -249,7 +250,7 @@ public class FilesystemLoadSaveManager<T extends XMLObject> extends AbstractCond
             try (final ByteArrayInputStream bais = new ByteArrayInputStream(source)) {
                 final XMLObject xmlObject = XMLObjectSupport.unmarshallFromInputStream(parserPool, bais);
                 xmlObject.getObjectMetadata().put(new XMLObjectSource(source));
-                updateLoadLastModified(key, file.lastModified());
+                updateLoadLastModified(key, Instant.ofEpochMilli(file.lastModified()));
                 //TODO via ctor, etc, does caller need to supply a Class so we can can test and throw an IOException, 
                 // rather than an unchecked ClassCastException?
                 return (T) xmlObject;
@@ -262,9 +263,9 @@ public class FilesystemLoadSaveManager<T extends XMLObject> extends AbstractCond
     /** {@inheritDoc} */
     protected synchronized boolean isUnmodifiedSinceLastLoad(@Nonnull final String key) throws IOException {
         final File file = buildFile(key);
-        final long lastModified = file.lastModified();
+        final Instant lastModified = Instant.ofEpochMilli(file.lastModified());
         log.trace("File '{}' last modified was: {}", file.getAbsolutePath(), lastModified);
-        return getLoadLastModified(key) != null && lastModified <= getLoadLastModified(key);
+        return getLoadLastModified(key) != null && !lastModified.isAfter(getLoadLastModified(key));
     }
 
     /** {@inheritDoc} */
diff --git a/opensaml-core/src/main/java/org/opensaml/core/xml/persist/MapLoadSaveManager.java b/opensaml-core/src/main/java/org/opensaml/core/xml/persist/MapLoadSaveManager.java
index 32194b5..103c353 100644
--- a/opensaml-core/src/main/java/org/opensaml/core/xml/persist/MapLoadSaveManager.java
+++ b/opensaml-core/src/main/java/org/opensaml/core/xml/persist/MapLoadSaveManager.java
@@ -18,6 +18,7 @@
 package org.opensaml.core.xml.persist;
 
 import java.io.IOException;
+import java.time.Instant;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Map;
@@ -43,17 +44,17 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
 public class MapLoadSaveManager<T extends XMLObject> extends AbstractConditionalLoadXMLObjectLoadSaveManager<T> {
     
     /** Logger. */
-    private Logger log = LoggerFactory.getLogger(MapLoadSaveManager.class);
+    @Nonnull private Logger log = LoggerFactory.getLogger(MapLoadSaveManager.class);
     
     /** The backing map. */
-    private Map<String,T> backingMap;
+    @Nonnull private Map<String,T> backingMap;
     
     /** Storage to track last modified time of data. */
-    private Map<String,Long> dataLastModified;
+    @Nonnull private Map<String,Instant> dataLastModified;
     
     /** Constructor. */
     public MapLoadSaveManager() {
-        this(new HashMap<String,T>(), new HashMap<String,Long>(), false);
+        this(new HashMap<>(), new HashMap<>(), false);
     }
 
     /** 
@@ -63,7 +64,7 @@ public class MapLoadSaveManager<T extends XMLObject> extends AbstractConditional
      *      as defined in {@link ConditionalLoadXMLObjectLoadSaveManager}
      * */
     public MapLoadSaveManager(@ParameterName(name="conditionalLoad") final boolean conditionalLoad) {
-        this(new HashMap<String,T>(), new HashMap<String,Long>(), conditionalLoad);
+        this(new HashMap<String,T>(), new HashMap<>(), conditionalLoad);
     }
 
     /**
@@ -78,7 +79,7 @@ public class MapLoadSaveManager<T extends XMLObject> extends AbstractConditional
      * @param map the backing map 
      */
     public MapLoadSaveManager(@ParameterName(name="map") @Nonnull final Map<String, T> map) {
-        this(map, new HashMap<String,Long>(), false);
+        this(map, new HashMap<>(), false);
     }
     
     /**
@@ -91,7 +92,7 @@ public class MapLoadSaveManager<T extends XMLObject> extends AbstractConditional
      */
     protected MapLoadSaveManager(
             @ParameterName(name="map") @Nonnull final Map<String, T> map,
-            @ParameterName(name="dataLastModified") @Nonnull final Map<String,Long> lastModifiedMap,
+            @ParameterName(name="dataLastModified") @Nonnull final Map<String,Instant> lastModifiedMap,
             @ParameterName(name="conditionalLoad") final boolean conditionalLoad) {
         super(conditionalLoad);
         backingMap = Constraint.isNotNull(map, "Backing map was null");
@@ -143,7 +144,7 @@ public class MapLoadSaveManager<T extends XMLObject> extends AbstractConditional
             throw new IOException(String.format("Value already exists for key '%s'", key));
         } else {
             backingMap.put(key, xmlObject);
-            dataLastModified.put(key, System.currentTimeMillis());
+            dataLastModified.put(key, Instant.now());
         }
     }
 
@@ -178,9 +179,10 @@ public class MapLoadSaveManager<T extends XMLObject> extends AbstractConditional
 
     /** {@inheritDoc} */
     protected boolean isUnmodifiedSinceLastLoad(@Nonnull final String key) throws IOException {
-        final Long lastModified = dataLastModified.get(key);
+        final Instant lastModified = dataLastModified.get(key);
         log.trace("Key '{}' last modified was: {}", key, lastModified);
-        return getLoadLastModified(key) != null && lastModified != null && lastModified <= getLoadLastModified(key);
+        return getLoadLastModified(key) != null && lastModified != null
+                && !lastModified.isAfter(getLoadLastModified(key));
     }
 
 }
diff --git a/opensaml-core/src/test/java/org/opensaml/core/xml/XMLObjectBaseTestCase.java b/opensaml-core/src/test/java/org/opensaml/core/xml/XMLObjectBaseTestCase.java
index 2cfcdbc..35d5ea6 100644
--- a/opensaml-core/src/test/java/org/opensaml/core/xml/XMLObjectBaseTestCase.java
+++ b/opensaml-core/src/test/java/org/opensaml/core/xml/XMLObjectBaseTestCase.java
@@ -82,23 +82,6 @@ public abstract class XMLObjectBaseTestCase extends OpenSAMLInitBaseTestCase {
     /** Baseline for duration calculations (comes from XML Schema standard). */
     private static Calendar baseline = new GregorianCalendar(1696, 9, 1, 0, 0, 0);
     
-    /**
-     * Helper method to extract the value of an XML duration attribute in milliseconds.
-     * 
-     * @param element element to pull attribute from
-     * @param name  name of attribute
-     * 
-     * @return the attribute value converted to milliseconds
-     * @throws DatatypeConfigurationException if a {@link DatatypeFactory} can't be constructed
-     */
-    public static long fetchDuration(@Nonnull final Element element, @Nonnull final QName name)
-            throws DatatypeConfigurationException {
-        final DatatypeFactory dtf = DatatypeFactory.newInstance();
-        final Attr attr = AttributeSupport.getAttribute(element, name);
-        Assert.assertNotNull(attr);
-        return dtf.newDuration(attr.getValue()).getTimeInMillis(baseline);
-    }
-    
     @BeforeClass
 	protected void initXMLObjectSupport() throws Exception {
         XMLUnit.setIgnoreWhitespace(true);
diff --git a/opensaml-core/src/test/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManagerTest.java b/opensaml-core/src/test/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManagerTest.java
index 6808c82..f74898b 100644
--- a/opensaml-core/src/test/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManagerTest.java
+++ b/opensaml-core/src/test/java/org/opensaml/core/xml/persist/FilesystemLoadSaveManagerTest.java
@@ -21,6 +21,7 @@ import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
+import java.time.Instant;
 import java.util.Iterator;
 import java.util.NoSuchElementException;
 import java.util.Set;
@@ -153,7 +154,7 @@ public class FilesystemLoadSaveManagerTest extends XMLObjectBaseTestCase {
         manager.save("foo", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME, true));
         
         Assert.assertNotNull(manager.load("foo"));
-        Long initialCachedModified = manager.getLoadLastModified("foo");
+        Instant initialCachedModified = manager.getLoadLastModified("foo");
         Assert.assertNotNull(initialCachedModified);
         
         // Hasn't changed
@@ -168,7 +169,7 @@ public class FilesystemLoadSaveManagerTest extends XMLObjectBaseTestCase {
         manager.save("foo", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME, true), true);
         
         Assert.assertNotNull(manager.load("foo"));
-        Long updatedCachedModified = manager.getLoadLastModified("foo");
+        Instant updatedCachedModified = manager.getLoadLastModified("foo");
         Assert.assertNotNull(updatedCachedModified);
         Assert.assertNotEquals(updatedCachedModified, initialCachedModified);
         
diff --git a/opensaml-core/src/test/java/org/opensaml/core/xml/persist/MapLoadSaveManagerTest.java b/opensaml-core/src/test/java/org/opensaml/core/xml/persist/MapLoadSaveManagerTest.java
index ccd4681..d6c3d98 100644
--- a/opensaml-core/src/test/java/org/opensaml/core/xml/persist/MapLoadSaveManagerTest.java
+++ b/opensaml-core/src/test/java/org/opensaml/core/xml/persist/MapLoadSaveManagerTest.java
@@ -18,6 +18,7 @@
 package org.opensaml.core.xml.persist;
 
 import java.io.IOException;
+import java.time.Instant;
 import java.util.Iterator;
 import java.util.NoSuchElementException;
 import java.util.Set;
@@ -118,7 +119,7 @@ public class MapLoadSaveManagerTest extends XMLObjectBaseTestCase {
         manager.save("foo", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME));
         
         Assert.assertNotNull(manager.load("foo"));
-        Long initialCachedModified = manager.getLoadLastModified("foo");
+        Instant initialCachedModified = manager.getLoadLastModified("foo");
         Assert.assertNotNull(initialCachedModified);
         
         // Hasn't changed
@@ -131,7 +132,7 @@ public class MapLoadSaveManagerTest extends XMLObjectBaseTestCase {
         manager.save("foo", (SimpleXMLObject) buildXMLObject(SimpleXMLObject.ELEMENT_NAME), true);
         
         Assert.assertNotNull(manager.load("foo"));
-        Long updatedCachedModified = manager.getLoadLastModified("foo");
+        Instant updatedCachedModified = manager.getLoadLastModified("foo");
         Assert.assertNotNull(updatedCachedModified);
         Assert.assertNotEquals(updatedCachedModified, initialCachedModified);
         
diff --git a/opensaml-profile-api/src/test/java/org/opensaml/profile/RequestContextBuilder.java b/opensaml-profile-api/src/test/java/org/opensaml/profile/RequestContextBuilder.java
index 36ad18b..67a280b 100644
--- a/opensaml-profile-api/src/test/java/org/opensaml/profile/RequestContextBuilder.java
+++ b/opensaml-profile-api/src/test/java/org/opensaml/profile/RequestContextBuilder.java
@@ -17,6 +17,8 @@
 
 package org.opensaml.profile;
 
+import java.time.Instant;
+
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
@@ -35,8 +37,8 @@ public class RequestContextBuilder {
     /** The ID of the inbound message. */
     private String inboundMessageId = NO_VAL;
 
-    /** The issue instant of the inbound message in milliseconds. */
-    private long inboundMessageIssueInstant;
+    /** The issue instant of the inbound message. */
+    private Instant inboundMessageIssueInstant;
 
     /** The issuer of the inbound message. */
     private String inboundMessageIssuer = NO_VAL;
@@ -47,8 +49,8 @@ public class RequestContextBuilder {
     /** The ID of the outbound message. */
     private String outboundMessageId = NO_VAL;
 
-    /** The issue instant of the outbound message in milliseconds. */
-    private long outboundMessageIssueInstant;
+    /** The issue instant of the outbound message. */
+    private Instant outboundMessageIssueInstant;
 
     /** The issuer of the outbound message. */
     private String outboundMessageIssuer = NO_VAL;
@@ -90,13 +92,13 @@ public class RequestContextBuilder {
     }
 
     /**
-     * Sets the issue instant of the inbound message in milliseconds.
+     * Sets the issue instant of the inbound message.
      * 
-     * @param instant issue instant of the inbound message in milliseconds
+     * @param instant issue instant of the inbound message
      * 
      * @return this builder
      */
-    @Nonnull public RequestContextBuilder setInboundMessageIssueInstant(final long instant) {
+    @Nonnull public RequestContextBuilder setInboundMessageIssueInstant(@Nullable final Instant instant) {
         inboundMessageIssueInstant = instant;
         return this;
     }
@@ -138,13 +140,13 @@ public class RequestContextBuilder {
     }
 
     /**
-     * Sets the issue instant of the outbound message in milliseconds.
+     * Sets the issue instant of the outbound message.
      * 
-     * @param instant issue instant of the outbound message in milliseconds
+     * @param instant issue instant of the outbound message
      * 
      * @return this builder
      */
-    @Nonnull public RequestContextBuilder setOutboundMessageIssueInstant(final long instant) {
+    @Nonnull public RequestContextBuilder setOutboundMessageIssueInstant(@Nullable final Instant instant) {
         outboundMessageIssueInstant = instant;
         return this;
     }
diff --git a/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/assertion/SAML20AssertionValidator.java b/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/assertion/SAML20AssertionValidator.java
index b887135..6c388b0 100644
--- a/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/assertion/SAML20AssertionValidator.java
+++ b/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/assertion/SAML20AssertionValidator.java
@@ -17,6 +17,7 @@
 
 package org.opensaml.saml.saml2.assertion;
 
+import java.time.Duration;
 import java.time.Instant;
 import java.util.Collection;
 import java.util.List;
@@ -26,6 +27,8 @@ import javax.annotation.Nullable;
 import javax.xml.namespace.QName;
 
 import net.shibboleth.utilities.java.support.collection.LazyMap;
+import net.shibboleth.utilities.java.support.primitive.DeprecationSupport;
+import net.shibboleth.utilities.java.support.primitive.DeprecationSupport.ObjectType;
 import net.shibboleth.utilities.java.support.primitive.StringSupport;
 import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
 import net.shibboleth.utilities.java.support.xml.SerializeSupport;
@@ -77,7 +80,7 @@ import org.w3c.dom.Element;
  * <li>
  * {@link SAML2AssertionValidationParameters#CLOCK_SKEW}:
  * Optional.
- * If not present the default clock skew of {@link SAML20AssertionValidator#DEFAULT_CLOCK_SKEW} milliseconds 
+ * If not present the default clock skew of {@link SAML20AssertionValidator#DEFAULT_CLOCK_SKEW} 
  * will be used.
  * </li>
  * </ul>
@@ -97,11 +100,11 @@ import org.w3c.dom.Element;
  * */
 public class SAML20AssertionValidator {
 
-    /** Default clock skew; {@value} milliseconds. */
-    public static final long DEFAULT_CLOCK_SKEW = 5 * 60 * 1000;
+    /** Default clock skew of 5 minutes. */
+    @Nonnull public static final Duration DEFAULT_CLOCK_SKEW = Duration.ofMinutes(5);
 
     /** Class logger. */
-    private final Logger log = LoggerFactory.getLogger(SAML20AssertionValidator.class);
+    @Nonnull private final Logger log = LoggerFactory.getLogger(SAML20AssertionValidator.class);
 
     /** Registered {@link Condition} validators. */
     private LazyMap<QName, ConditionValidator> conditionValidators;
@@ -167,20 +170,31 @@ public class SAML20AssertionValidator {
 
     /**
      * Gets the clock skew from the {@link ValidationContext#getStaticParameters()} parameters. If the parameter is not
-     * set or is not a positive {@link Long} then the {@link #DEFAULT_CLOCK_SKEW} is used.
+     * set or is not a non-zero {@link Duration} then the {@link #DEFAULT_CLOCK_SKEW} is used.
      * 
      * @param context current validation context
      * 
      * @return the clock skew
      */
-    public static long getClockSkew(@Nonnull final ValidationContext context) {
-        long clockSkew = DEFAULT_CLOCK_SKEW;
+    public static Duration getClockSkew(@Nonnull final ValidationContext context) {
+        Duration clockSkew = DEFAULT_CLOCK_SKEW;
 
         if (context.getStaticParameters().containsKey(SAML2AssertionValidationParameters.CLOCK_SKEW)) {
             try {
-                clockSkew = (Long) context.getStaticParameters().get(SAML2AssertionValidationParameters.CLOCK_SKEW);
-                if (clockSkew < 1) {
+                final Object raw = context.getStaticParameters().get(SAML2AssertionValidationParameters.CLOCK_SKEW);
+                if (raw instanceof Duration) {
+                    clockSkew = (Duration) raw;
+                } else if (raw instanceof Long) {
+                    clockSkew = Duration.ofMillis((Long) raw);
+                    // This is a V4 deprecation, remove in V5.
+                    DeprecationSupport.warn(ObjectType.CONFIGURATION, SAML2AssertionValidationParameters.CLOCK_SKEW,
+                            null, Duration.class.getName());
+                }
+                
+                if (clockSkew.isZero()) {
                     clockSkew = DEFAULT_CLOCK_SKEW;
+                } else if (clockSkew.isNegative()) {
+                    clockSkew = clockSkew.abs();
                 }
             } catch (final ClassCastException e) {
                 clockSkew = DEFAULT_CLOCK_SKEW;
@@ -473,12 +487,12 @@ public class SAML20AssertionValidator {
         }
         
         final Instant now = Instant.now();
-        final long clockSkew = getClockSkew(context);
+        final Duration clockSkew = getClockSkew(context);
 
         final Instant notBefore = conditions.getNotBefore();
         log.debug("Evaluating Conditions NotBefore '{}' against 'skewed now' time '{}'",
-                notBefore, now.plusMillis(clockSkew));
-        if (notBefore != null && notBefore.isAfter(now.plusMillis(clockSkew))) {
+                notBefore, now.plus(clockSkew));
+        if (notBefore != null && notBefore.isAfter(now.plus(clockSkew))) {
             context.setValidationFailureMessage(String.format(
                     "Assertion '%s' with NotBefore condition of '%s' is not yet valid", assertion.getID(), notBefore));
             return ValidationResult.INVALID;
@@ -486,8 +500,8 @@ public class SAML20AssertionValidator {
 
         final Instant notOnOrAfter = conditions.getNotOnOrAfter();
         log.debug("Evaluating Conditions NotOnOrAfter '{}' against 'skewed now' time '{}'",
-                notOnOrAfter, now.minusMillis(clockSkew));
-        if (notOnOrAfter != null && notOnOrAfter.isBefore(now.minusMillis(clockSkew))) {
+                notOnOrAfter, now.minus(clockSkew));
+        if (notOnOrAfter != null && notOnOrAfter.isBefore(now.minus(clockSkew))) {
             context.setValidationFailureMessage(String.format(
                     "Assertion '%s' with NotOnOrAfter condition of '%s' is no longer valid", assertion.getID(),
                     notOnOrAfter));
diff --git a/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/assertion/SAML2AssertionValidationParameters.java b/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/assertion/SAML2AssertionValidationParameters.java
index 238b33f..a43e6f2 100644
--- a/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/assertion/SAML2AssertionValidationParameters.java
+++ b/opensaml-saml-api/src/main/java/org/opensaml/saml/saml2/assertion/SAML2AssertionValidationParameters.java
@@ -36,7 +36,7 @@ public final class SAML2AssertionValidationParameters {
     public static final String STMT_INFIX = ".Statement";
     
     /**
-     * Carries a {@link java.lang.Long} specifying a clock skew value in milliseconds.
+     * Carries a {@link java.lang.Duration} specifying a clock skew value.
      */
     public static final String CLOCK_SKEW = STD_PREFIX + ".ClockSkew";
 
@@ -95,8 +95,8 @@ public final class SAML2AssertionValidationParameters {
     public static final String COND_VALID_AUDIENCES = STD_PREFIX + COND_INFIX + ".ValidAudiences";
 
     /**
-     * Carries a {@link java.lang.Long} representing the per-invocation value for the Assertion 
-     * replay cache expiration, in milliseconds.
+     * Carries a {@link java.lang.Duration} representing the per-invocation value for the Assertion 
+     * replay cache expiration.
      */
     public static final String COND_ONE_TIME_USE_EXPIRES = STD_PREFIX + COND_INFIX + ".OneTimeUseExpires";
     
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/common/profile/impl/AddNotOnOrAfterConditionToAssertions.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/common/profile/impl/AddNotOnOrAfterConditionToAssertions.java
index 345426c..3e11122 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/common/profile/impl/AddNotOnOrAfterConditionToAssertions.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/common/profile/impl/AddNotOnOrAfterConditionToAssertions.java
@@ -93,9 +93,9 @@ public class AddNotOnOrAfterConditionToAssertions extends AbstractConditionalPro
     }
     
     /**
-     * Set the default assertion lifetime in milliseconds.
+     * Set the default assertion lifetime.
      * 
-     * @param lifetime  default lifetime in milliseconds
+     * @param lifetime  default lifetime
      */
     public void setDefaultAssertionLifetime(@Nonnull final Duration lifetime) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicMetadataResolver.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicMetadataResolver.java
index f92fcd7..fe95dce 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicMetadataResolver.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/AbstractDynamicMetadataResolver.java
@@ -148,7 +148,7 @@ public abstract class AbstractDynamicMetadataResolver extends AbstractMetadataRe
     /** Factor used to compute when the next refresh interval will occur. Default value: 0.75 */
     @Positive private Float refreshDelayFactor;
     
-    /** The maximum idle time in milliseconds for which the resolver will keep data for a given entityID, 
+    /** The maximum idle time for which the resolver will keep data for a given entityID, 
      * before it is removed. */
     @Nonnull private Duration maxIdleEntityData;
     
@@ -156,7 +156,7 @@ public abstract class AbstractDynamicMetadataResolver extends AbstractMetadataRe
     private boolean removeIdleEntityData;
     
     /** Impending expiration warning threshold for metadata refresh. 
-     * Default value: 0ms (disabled). */
+     * Default value: 0 (disabled). */
     @Nonnull private Duration expirationWarningThreshold;
     
     /** The interval at which the cleanup task should run. */
@@ -1674,7 +1674,7 @@ public abstract class AbstractDynamicMetadataResolver extends AbstractMetadataRe
         /** Time at which should start attempting to refresh the metadata. */
         private Instant refreshTriggerTime;
         
-        /** The last time in milliseconds at which the entity's backing store data was accessed. */
+        /** The last time at which the entity's backing store data was accessed. */
         private Instant lastAccessedTime;
         
         /** The time at which the negative lookup cache flag expires, if set. */
@@ -1762,7 +1762,7 @@ public abstract class AbstractDynamicMetadataResolver extends AbstractMetadataRe
         /**
          * Get the last time at which the entity's backing store data was accessed.
          * 
-         * @return the time in milliseconds since the epoch
+         * @return last access time
          */
         @Nonnull public Instant getLastAccessedTime() {
             return lastAccessedTime;
@@ -1837,7 +1837,7 @@ public abstract class AbstractDynamicMetadataResolver extends AbstractMetadataRe
 
         /**
          *  Purge metadata which is either 1) expired or 2) (if {@link #isRemoveIdleEntityData()} is true) 
-         *  which hasn't been accessed within the last {@link #getMaxIdleEntityData()} milliseconds.
+         *  which hasn't been accessed within the last {@link #getMaxIdleEntityData()} duration.
          */
         private void removeExpiredAndIdleMetadata() {
             final Instant now = Instant.now();
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/assertion/impl/AbstractSubjectConfirmationValidator.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/assertion/impl/AbstractSubjectConfirmationValidator.java
index a6c3cdb..4fe74d1 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/assertion/impl/AbstractSubjectConfirmationValidator.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/assertion/impl/AbstractSubjectConfirmationValidator.java
@@ -124,7 +124,7 @@ public abstract class AbstractSubjectConfirmationValidator implements SubjectCon
     @Nonnull protected ValidationResult validateNotBefore(@Nonnull final SubjectConfirmation confirmation, 
             @Nonnull final Assertion assertion, @Nonnull final ValidationContext context) 
                     throws AssertionValidationException {
-        final Instant skewedNow = Instant.now().plusMillis(SAML20AssertionValidator.getClockSkew(context));
+        final Instant skewedNow = Instant.now().plus(SAML20AssertionValidator.getClockSkew(context));
         final Instant notBefore = confirmation.getSubjectConfirmationData().getNotBefore();
         
         log.debug("Evaluating SubjectConfirmationData NotBefore '{}' against 'skewed now' time '{}'",
@@ -155,7 +155,7 @@ public abstract class AbstractSubjectConfirmationValidator implements SubjectCon
     @Nonnull protected ValidationResult validateNotOnOrAfter(@Nonnull final SubjectConfirmation confirmation, 
             @Nonnull final Assertion assertion, @Nonnull final ValidationContext context) 
                     throws AssertionValidationException {
-        final Instant skewedNow = Instant.now().minusMillis(SAML20AssertionValidator.getClockSkew(context));
+        final Instant skewedNow = Instant.now().minus(SAML20AssertionValidator.getClockSkew(context));
         final Instant notOnOrAfter = confirmation.getSubjectConfirmationData().getNotOnOrAfter();
         
         log.debug("Evaluating SubjectConfirmationData NotOnOrAfter '{}' against 'skewed now' time '{}'",
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/assertion/impl/OneTimeUseConditionValidator.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/assertion/impl/OneTimeUseConditionValidator.java
index e1697fb..7de042c 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/assertion/impl/OneTimeUseConditionValidator.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/assertion/impl/OneTimeUseConditionValidator.java
@@ -28,6 +28,8 @@ import javax.xml.namespace.QName;
 
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
 import net.shibboleth.utilities.java.support.logic.Constraint;
+import net.shibboleth.utilities.java.support.primitive.DeprecationSupport;
+import net.shibboleth.utilities.java.support.primitive.DeprecationSupport.ObjectType;
 import net.shibboleth.utilities.java.support.primitive.StringSupport;
 
 import org.opensaml.saml.common.assertion.AssertionValidationException;
@@ -82,9 +84,8 @@ public class OneTimeUseConditionValidator implements ConditionValidator {
      * Constructor.
      * 
      * @param replay reply cache used to track which assertions have been used
-     * @param expires time (in milliseconds since beginning of epoch) for disposal of tracked
-     *         assertion from the replay cache.  May be null, then defaults to 
-     *         {@link #DEFAULT_CACHE_EXPIRES}.
+     * @param expires time for disposal of tracked assertion from the replay cache.
+     *      May be null, then defaults to 8 hours
      */
     public OneTimeUseConditionValidator(@Nonnull final ReplayCache replay, @Nullable final Duration expires) {
         replayCache = Constraint.isNotNull(replay, "Replay cache was null");
@@ -151,25 +152,29 @@ public class OneTimeUseConditionValidator implements ConditionValidator {
      * @return the effective one-time use expiration for the assertion being evaluated
      */
     @Nonnull protected Instant getExpires(final Assertion assertion, final ValidationContext context) {
-        Long expires = null;
-        try {
-            expires = (Long) context.getStaticParameters().get(
-                    SAML2AssertionValidationParameters.COND_ONE_TIME_USE_EXPIRES);
-        } catch (final ClassCastException e) {
-            log.warn("Value of param was not a Long: {}", SAML2AssertionValidationParameters.COND_ONE_TIME_USE_EXPIRES);
+        Duration expires = null;
+        final Object raw = context.getStaticParameters().get(
+                SAML2AssertionValidationParameters.COND_ONE_TIME_USE_EXPIRES);
+        if (raw instanceof Duration) {
+            expires = (Duration) raw;
+        } else if (raw instanceof Long) {
+            expires = Duration.ofMillis((Long) raw);
+            DeprecationSupport.warn(ObjectType.CONFIGURATION,
+                    SAML2AssertionValidationParameters.COND_ONE_TIME_USE_EXPIRES, null, Duration.class.getName());
         }
+        
         log.debug("Saw one-time use cache expires context param: {}", expires);
         
         Duration suppliedExpiration = null;
         
-        if (expires == null) {
+        if (expires == null || expires.isZero()) {
             suppliedExpiration = getReplayCacheExpires();
-        } else if (expires < 0) {
+        } else if (expires.isNegative()) {
             log.warn("Supplied context param for replay cache expires '{}' was negative, using configured expiration", 
                     expires);
             suppliedExpiration = getReplayCacheExpires();
         } else {
-            suppliedExpiration = Duration.ofMillis(expires);
+            suppliedExpiration = expires;
         }
              
         log.debug("Effective one-time use cache expires of: {}", suppliedExpiration);
diff --git a/opensaml-soap-impl/src/main/java/org/opensaml/soap/wssecurity/messaging/impl/AddTimestampHandler.java b/opensaml-soap-impl/src/main/java/org/opensaml/soap/wssecurity/messaging/impl/AddTimestampHandler.java
index d27c907..68fdc3e 100644
--- a/opensaml-soap-impl/src/main/java/org/opensaml/soap/wssecurity/messaging/impl/AddTimestampHandler.java
+++ b/opensaml-soap-impl/src/main/java/org/opensaml/soap/wssecurity/messaging/impl/AddTimestampHandler.java
@@ -17,6 +17,7 @@
 
 package org.opensaml.soap.wssecurity.messaging.impl;
 
+import java.time.Duration;
 import java.time.Instant;
 import java.util.function.Function;
 
@@ -39,7 +40,7 @@ import org.slf4j.LoggerFactory;
 
 /**
  * Handler implementation that adds a wsse:Timestamp header to the wsse:Security header
- *  of the outbound SOAP envelope.
+ * of the outbound SOAP envelope.
  */
 public class AddTimestampHandler extends AbstractHeaderGeneratingMessageHandler {
     
@@ -56,15 +57,15 @@ public class AddTimestampHandler extends AbstractHeaderGeneratingMessageHandler
      * is explicitly supplied by the other supported mechanisms. */
     private boolean useCurrentTimeAsDefaultCreated;
     
-    /** Parameter indicating the offset from Created, in milliseconds, used to calculate the Expires time, 
+    /** Parameter indicating the offset from Created used to calculate the Expires time, 
      * if no Expires value is explicitly supplied via the other supported mechanisms. */
-    private Long expiresOffsetFromCreated;
+    @Nullable private Duration expiresOffsetFromCreated;
     
     /** The effective Created value to use. */
-    private Instant createdValue;
+    @Nullable private Instant createdValue;
     
     /** The effective Expires value to use. */
-    private Instant expiresValue;
+    @Nullable private Instant expiresValue;
     
     /**
      * Get the context lookup function for the Created time.
@@ -129,28 +130,29 @@ public class AddTimestampHandler extends AbstractHeaderGeneratingMessageHandler
     }
     
     /**
-     * Get the parameter indicating the offset from Created, in milliseconds, used to calculate the Expires time, 
+     * Get the parameter indicating the offset from Created used to calculate the Expires time, 
      * if no Expires value is explicitly supplied via the other supported mechanisms. 
      * 
      * @return the expires offset, or null
      */
-    @Nullable public Long getExpiresOffsetFromCreated() {
+    @Nullable public Duration getExpiresOffsetFromCreated() {
         return expiresOffsetFromCreated;
     }
 
     /**
-     * Set the parameter indicating the offset from Created, in milliseconds, used to calculate the Expires time, 
+     * Set the parameter indicating the offset from Created used to calculate the Expires time, 
      * if no Expires value is explicitly supplied via the other supported mechanisms. 
      * 
-     * @param value the expires off set, or null
+     * @param value the expires offset, or null
      */
-    public void setExpiresOffsetFromCreated(@Nullable final Long value) {
+    public void setExpiresOffsetFromCreated(@Nullable final Duration value) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
         ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
         expiresOffsetFromCreated = value;
     }
 
     /** {@inheritDoc} */
+    @Override
     protected boolean doPreInvoke(@Nonnull final MessageContext messageContext) throws MessageHandlerException {
         if (!super.doPreInvoke(messageContext)) {
             return false;
@@ -236,10 +238,10 @@ public class AddTimestampHandler extends AbstractHeaderGeneratingMessageHandler
         
         if (value == null) {
             if (getExpiresOffsetFromCreated() != null && created != null) {
-                return created.plusMillis(getExpiresOffsetFromCreated());
+                return created.plus(getExpiresOffsetFromCreated());
             }
         }
         return value;
     }
 
-}
+}
\ No newline at end of file
diff --git a/opensaml-soap-impl/src/test/java/org/opensaml/soap/wssecurity/messaging/impl/AddTimestampHandlerTest.java b/opensaml-soap-impl/src/test/java/org/opensaml/soap/wssecurity/messaging/impl/AddTimestampHandlerTest.java
index 162d966..5347d69 100644
--- a/opensaml-soap-impl/src/test/java/org/opensaml/soap/wssecurity/messaging/impl/AddTimestampHandlerTest.java
+++ b/opensaml-soap-impl/src/test/java/org/opensaml/soap/wssecurity/messaging/impl/AddTimestampHandlerTest.java
@@ -17,6 +17,7 @@
 
 package org.opensaml.soap.wssecurity.messaging.impl;
 
+import java.time.Duration;
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
 
@@ -56,7 +57,7 @@ public class AddTimestampHandlerTest extends SOAPMessagingBaseTestCase {
     @Test
     public void testNoInputUsingCurrentTimeAndOffset() throws ComponentInitializationException, MessageHandlerException {
         handler.setUseCurrentTimeAsDefaultCreated(true);
-        handler.setExpiresOffsetFromCreated(5*60*1000L);
+        handler.setExpiresOffsetFromCreated(Duration.ofMinutes(5));
         
         handler.initialize();
         handler.invoke(getMessageContext());
@@ -135,7 +136,7 @@ public class AddTimestampHandlerTest extends SOAPMessagingBaseTestCase {
         Instant created = Instant.now();
         Instant expires = created.plus(5, ChronoUnit.MINUTES);
         getMessageContext().getSubcontext(WSSecurityContext.class, true).setTimestampCreated(created);
-        handler.setExpiresOffsetFromCreated(5*60*1000L);
+        handler.setExpiresOffsetFromCreated(Duration.ofMinutes(5));
         
         handler.initialize();
         handler.invoke(getMessageContext());
@@ -177,7 +178,7 @@ public class AddTimestampHandlerTest extends SOAPMessagingBaseTestCase {
         Instant created = Instant.now();
         Instant expires = created.plus(5, ChronoUnit.MINUTES);
         handler.setCreatedLookup(FunctionSupport.constant(created));
-        handler.setExpiresOffsetFromCreated(5*60*1000L);
+        handler.setExpiresOffsetFromCreated(Duration.ofMinutes(5));
         
         
         handler.initialize();

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


More information about the commits mailing list