You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ws.apache.org by co...@apache.org on 2020/04/14 07:34:37 UTC

[ws-wss4j] 02/02: Adding more expiration unit tests + fixing the caching logic

This is an automated email from the ASF dual-hosted git repository.

coheigea pushed a commit to branch WSS-632
in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git

commit 43a9d8d04c0cce0c3773c209230ebcd0978877d8
Author: Colm O hEigeartaigh <co...@apache.org>
AuthorDate: Fri Apr 10 10:29:03 2020 +0100

    Adding more expiration unit tests + fixing the caching logic
---
 .../apache/wss4j/common/cache/EHCacheExpiry.java   |  16 ++--
 .../wss4j/common/cache/EHCacheReplayCache.java     |  11 +--
 .../apache/wss4j/common/cache/EHCacheValue.java    |  11 +--
 .../wss4j/common/cache/EHCacheExpiryTest.java      | 100 +++++++++++++++++++++
 .../apache/wss4j/common/cache/ReplayCacheTest.java |  55 ++++++++++++
 5 files changed, 176 insertions(+), 17 deletions(-)

diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/cache/EHCacheExpiry.java b/ws-security-common/src/main/java/org/apache/wss4j/common/cache/EHCacheExpiry.java
index 33e13dd..721931c 100644
--- a/ws-security-common/src/main/java/org/apache/wss4j/common/cache/EHCacheExpiry.java
+++ b/ws-security-common/src/main/java/org/apache/wss4j/common/cache/EHCacheExpiry.java
@@ -20,6 +20,7 @@
 package org.apache.wss4j.common.cache;
 
 import java.time.Duration;
+import java.time.Instant;
 import java.time.temporal.ChronoUnit;
 import java.util.function.Supplier;
 
