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/05/22 16:00:22 UTC

[ws-wss4j] 02/03: Refactor caching so that it doesn't use XML

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

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

commit b73e1dac1320bf7ccb13fb3838006bde80a83c86
Author: Colm O hEigeartaigh <co...@apache.org>
AuthorDate: Fri May 22 16:05:04 2020 +0100

    Refactor caching so that it doesn't use XML
---
 .../wss4j/common/cache/EHCacheReplayCache.java     | 102 ++++++++++++++-------
 .../apache/wss4j/common/cache/WSS4JCacheUtil.java  |  24 -----
 .../src/main/resources/wss4j-ehcache.xml           |  19 ----
 .../apache/wss4j/common/cache/ReplayCacheTest.java |  68 ++++++++++++--
 .../apache/wss4j/dom/common/SecurityTestUtil.java  |  27 +++---
 .../org/apache/wss4j/stax/test/ReplayTest.java     |   7 +-
 .../apache/wss4j/stax/test/UsernameTokenTest.java  |   5 +-
 7 files changed, 144 insertions(+), 108 deletions(-)

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 159b589..edfcc24 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
@@ -19,45 +19,84 @@
 
 package org.apache.wss4j.common.cache;
 
-import java.io.IOException;
-import java.net.URL;
+import java.io.File;
 import java.nio.file.Path;
 import java.time.Instant;
 
 import org.apache.wss4j.common.ext.WSSecurityException;
-import org.apache.wss4j.common.util.Loader;
 import org.ehcache.Cache;
 import org.ehcache.CacheManager;
+import org.ehcache.CachePersistenceException;
+import org.ehcache.PersistentCacheManager;
 import org.ehcache.Status;
 import org.ehcache.config.builders.CacheConfigurationBuilder;
 import org.ehcache.config.builders.CacheManagerBuilder;
