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 <[email protected]> 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); }