@@ -32,25 +33,26 @@ import org.ehcache.expiry.ExpiryPolicy;
 public class EHCacheExpiry implements ExpiryPolicy<String, EHCacheValue> {
 
     /**
-     * The default time to live (60 minutes)
+     * The default time to live in seconds (60 minutes)
      */
     public static final long DEFAULT_TTL = 3600L;
 
     /**
-     * The max time to live (12 hours)
+     * The max time to live in seconds (12 hours)
      */
     public static final long MAX_TTL = DEFAULT_TTL * 12L;
 
 
     @Override
     public Duration getExpiryForCreation(String s, EHCacheValue ehCacheValue) {
-        long parsedTTL = ehCacheValue.getExpiry();
-        if (parsedTTL <= 0 || parsedTTL > MAX_TTL) {
-            // Use default value
-            parsedTTL = DEFAULT_TTL;
+        Instant expiry = ehCacheValue.getExpiry();
+        Instant now = Instant.now();
+
+        if (expiry == null || expiry.isBefore(now) || expiry.isAfter(now.plusSeconds(MAX_TTL))) {
+            return Duration.of(DEFAULT_TTL, ChronoUnit.SECONDS);
         }
 
-        return Duration.of(parsedTTL, ChronoUnit.SECONDS);
+        return Duration.of(expiry.toEpochMilli() - now.toEpochMilli(), ChronoUnit.MILLIS);
     }
 
     @Override
diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/cache/EHCacheReplayCache.java b/ws-security-common/src/main/java/org/apache/wss4j/common/cache/EHCacheReplayCache.java
index 08aceaa..222f2a8 100644
--- a/ws-security-common/src/main/java/org/apache/wss4j/common/cache/EHCacheReplayCache.java
+++ b/ws-security-common/src/main/java/org/apache/wss4j/common/cache/EHCacheReplayCache.java
@@ -104,11 +104,7 @@ public class EHCacheReplayCache implements ReplayCache {
             return;
         }
 
-        long ttl = 0;
-        if (expiry != null) {
-            ttl = expiry.getEpochSecond() - Instant.now().getEpochSecond();
-        }
-        cache.put(identifier, new EHCacheValue(identifier, ttl));
+        cache.put(identifier, new EHCacheValue(identifier, expiry));
     }
 
     /**
@@ -123,6 +119,11 @@ public class EHCacheReplayCache implements ReplayCache {
         return element != null;
     }
 
+    // Only exposed for testing
+    EHCacheValue get(String identifier) {
+        return cache.get(identifier);
+    }
+
     @Override
     public synchronized void close() {
         if (cacheManager.getStatus() == Status.AVAILABLE) {
diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/cache/EHCacheValue.java b/ws-security-common/src/main/java/org/apache/wss4j/common/cache/EHCacheValue.java
index 352b71f..65d460e 100644
--- a/ws-security-common/src/main/java/org/apache/wss4j/common/cache/EHCacheValue.java
+++ b/ws-security-common/src/main/java/org/apache/wss4j/common/cache/EHCacheValue.java
@@ -6,9 +6,9 @@
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License. You may obtain a copy of the License at
- *
+ * <p>
  * http://www.apache.org/licenses/LICENSE-2.0
- *
+ * <p>
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -20,6 +20,7 @@
 package org.apache.wss4j.common.cache;
 
 import java.io.Serializable;
+import java.time.Instant;
 
 /**
  * A cache value for EHCache. It contains the identifier to be cached as well as a custom expiry.
@@ -27,9 +28,9 @@ import java.io.Serializable;
 public class EHCacheValue implements Serializable {
 
     private final String identifier;
-    private final long expiry;
+    private final Instant expiry;
 
-    public EHCacheValue(String identifier, long expiry) {
+    public EHCacheValue(String identifier, Instant expiry) {
         this.identifier = identifier;
         this.expiry = expiry;
     }
@@ -38,7 +39,7 @@ public class EHCacheValue implements Serializable {
         return identifier;
     }
 
-    public long getExpiry() {
+    public Instant getExpiry() {
         return expiry;
     }
 
diff --git a/ws-security-common/src/test/java/org/apache/wss4j/common/cache/EHCacheExpiryTest.java b/ws-security-common/src/test/java/org/apache/wss4j/common/cache/EHCacheExpiryTest.java
new file mode 100644
index 0000000..fd3b3ca
--- /dev/null
+++ b/ws-security-common/src/test/java/org/apache/wss4j/common/cache/EHCacheExpiryTest.java
@@ -0,0 +1,100 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.wss4j.common.cache;
+
+import java.time.Duration;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Some unit tests for the EHCacheExpiry implementation
+ */
+public class EHCacheExpiryTest {
+
+    @Test
+    public void testNoExpirySpecified() {
+        EHCacheExpiry cacheExpiry = new EHCacheExpiry();
+
+        Duration expiryForCreation =
+                cacheExpiry.getExpiryForCreation("xyz",
+                        new EHCacheValue("xyz", null));
+        assertNotNull(expiryForCreation);
+
+        assertEquals(EHCacheExpiry.DEFAULT_TTL, expiryForCreation.getSeconds());
+    }
+
+    @Test
+    public void testExpirySpecified() {
+        EHCacheExpiry cacheExpiry = new EHCacheExpiry();
+
+        Duration expiryForCreation =
+                cacheExpiry.getExpiryForCreation("xyz",
+                        new EHCacheValue("xyz", Instant.now().plusSeconds(30L)));
+        assertNotNull(expiryForCreation);
+
+        // Some loose boundary checking to allow for slow tests
+        assertTrue(expiryForCreation.getSeconds() <= 30L);
+        assertTrue(expiryForCreation.getSeconds() > 30L - 5L);
+    }
+
+    @Test
+    public void testExpirySpecified2() {
+        EHCacheExpiry cacheExpiry = new EHCacheExpiry();
+
+        Duration expiryForCreation =
+                cacheExpiry.getExpiryForCreation("xyz",
+                        new EHCacheValue("xyz", Instant.now().plus(6L, ChronoUnit.HOURS)));
+        assertNotNull(expiryForCreation);
+
+        // Some loose boundary checking to allow for slow tests
+        assertTrue(expiryForCreation.getSeconds() <= 6 * 60 * 60L);
+        assertTrue(expiryForCreation.getSeconds() > 6 * 60 * 60L - 5L);
+    }
+
+    @Test
+    public void testNegativeExpirySpecified() {
+        EHCacheExpiry cacheExpiry = new EHCacheExpiry();
+
+        Duration expiryForCreation =
+                cacheExpiry.getExpiryForCreation("xyz",
+                        new EHCacheValue("xyz", Instant.now().minusSeconds(30L)));
+        assertNotNull(expiryForCreation);
+
+        assertEquals(EHCacheExpiry.DEFAULT_TTL, expiryForCreation.getSeconds());
+    }
+
+    @Test
+    public void testHugeExpirySpecified() {
+        EHCacheExpiry cacheExpiry = new EHCacheExpiry();
+
+        Duration expiryForCreation =
+                cacheExpiry.getExpiryForCreation("xyz",
+                        new EHCacheValue("xyz", Instant.now().plus(14, ChronoUnit.HOURS)));
+        assertNotNull(expiryForCreation);
+
+        assertEquals(EHCacheExpiry.DEFAULT_TTL, expiryForCreation.getSeconds());
+    }
+}
\ No newline at end of file
diff --git a/ws-security-common/src/test/java/org/apache/wss4j/common/cache/ReplayCacheTest.java b/ws-security-common/src/test/java/org/apache/wss4j/common/cache/ReplayCacheTest.java
index fb7347f..90778a7 100644
--- a/ws-security-common/src/test/java/org/apache/wss4j/common/cache/ReplayCacheTest.java
+++ b/ws-security-common/src/test/java/org/apache/wss4j/common/cache/ReplayCacheTest.java
@@ -22,11 +22,15 @@ package org.apache.wss4j.common.cache;
 import java.io.IOException;
 import java.net.URL;
 import java.time.Instant;
+import java.time.temporal.ChronoUnit;
 import java.util.UUID;
 
 import org.junit.jupiter.api.Test;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
@@ -73,6 +77,57 @@ public class ReplayCacheTest {
         replayCache.close();
     }
 
+    // No expiry specified so it falls back to the default
+    @Test
+    public void testEhCacheReplayCacheNoExpirySpecified() throws Exception {
+        ReplayCache replayCache = new EHCacheReplayCache("xyz", (URL)null);
+
+        String id = UUID.randomUUID().toString();
+        replayCache.add(id);
+        assertTrue(replayCache.contains(id));
+
+        EHCacheValue ehCacheValue = ((EHCacheReplayCache) replayCache).get(id);
+        assertNotNull(ehCacheValue);
+        assertNull(ehCacheValue.getExpiry());
+        assertEquals(id, ehCacheValue.getIdentifier());
+
+        replayCache.close();
+    }
+
+    // The negative expiry is rejected and it falls back to the default
+    @Test
+    public void testEhCacheReplayCacheNegativeExpiry() throws Exception {
+        ReplayCache replayCache = new EHCacheReplayCache("xyz", (URL)null);
+
+        String id = UUID.randomUUID().toString();
+        replayCache.add(id, Instant.now().minusSeconds(100L));
+        assertTrue(replayCache.contains(id));
+
+        EHCacheValue ehCacheValue = ((EHCacheReplayCache) replayCache).get(id);
+        assertNotNull(ehCacheValue);
+        assertNotNull(ehCacheValue.getExpiry());
+        assertEquals(id, ehCacheValue.getIdentifier());
+
+        replayCache.close();
+    }
+
+    // The huge expiry is rejected and it falls back to the default
+    @Test
+    public void testEhCacheReplayCacheHugeExpiry() throws Exception {
+        ReplayCache replayCache = new EHCacheReplayCache("xyz", (URL)null);
+
+        String id = UUID.randomUUID().toString();
+        replayCache.add(id, Instant.now().plus(14, ChronoUnit.HOURS));
+        assertTrue(replayCache.contains(id));
+
+        EHCacheValue ehCacheValue = ((EHCacheReplayCache) replayCache).get(id);
+        assertNotNull(ehCacheValue);
+        assertNotNull(ehCacheValue.getExpiry());
+        assertEquals(id, ehCacheValue.getIdentifier());
+
+        replayCache.close();
+    }
+
     private void testReplayCacheInstance(ReplayCache replayCache) throws InterruptedException, IOException {
 
         // Test default TTL caches OK