-import org.ehcache.xml.XmlConfiguration;
+import org.ehcache.config.builders.ResourcePoolsBuilder;
+import org.ehcache.config.units.EntryUnit;
+import org.ehcache.config.units.MemoryUnit;
 
 /**
- * An in-memory EHCache implementation of the ReplayCache interface. The default TTL is 60 minutes and the
- * max TTL is 12 hours.
+ * An in-memory EHCache implementation of the ReplayCache interface, that overflows to disk.
+ * The default TTL is 60 minutes and the max TTL is 12 hours.
  */
 public class EHCacheReplayCache implements ReplayCache {
 
     private static final org.slf4j.Logger LOG =
             org.slf4j.LoggerFactory.getLogger(EHCacheReplayCache.class);
-    private static final String CACHE_TEMPLATE_NAME = "wss4jCache";
 
     private final Cache<String, EHCacheValue> cache;
     private final CacheManager cacheManager;
     private final String key;
+    private final Path diskstorePath;
+    private final boolean persistent;
 
-    public EHCacheReplayCache(String key, URL configFileURL, Path diskstorePath) throws WSSecurityException {
+    public EHCacheReplayCache(String key) throws WSSecurityException {
+        this(key, null);
+    }
+
+    public EHCacheReplayCache(String key, Path diskstorePath) throws WSSecurityException {
+        this(key, diskstorePath, 50, 10000, false);
+    }
+
+    public EHCacheReplayCache(String key, Path diskstorePath, long diskSize, long heapEntries, boolean persistent)
+            throws WSSecurityException {
         this.key = key;
+        this.diskstorePath = diskstorePath;
+        this.persistent = persistent;
+
+        // Do some sanity checking on the arguments
+        if (key == null || persistent && diskstorePath == null) {
+            throw new NullPointerException();
+        }
+        if (diskstorePath != null && (diskSize < 5 || diskSize > 10000)) {
+            throw new IllegalArgumentException("The diskSize parameter must be between 5 and 10000 (megabytes)");
+        }
+        if (heapEntries < 100) {
+            throw new IllegalArgumentException("The heapEntries parameter must be greater than 100 (entries)");
+        }
+
         try {
-            XmlConfiguration xmlConfig = new XmlConfiguration(getConfigFileURL(configFileURL));
+            ResourcePoolsBuilder resourcePoolsBuilder = ResourcePoolsBuilder.newResourcePoolsBuilder()
+                    .heap(heapEntries, EntryUnit.ENTRIES);
+            if (diskstorePath != null) {
+                resourcePoolsBuilder = resourcePoolsBuilder.disk(diskSize, MemoryUnit.MB, persistent);
+            }
+
             CacheConfigurationBuilder<String, EHCacheValue> configurationBuilder =
-                    xmlConfig.newCacheConfigurationBuilderFromTemplate(CACHE_TEMPLATE_NAME, String.class, EHCacheValue.class);
-            CacheManagerBuilder builder = CacheManagerBuilder.newCacheManagerBuilder().withCache(key, configurationBuilder);
+                    CacheConfigurationBuilder.newCacheConfigurationBuilder(
+                            String.class, EHCacheValue.class, resourcePoolsBuilder)
+                            .withExpiry(new EHCacheExpiry());
+
             if (diskstorePath != null) {
-                builder = builder.with(CacheManagerBuilder.persistence(diskstorePath.toFile()));
+                cacheManager = CacheManagerBuilder.newCacheManagerBuilder()
+                        .with(CacheManagerBuilder.persistence(diskstorePath.toFile()))
+                        .withCache(key, configurationBuilder)
+                        .build();
+            } else {
+                cacheManager = CacheManagerBuilder.newCacheManagerBuilder()
+                        .withCache(key, configurationBuilder)
+                        .build();
             }
-            cacheManager = builder.build();
 
             cacheManager.init();
             cache = cacheManager.getCache(key, String.class, EHCacheValue.class);
@@ -67,25 +106,6 @@ public class EHCacheReplayCache implements ReplayCache {
         }
     }
 
-    private URL getConfigFileURL(URL suppliedConfigFileURL) {
-        if (suppliedConfigFileURL == null) {
-            //using the default
-            String defaultConfigFile = "/wss4j-ehcache.xml";
-            URL configFileURL = null;
-            try {
-                configFileURL = Loader.getResource(defaultConfigFile);
-                if (configFileURL == null) {
-                    configFileURL = new URL(defaultConfigFile);
-                }
-                return configFileURL;
-            } catch (IOException e) {
-                // Do nothing
-                LOG.debug(e.getMessage());
-            }
-        }
-        return suppliedConfigFileURL;
-    }
-
     /**
      * Add the given identifier to the cache. It will be cached for a default amount of time.
      * @param identifier The identifier to be added
@@ -128,7 +148,25 @@ public class EHCacheReplayCache implements ReplayCache {
     public synchronized void close() {
         if (cacheManager.getStatus() == Status.AVAILABLE) {
             cacheManager.removeCache(key);
+
             cacheManager.close();
+
+            if (!persistent && cacheManager instanceof PersistentCacheManager) {
+                try {
+                    ((PersistentCacheManager) cacheManager).destroy();
+                } catch (CachePersistenceException e) {
+                    LOG.debug("Error in shutting down persistent cache", e);
+                }
+
+                // As we're not using a persistent disk store, just delete it - it should be empty after calling
+                // destroy above
+                if (diskstorePath != null) {
+                    File file = diskstorePath.toFile();
+                    if (file.exists() && file.canWrite()) {
+                        file.delete();
+                    }
+                }
+            }
         }
     }
 
diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/cache/WSS4JCacheUtil.java b/ws-security-common/src/main/java/org/apache/wss4j/common/cache/WSS4JCacheUtil.java
index 38bd8ab..12f95f8 100644
--- a/ws-security-common/src/main/java/org/apache/wss4j/common/cache/WSS4JCacheUtil.java
+++ b/ws-security-common/src/main/java/org/apache/wss4j/common/cache/WSS4JCacheUtil.java
@@ -19,11 +19,6 @@
 
 package org.apache.wss4j.common.cache;
 
-import java.io.IOException;
-import java.net.URL;
-
-import org.apache.wss4j.common.util.Loader;
-
 /**
  * Some functionality to detect if EhCache is available or not.
  */
@@ -55,23 +50,4 @@ public final class WSS4JCacheUtil {
         return EH_CACHE_INSTALLED;
     }
 
-    public static URL getConfigFileURL(Object o) {
-        if (o instanceof String) {
-            try {
-                URL url = Loader.getResource((String)o);
-                if (url == null) {
-                    url = new URL((String)o);
-                }
-                return url;
-            } catch (IOException e) {
-                // Do nothing
-                LOG.debug(e.getMessage());
-            }
-        } else if (o instanceof URL) {
-            return (URL)o;
-        }
-        return null;
-    }
-
-
 }
diff --git a/ws-security-common/src/main/resources/wss4j-ehcache.xml b/ws-security-common/src/main/resources/wss4j-ehcache.xml
deleted file mode 100644
index 86fc27b..0000000
--- a/ws-security-common/src/main/resources/wss4j-ehcache.xml
+++ /dev/null
@@ -1,19 +0,0 @@
-<config
-    xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance'
-    xmlns='http://www.ehcache.org/v3'
-    xsi:schemaLocation="http://www.ehcache.org/v3 http://www.ehcache.org/schema/ehcache-core-3.8.xsd">
-
-    <cache-template name="wss4jCache">
-        <key-type>java.lang.String</key-type>
-        <value-type>org.apache.wss4j.common.cache.EHCacheValue</value-type>
-        <expiry>
-            <class>org.apache.wss4j.common.cache.EHCacheExpiry</class>
-        </expiry>
-        <resources>
-            <heap unit="entries">5000</heap>
-            <disk unit="MB" persistent="false">10</disk>
-        </resources>
-
-    </cache-template>
-
-</config>
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 60d264c..cd41960 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
@@ -21,13 +21,13 @@ package org.apache.wss4j.common.cache;
 
 import java.io.File;
 import java.io.IOException;
-import java.net.URL;
 import java.nio.file.Path;
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
 import java.util.Random;
 import java.util.UUID;
 
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -52,7 +52,16 @@ public class ReplayCacheTest {
 
     @Test
     public void testEhCacheReplayCache() throws Exception {
-        ReplayCache replayCache = new EHCacheReplayCache("xyz", (URL)null, getDiskstorePath("abc"));
+        ReplayCache replayCache = new EHCacheReplayCache("xyz", getDiskstorePath("abc"));
+
+        testReplayCacheInstance(replayCache);
+
+        replayCache.close();
+    }
+
+    @Test
+    public void testEhCacheReplayCacheNoPath() throws Exception {
+        ReplayCache replayCache = new EHCacheReplayCache("xyz");
 
         testReplayCacheInstance(replayCache);
 
@@ -61,9 +70,9 @@ public class ReplayCacheTest {
 
     @Test
     public void testEhCacheDifferentCaches() throws Exception {
-        ReplayCache replayCache = new EHCacheReplayCache("abc", (URL)null, getDiskstorePath("abc"));
+        ReplayCache replayCache = new EHCacheReplayCache("abc", getDiskstorePath("abc"));
 
-        ReplayCache replayCache2 = new EHCacheReplayCache("cba", (URL)null, getDiskstorePath("cba"));
+        ReplayCache replayCache2 = new EHCacheReplayCache("cba", getDiskstorePath("cba"));
 
         String id = UUID.randomUUID().toString();
         replayCache.add(id);
@@ -75,8 +84,21 @@ public class ReplayCacheTest {
     }
 
     @Test
+    public void testOverflowToDisk() throws Exception {
+        ReplayCache replayCache = new EHCacheReplayCache("abc", getDiskstorePath("abc"));
+        
+        for (int i = 0; i < 10050; i++) {
+            String id = Integer.toString(i);
+            replayCache.add(id);
+            assertTrue(replayCache.contains(id));
+        }
+
+        replayCache.close();
+    }
+
+    @Test
     public void testEhCacheCloseCacheTwice() throws Exception {
-        ReplayCache replayCache = new EHCacheReplayCache("abc", (URL)null, getDiskstorePath("abc"));
+        ReplayCache replayCache = new EHCacheReplayCache("abc", getDiskstorePath("abc"));
         replayCache.close();
         replayCache.close();
     }
@@ -84,7 +106,7 @@ public class ReplayCacheTest {
     // No expiry specified so it falls back to the default
     @Test
     public void testEhCacheReplayCacheNoExpirySpecified() throws Exception {
-        ReplayCache replayCache = new EHCacheReplayCache("xyz", (URL)null, getDiskstorePath("xyz"));
+        ReplayCache replayCache = new EHCacheReplayCache("xyz", getDiskstorePath("xyz"));
 
         String id = UUID.randomUUID().toString();
         replayCache.add(id);
@@ -101,7 +123,7 @@ public class ReplayCacheTest {
     // 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, getDiskstorePath("xyz"));
+        ReplayCache replayCache = new EHCacheReplayCache("xyz", getDiskstorePath("xyz"));
 
         String id = UUID.randomUUID().toString();
         replayCache.add(id, Instant.now().minusSeconds(100L));
@@ -118,7 +140,7 @@ public class ReplayCacheTest {
     // 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, getDiskstorePath("xyz"));
+        ReplayCache replayCache = new EHCacheReplayCache("xyz", getDiskstorePath("xyz"));
 
         String id = UUID.randomUUID().toString();
         replayCache.add(id, Instant.now().plus(14, ChronoUnit.HOURS));
@@ -132,6 +154,36 @@ public class ReplayCacheTest {
         replayCache.close();
     }
 
+    @Test
+    public void testNullKey() throws Exception {
+        Assertions.assertThrows(NullPointerException.class, () ->
+                new EHCacheReplayCache(null, getDiskstorePath("xyz")));
+    }
+
+    @Test
+    public void testPersistentAndDiskstoreNull() throws Exception {
+        Assertions.assertThrows(NullPointerException.class, () ->
+                new EHCacheReplayCache("abc", null, 10, 10000, true));
+    }
+
+    @Test
+    public void testZeroDiskSize() throws Exception {
+        Assertions.assertThrows(IllegalArgumentException.class, () ->
+                new EHCacheReplayCache("abc", getDiskstorePath("abc"), 0, 10000, false));
+    }
+
+    @Test
+    public void testTooLargeDiskSize() throws Exception {
+        Assertions.assertThrows(IllegalArgumentException.class, () ->
+                new EHCacheReplayCache("abc", getDiskstorePath("abc"), 10001, 10000, false));
+    }
+
+    @Test
+    public void testTooSmallHeapEntries() throws Exception {
+        Assertions.assertThrows(IllegalArgumentException.class, () ->
+                new EHCacheReplayCache("abc", getDiskstorePath("abc"), 10, 10, false));
+    }
+
     private void testReplayCacheInstance(ReplayCache replayCache) throws InterruptedException, IOException {
 
         // Test default TTL caches OK
diff --git a/ws-security-dom/src/test/java/org/apache/wss4j/dom/common/SecurityTestUtil.java b/ws-security-dom/src/test/java/org/apache/wss4j/dom/common/SecurityTestUtil.java
index d6a8d3a..8eb1b35 100644
--- a/ws-security-dom/src/test/java/org/apache/wss4j/dom/common/SecurityTestUtil.java
+++ b/ws-security-dom/src/test/java/org/apache/wss4j/dom/common/SecurityTestUtil.java
@@ -24,8 +24,6 @@ import java.util.Random;
 import org.apache.wss4j.common.cache.EHCacheReplayCache;
 import org.apache.wss4j.common.cache.ReplayCache;
 import org.apache.wss4j.common.ext.WSSecurityException;
-import org.apache.wss4j.dom.util.WSSecurityUtil;
-import org.apache.xml.security.utils.XMLUtils;
 
 /**
  * A utility class for security tests
@@ -37,23 +35,22 @@ public final class SecurityTestUtil {
     }
 
     public static void cleanup() {
-        String tmpDir = System.getProperty("java.io.tmpdir");
-        if (tmpDir != null) {
-            File[] tmpFiles = new File(tmpDir).listFiles();
-            if (tmpFiles != null) {
-                for (File tmpFile : tmpFiles) {
-                    if (tmpFile.exists() && tmpFile.getName().endsWith(".data")) {
-                        tmpFile.delete();
-                    }
-                }
-            }
-        }
+//        String tmpDir = System.getProperty("java.io.tmpdir");
+//        if (tmpDir != null) {
+//            File[] tmpFiles = new File(tmpDir).listFiles();
+//            if (tmpFiles != null) {
+//                for (File tmpFile : tmpFiles) {
+//                    if (tmpFile.exists() && tmpFile.getName().endsWith(".data")) {
+//                        tmpFile.delete();
+//                    }
+//                }
+//            }
+//        }
     }
 
     public static ReplayCache createCache(String key) throws WSSecurityException {
-        String cacheKey = key + XMLUtils.encodeToString(WSSecurityUtil.generateNonce(10));
         String diskKey = key + "-" + Math.abs(new Random().nextInt());
         File diskstore = new File(System.getProperty("java.io.tmpdir"), diskKey);
-        return new EHCacheReplayCache(cacheKey, null, diskstore.toPath());
+        return new EHCacheReplayCache(key, diskstore.toPath());
     }
 }
diff --git a/ws-security-stax/src/test/java/org/apache/wss4j/stax/test/ReplayTest.java b/ws-security-stax/src/test/java/org/apache/wss4j/stax/test/ReplayTest.java
index 00b0fed..672842c 100644
--- a/ws-security-stax/src/test/java/org/apache/wss4j/stax/test/ReplayTest.java
+++ b/ws-security-stax/src/test/java/org/apache/wss4j/stax/test/ReplayTest.java
@@ -45,7 +45,6 @@ import org.apache.wss4j.stax.test.saml.SAML2CallbackHandler;
 import org.apache.wss4j.stax.test.utils.StAX2DOM;
 import org.apache.wss4j.stax.validate.SamlTokenValidatorImpl;
 import org.apache.xml.security.exceptions.XMLSecurityException;
-import org.apache.xml.security.utils.XMLUtils;
 import org.junit.jupiter.api.Test;
 import org.w3c.dom.Document;
 import org.w3c.dom.NodeList;
@@ -58,15 +57,11 @@ import static org.junit.jupiter.api.Assertions.fail;
 public class ReplayTest extends AbstractTestBase {
 
     private ReplayCache createCache(String key) throws WSSecurityException {
-        byte[] nonceValue;
         try {
-            nonceValue = WSSConstants.generateBytes(10);
-            String cacheKey = key + XMLUtils.encodeToString(nonceValue);
-
             String diskKey = key + "-" + Math.abs(new Random().nextInt());
             File diskstore = new File(System.getProperty("java.io.tmpdir"), diskKey);
 
-            return new EHCacheReplayCache(cacheKey, null, diskstore.toPath());
+            return new EHCacheReplayCache(key, diskstore.toPath());
         } catch (XMLSecurityException e) {
             throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, e);
         }
diff --git a/ws-security-stax/src/test/java/org/apache/wss4j/stax/test/UsernameTokenTest.java b/ws-security-stax/src/test/java/org/apache/wss4j/stax/test/UsernameTokenTest.java
index 048f368..98b6611 100644
--- a/ws-security-stax/src/test/java/org/apache/wss4j/stax/test/UsernameTokenTest.java
+++ b/ws-security-stax/src/test/java/org/apache/wss4j/stax/test/UsernameTokenTest.java
@@ -869,13 +869,10 @@ public class UsernameTokenTest extends AbstractTestBase {
     }
 
     private ReplayCache createCache(String key) throws WSSecurityException {
-        byte[] nonceValue;
         try {
-            nonceValue = WSSConstants.generateBytes(10);
-            String cacheKey = key + XMLUtils.encodeToString(nonceValue);
             String diskKey = key + "-" + Math.abs(new Random().nextInt());
             File diskstore = new File(System.getProperty("java.io.tmpdir"), diskKey);
-            return new EHCacheReplayCache(cacheKey, null, diskstore.toPath());
+            return new EHCacheReplayCache(key, diskstore.toPath());
         } catch (XMLSecurityException e) {
             throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, e);
         }