Some work on making the StorageResolvers thread-safe
git-svn-id: https://svn.apache.org/repos/asf/santuario/xml-security-java/trunk@1872762 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/src/main/java/org/apache/xml/security/keys/keyresolver/KeyResolverSpi.java b/src/main/java/org/apache/xml/security/keys/keyresolver/KeyResolverSpi.java
index e07dc3a..290473a 100644
--- a/src/main/java/org/apache/xml/security/keys/keyresolver/KeyResolverSpi.java
+++ b/src/main/java/org/apache/xml/security/keys/keyresolver/KeyResolverSpi.java
@@ -43,6 +43,8 @@
* <KeyResolver URI="http://www.w3.org/2000/09/xmldsig#KeyValue"
* JAVACLASS="MyPackage.MyKeyValueImpl"//gt;
* </PRE>
+ *
+ * Extensions of this class must be thread-safe.
*/
public abstract class KeyResolverSpi {
diff --git a/src/main/java/org/apache/xml/security/keys/storage/StorageResolver.java b/src/main/java/org/apache/xml/security/keys/storage/StorageResolver.java
index 3232444..aca08a2 100644
--- a/src/main/java/org/apache/xml/security/keys/storage/StorageResolver.java
+++ b/src/main/java/org/apache/xml/security/keys/storage/StorageResolver.java
@@ -38,7 +38,7 @@
org.slf4j.LoggerFactory.getLogger(StorageResolver.class);
/** Field storageResolvers */
- private List<StorageResolverSpi> storageResolvers;
+ private final List<StorageResolverSpi> storageResolvers = new ArrayList<>();
/**
* Constructor StorageResolver
@@ -61,9 +61,6 @@
* @param resolver
*/
public void add(StorageResolverSpi resolver) {
- if (storageResolvers == null) {
- storageResolvers = new ArrayList<>();
- }
this.storageResolvers.add(resolver);
}
@@ -122,10 +119,10 @@
static class StorageResolverIterator implements Iterator<Certificate> {
/** Field resolvers */
- Iterator<StorageResolverSpi> resolvers = null;
+ private final Iterator<StorageResolverSpi> resolvers;
/** Field currentResolver */
- Iterator<Certificate> currentResolver = null;
+ private Iterator<Certificate> currentResolver;
/**
* Constructor StorageResolverIterator
diff --git a/src/main/java/org/apache/xml/security/keys/storage/implementations/CertsInFilesystemDirectoryResolver.java b/src/main/java/org/apache/xml/security/keys/storage/implementations/CertsInFilesystemDirectoryResolver.java
index fe7523d..f7c0746 100644
--- a/src/main/java/org/apache/xml/security/keys/storage/implementations/CertsInFilesystemDirectoryResolver.java
+++ b/src/main/java/org/apache/xml/security/keys/storage/implementations/CertsInFilesystemDirectoryResolver.java
@@ -31,8 +31,10 @@
import java.security.cert.CertificateNotYetValidException;
import java.security.cert.X509Certificate;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.Iterator;
import java.util.List;
+import java.util.NoSuchElementException;
import org.apache.xml.security.keys.storage.StorageResolverException;
import org.apache.xml.security.keys.storage.StorageResolverSpi;
@@ -49,11 +51,8 @@
CertsInFilesystemDirectoryResolver.class
);
- /** Field merlinsCertificatesDir */
- private String merlinsCertificatesDir;
-
/** Field certs */
- private List<X509Certificate> certs = new ArrayList<>();
+ private final List<X509Certificate> certs;
/**
* @param directoryName
@@ -61,19 +60,8 @@
*/
public CertsInFilesystemDirectoryResolver(String directoryName)
throws StorageResolverException {
- this.merlinsCertificatesDir = directoryName;
- this.readCertsFromHarddrive();
- }
-
- /**
- * Method readCertsFromHarddrive
- *
- * @throws StorageResolverException
- */
- private void readCertsFromHarddrive() throws StorageResolverException {
-
- File certDir = new File(this.merlinsCertificatesDir);
+ File certDir = new File(directoryName);
List<String> al = new ArrayList<>();
String[] names = certDir.list();
@@ -88,13 +76,13 @@
}
CertificateFactory cf = null;
-
try {
cf = CertificateFactory.getInstance("X.509");
} catch (CertificateException ex) {
throw new StorageResolverException(ex);
}
+ List<X509Certificate> tmpCerts = new ArrayList<>();
for (int i = 0; i < al.size(); i++) {
String filename = certDir.getAbsolutePath() + File.separator + al.get(i);
boolean added = false;
@@ -106,7 +94,7 @@
//add to ArrayList
cert.checkValidity();
- this.certs.add(cert);
+ tmpCerts.add(cert);
dn = cert.getSubjectX500Principal().getName();
added = true;
@@ -136,6 +124,8 @@
LOG.debug("Added certificate: {}", dn);
}
}
+
+ certs = Collections.unmodifiableList(tmpCerts);
}
/** {@inheritDoc} */
@@ -149,7 +139,7 @@
private static class FilesystemIterator implements Iterator<Certificate> {
/** Field certs */
- private List<X509Certificate> certs;
+ private final List<X509Certificate> certs;
/** Field i */
private int i;
@@ -171,7 +161,11 @@
/** {@inheritDoc} */
public Certificate next() {
- return this.certs.get(this.i++);
+ if (hasNext()) {
+ return this.certs.get(this.i++);
+ }
+
+ throw new NoSuchElementException();
}
/**
diff --git a/src/main/java/org/apache/xml/security/keys/storage/implementations/KeyStoreResolver.java b/src/main/java/org/apache/xml/security/keys/storage/implementations/KeyStoreResolver.java
index 294720f..2b6b784 100644
--- a/src/main/java/org/apache/xml/security/keys/storage/implementations/KeyStoreResolver.java
+++ b/src/main/java/org/apache/xml/security/keys/storage/implementations/KeyStoreResolver.java
@@ -21,8 +21,11 @@
import java.security.KeyStore;
import java.security.KeyStoreException;
import java.security.cert.Certificate;
+import java.util.ArrayList;
+import java.util.Collections;
import java.util.Enumeration;
import java.util.Iterator;
+import java.util.List;
import java.util.NoSuchElementException;
import org.apache.xml.security.keys.storage.StorageResolverException;
@@ -34,8 +37,11 @@
*/
public class KeyStoreResolver extends StorageResolverSpi {
+ private static final org.slf4j.Logger LOG =
+ org.slf4j.LoggerFactory.getLogger(KeyStoreResolver.class);
+
/** Field keyStore */
- private KeyStore keyStore;
+ private final KeyStore keyStore;
/**
* Constructor KeyStoreResolver
@@ -63,14 +69,9 @@
*/
static class KeyStoreIterator implements Iterator<Certificate> {
- /** Field keyStore */
- KeyStore keyStore = null;
+ private final List<Certificate> certs;
- /** Field aliases */
- Enumeration<String> aliases = null;
-
- /** Field nextCert */
- Certificate nextCert = null;
+ private int i;
/**
* Constructor KeyStoreIterator
@@ -78,45 +79,37 @@
* @param keyStore
*/
public KeyStoreIterator(KeyStore keyStore) {
+
+ List<Certificate> tmpCerts = new ArrayList<>();
try {
- this.keyStore = keyStore;
- this.aliases = this.keyStore.aliases();
+ Enumeration<String> aliases = keyStore.aliases();
+ while (aliases.hasMoreElements()) {
+ String alias = aliases.nextElement();
+ Certificate cert = keyStore.getCertificate(alias);
+ if (cert != null) {
+ tmpCerts.add(cert);
+ }
+ }
} catch (KeyStoreException ex) {
- // empty Enumeration
- this.aliases = new Enumeration<String>() {
- public boolean hasMoreElements() {
- return false;
- }
- public String nextElement() {
- return null;
- }
- };
+ LOG.debug("Error reading certificates: {}", ex.getMessage());
}
+
+ certs = Collections.unmodifiableList(tmpCerts);
+ this.i = 0;
}
/** {@inheritDoc} */
public boolean hasNext() {
- if (nextCert == null) {
- nextCert = findNextCert();
- }
-
- return nextCert != null;
+ return this.i < this.certs.size();
}
/** {@inheritDoc} */
public Certificate next() {
- if (nextCert == null) {
- // maybe caller did not call hasNext()
- nextCert = findNextCert();
-
- if (nextCert == null) {
- throw new NoSuchElementException();
- }
+ if (hasNext()) {
+ return this.certs.get(this.i++);
}
- Certificate ret = nextCert;
- nextCert = null;
- return ret;
+ throw new NoSuchElementException();
}
/**
@@ -126,24 +119,6 @@
throw new UnsupportedOperationException("Can't remove keys from KeyStore");
}
- // Find the next entry that contains a certificate and return it.
- // In particular, this skips over entries containing symmetric keys.
- private Certificate findNextCert() {
- while (this.aliases.hasMoreElements()) {
- String alias = this.aliases.nextElement();
- try {
- Certificate cert = this.keyStore.getCertificate(alias);
- if (cert != null) {
- return cert;
- }
- } catch (KeyStoreException ex) {
- return null;
- }
- }
-
- return null;
- }
-
}
}
diff --git a/src/main/java/org/apache/xml/security/keys/storage/implementations/SingleCertificateResolver.java b/src/main/java/org/apache/xml/security/keys/storage/implementations/SingleCertificateResolver.java
index 5d77acc..34f6b50 100644
--- a/src/main/java/org/apache/xml/security/keys/storage/implementations/SingleCertificateResolver.java
+++ b/src/main/java/org/apache/xml/security/keys/storage/implementations/SingleCertificateResolver.java
@@ -32,7 +32,7 @@
public class SingleCertificateResolver extends StorageResolverSpi {
/** Field certificate */
- private X509Certificate certificate;
+ private final X509Certificate certificate;
/**
* @param x509cert the single {@link X509Certificate}
@@ -52,10 +52,10 @@
static class InternalIterator implements Iterator<Certificate> {
/** Field alreadyReturned */
- boolean alreadyReturned = false;
+ private boolean alreadyReturned;
/** Field certificate */
- X509Certificate certificate = null;
+ private final X509Certificate certificate;
/**
* Constructor InternalIterator
