Repository: mina-sshd Updated Branches: refs/heads/master cbb92d2a8 -> be51bdc8a
[SSHD-732] ClientIdentitiesWatcher should load keys one at a time Make sure iterators are lazy, i.e. they don't call hasNext() or next() unless actually required. Use the Stream api in some cases to simplify the code. Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/612218b7 Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/612218b7 Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/612218b7 Branch: refs/heads/master Commit: 612218b7739435e2596fa47d010f3628d91569e4 Parents: cbb92d2 Author: Guillaume Nodet <gno...@apache.org> Authored: Fri Mar 10 11:58:37 2017 +0100 Committer: Guillaume Nodet <gno...@apache.org> Committed: Fri Mar 10 11:58:37 2017 +0100 ---------------------------------------------------------------------- .../auth/hostbased/HostKeyIdentityProvider.java | 5 +- .../auth/pubkey/UserAuthPublicKeyIterator.java | 36 ++++-------- .../keys/BuiltinClientIdentitiesWatcher.java | 38 ++++--------- .../config/keys/ClientIdentitiesWatcher.java | 59 +++++++++++++------- .../common/keyprovider/KeyIdentityProvider.java | 20 ++++++- .../apache/sshd/common/util/GenericUtils.java | 45 +++------------ 6 files changed, 90 insertions(+), 113 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/612218b7/sshd-core/src/main/java/org/apache/sshd/client/auth/hostbased/HostKeyIdentityProvider.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/client/auth/hostbased/HostKeyIdentityProvider.java b/sshd-core/src/main/java/org/apache/sshd/client/auth/hostbased/HostKeyIdentityProvider.java index 62206a1..b141eb5 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/auth/hostbased/HostKeyIdentityProvider.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/auth/hostbased/HostKeyIdentityProvider.java @@ -47,9 +47,6 @@ public interface HostKeyIdentityProvider { } static HostKeyIdentityProvider wrap(Iterable<? extends KeyPair> pairs) { - return () -> () -> { - Iterator<? extends KeyPair> iter = GenericUtils.iteratorOf(pairs); - return GenericUtils.wrapIterator(iter, kp -> new Pair<>(kp, Collections.<X509Certificate>emptyList())); - }; + return () -> GenericUtils.wrapIterable(pairs, kp -> new Pair<>(kp, Collections.<X509Certificate>emptyList())); } } http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/612218b7/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKeyIterator.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKeyIterator.java b/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKeyIterator.java index 0ecb740..e1c7c86 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKeyIterator.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/auth/pubkey/UserAuthPublicKeyIterator.java @@ -27,6 +27,7 @@ import java.util.LinkedList; import java.util.NoSuchElementException; import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Stream; import org.apache.sshd.agent.SshAgent; import org.apache.sshd.agent.SshAgentFactory; @@ -42,22 +43,21 @@ import org.apache.sshd.common.util.GenericUtils; public class UserAuthPublicKeyIterator extends AbstractKeyPairIterator<PublicKeyIdentity> implements Channel { private final AtomicBoolean open = new AtomicBoolean(true); - private final Iterator<Iterator<? extends PublicKeyIdentity>> iterators; private Iterator<? extends PublicKeyIdentity> current; private SshAgent agent; public UserAuthPublicKeyIterator(ClientSession session, SignatureFactoriesManager signatureFactories) throws Exception { super(session); - Collection<Iterator<? extends PublicKeyIdentity>> identities = new LinkedList<>(); - identities.add(new SessionKeyPairIterator(session, signatureFactories, KeyIdentityProvider.iteratorOf(session))); + Collection<Stream<? extends PublicKeyIdentity>> identities = new LinkedList<>(); FactoryManager manager = Objects.requireNonNull(session.getFactoryManager(), "No session factory manager"); SshAgentFactory factory = manager.getAgentFactory(); if (factory != null) { try { agent = Objects.requireNonNull(factory.createClient(manager), "No agent created"); - identities.add(new SshAgentPublicKeyIterator(session, agent)); + identities.add(agent.getIdentities().stream() + .map(kp -> new KeyAgentIdentity(agent, kp.getFirst(), kp.getSecond()))); } catch (Exception e) { try { closeAgent(); @@ -69,8 +69,12 @@ public class UserAuthPublicKeyIterator extends AbstractKeyPairIterator<PublicKey } } - iterators = GenericUtils.iteratorOf(identities); - current = nextIterator(iterators); + identities.add(Stream.of(KeyIdentityProvider.providerOf(session)) + .map(KeyIdentityProvider::loadKeys) + .flatMap(GenericUtils::stream) + .map(kp -> new KeyPairIdentity(session, signatureFactories, kp))); + + current = identities.stream().flatMap(r -> r).iterator(); } @Override @@ -79,7 +83,7 @@ public class UserAuthPublicKeyIterator extends AbstractKeyPairIterator<PublicKey return false; } - return current != null; + return current.hasNext(); } @Override @@ -87,12 +91,7 @@ public class UserAuthPublicKeyIterator extends AbstractKeyPairIterator<PublicKey if (!isOpen()) { throw new NoSuchElementException("Iterator is closed"); } - - PublicKeyIdentity pki = current.next(); - if (!current.hasNext()) { - current = nextIterator(iterators); - } - return pki; + return current.next(); } @Override @@ -117,15 +116,4 @@ public class UserAuthPublicKeyIterator extends AbstractKeyPairIterator<PublicKey } } - protected Iterator<? extends PublicKeyIdentity> nextIterator( - Iterator<? extends Iterator<? extends PublicKeyIdentity>> available) { - while ((available != null) && available.hasNext()) { - Iterator<? extends PublicKeyIdentity> iter = available.next(); - if (iter.hasNext()) { - return iter; - } - } - - return null; - } } http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/612218b7/sshd-core/src/main/java/org/apache/sshd/client/config/keys/BuiltinClientIdentitiesWatcher.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/client/config/keys/BuiltinClientIdentitiesWatcher.java b/sshd-core/src/main/java/org/apache/sshd/client/config/keys/BuiltinClientIdentitiesWatcher.java index e2cc319..ab9929b 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/config/keys/BuiltinClientIdentitiesWatcher.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/config/keys/BuiltinClientIdentitiesWatcher.java @@ -24,7 +24,6 @@ import java.security.KeyPair; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.LinkedList; import java.util.List; import java.util.Objects; import java.util.function.Supplier; @@ -70,33 +69,20 @@ public class BuiltinClientIdentitiesWatcher extends ClientIdentitiesWatcher { } @Override - public List<KeyPair> loadKeys() { - List<KeyPair> keys = super.loadKeys(); - if (GenericUtils.isEmpty(keys) || (!isSupportedOnly())) { - return keys; - } - - Collection<KeyPair> toRemove = null; - for (KeyPair kp : keys) { - BuiltinIdentities id = BuiltinIdentities.fromKeyPair(kp); - if ((id != null) && id.isSupported()) { - continue; - } - - if (log.isDebugEnabled()) { - log.debug("loadKeys - remove unsupported identity={}, key-type={}, key={}", - id, KeyUtils.getKeyType(kp), KeyUtils.getFingerPrint(kp.getPublic())); - } + public Iterable<KeyPair> loadKeys() { + return isSupportedOnly() ? loadKeys(this::isSupported) : super.loadKeys(); + } - if (toRemove == null) { - toRemove = new LinkedList<>(); - } - toRemove.add(kp); + private boolean isSupported(KeyPair kp) { + BuiltinIdentities id = BuiltinIdentities.fromKeyPair(kp); + if ((id != null) && id.isSupported()) { + return true; } - - GenericUtils.forEach(toRemove, keys::remove); - - return keys; + if (log.isDebugEnabled()) { + log.debug("loadKeys - remove unsupported identity={}, key-type={}, key={}", + id, KeyUtils.getKeyType(kp), KeyUtils.getFingerPrint(kp.getPublic())); + } + return false; } public static List<Path> getDefaultBuiltinIdentitiesPaths(Path keysFolder) { http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/612218b7/sshd-core/src/main/java/org/apache/sshd/client/config/keys/ClientIdentitiesWatcher.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/client/config/keys/ClientIdentitiesWatcher.java b/sshd-core/src/main/java/org/apache/sshd/client/config/keys/ClientIdentitiesWatcher.java index 3a78958..6c2bb0b 100644 --- a/sshd-core/src/main/java/org/apache/sshd/client/config/keys/ClientIdentitiesWatcher.java +++ b/sshd-core/src/main/java/org/apache/sshd/client/config/keys/ClientIdentitiesWatcher.java @@ -21,12 +21,15 @@ package org.apache.sshd.client.config.keys; import java.nio.file.Path; import java.security.KeyPair; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.Optional; +import java.util.function.Function; +import java.util.function.Predicate; import java.util.function.Supplier; +import java.util.stream.Stream; import org.apache.sshd.common.config.keys.FilePasswordProvider; import org.apache.sshd.common.keyprovider.AbstractKeyPairProvider; @@ -69,32 +72,46 @@ public class ClientIdentitiesWatcher extends AbstractKeyPairProvider implements } @Override - public List<KeyPair> loadKeys() { - if (GenericUtils.isEmpty(providers)) { - return Collections.emptyList(); - } + public Iterable<KeyPair> loadKeys() { + return loadKeys(null); + } - List<KeyPair> keys = new ArrayList<>(providers.size()); // optimistic initialization - for (ClientIdentityProvider p : providers) { - try { - KeyPair kp = p.getClientIdentity(); - if (kp == null) { - if (log.isDebugEnabled()) { - log.debug("loadKeys({}) no key loaded", p); - } - continue; - } + protected Iterable<KeyPair> loadKeys(Predicate<KeyPair> filter) { + return () -> { + Stream<KeyPair> stream = safeMap(GenericUtils.stream(providers), this::doGetKeyPair); + if (filter != null) { + stream = stream.filter(filter); + } + return stream.iterator(); + }; + } + + /** + * Performs a mapping operation on the stream, discarding any null values + * returned by the mapper. + */ + private <U, V> Stream<V> safeMap(Stream<U> stream, Function<U, V> mapper) { + return stream.map(u -> Optional.ofNullable(mapper.apply(u))) + .filter(Optional::isPresent) + .map(Optional::get); + } - keys.add(kp); - } catch (Throwable e) { - log.warn("loadKeys({}) failed ({}) to load key: {}", p, e.getClass().getSimpleName(), e.getMessage()); + private KeyPair doGetKeyPair(ClientIdentityProvider p) { + try { + KeyPair kp = p.getClientIdentity(); + if (kp == null) { if (log.isDebugEnabled()) { - log.debug("loadKeys(" + p + ") key load failure details", e); + log.debug("loadKeys({}) no key loaded", p); } } + return kp; + } catch (Throwable e) { + log.warn("loadKeys({}) failed ({}) to load key: {}", p, e.getClass().getSimpleName(), e.getMessage()); + if (log.isDebugEnabled()) { + log.debug("loadKeys(" + p + ") key load failure details", e); + } + return null; } - - return keys; } public static List<ClientIdentityProvider> buildProviders( http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/612218b7/sshd-core/src/main/java/org/apache/sshd/common/keyprovider/KeyIdentityProvider.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/common/keyprovider/KeyIdentityProvider.java b/sshd-core/src/main/java/org/apache/sshd/common/keyprovider/KeyIdentityProvider.java index c8cddfa..626c706 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/keyprovider/KeyIdentityProvider.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/keyprovider/KeyIdentityProvider.java @@ -65,6 +65,24 @@ public interface KeyIdentityProvider { Iterable<KeyPair> loadKeys(); /** + * Creates a "unified" {@link KeyIdentityProvider} of key pairs out of the registered + * {@link KeyPair} identities and the extra available ones as a single iterator + * of key pairs + * + * + * @param session The {@link ClientSession} - ignored if {@code null} (i.e., empty + * iterator returned) + * @return The wrapping KeyIdentityProvider + * @see ClientSession#getRegisteredIdentities() + * @see ClientSession#getKeyPairProvider() + */ + static KeyIdentityProvider providerOf(ClientSession session) { + return session == null + ? EMPTY_KEYS_PROVIDER + : resolveKeyIdentityProvider(session.getRegisteredIdentities(), session.getKeyPairProvider()); + } + + /** * Creates a "unified" {@link Iterator} of key pairs out of the registered * {@link KeyPair} identities and the extra available ones as a single iterator * of key pairs @@ -76,7 +94,7 @@ public interface KeyIdentityProvider { * @see ClientSession#getKeyPairProvider() */ static Iterator<KeyPair> iteratorOf(ClientSession session) { - return (session == null) ? Collections.<KeyPair>emptyIterator() : iteratorOf(session.getRegisteredIdentities(), session.getKeyPairProvider()); + return iteratorOf(providerOf(session)); } /** http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/612218b7/sshd-core/src/main/java/org/apache/sshd/common/util/GenericUtils.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/common/util/GenericUtils.java b/sshd-core/src/main/java/org/apache/sshd/common/util/GenericUtils.java index 82d78d1..e2f8733 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/util/GenericUtils.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/util/GenericUtils.java @@ -33,7 +33,6 @@ import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; import java.util.SortedMap; @@ -756,7 +755,13 @@ public final class GenericUtils { } public static <U, V> Iterable<V> wrapIterable(Iterable<? extends U> iter, Function<U, V> mapper) { - return () -> wrapIterator(iteratorOf(iter), mapper); + return () -> wrapIterator(iter, mapper); + } + + public static <U, V> Iterator<V> wrapIterator(Iterable<? extends U> iter, Function<U, V> mapper) { + return stream(iter) + .map(mapper::apply) + .iterator(); } public static <U, V> Iterator<V> wrapIterator(Iterator<? extends U> iter, Function<U, V> mapper) { @@ -784,41 +789,7 @@ public final class GenericUtils { * @return The wrapping instance */ public static <T> Iterable<T> multiIterableSuppliers(Iterable<? extends Supplier<? extends Iterable<? extends T>>> providers) { - return (providers == null) ? Collections.emptyList() : () -> new Iterator<T>() { - private final Iterator<? extends Supplier<? extends Iterable<? extends T>>> iter = iteratorOf(providers); - private Iterator<? extends T> current = nextIterator(); - - @Override - public boolean hasNext() { - return current != null; - } - - @Override - public T next() { - if (current == null) { - throw new NoSuchElementException("No more elements"); - } - - T value = current.next(); - if (!current.hasNext()) { - current = nextIterator(); - } - - return value; - } - - private Iterator<? extends T> nextIterator() { - while (iter.hasNext()) { - Supplier<? extends Iterable<? extends T>> supplier = iter.next(); - Iterator<? extends T> values = iteratorOf((supplier == null) ? null : supplier.get()); - if (values.hasNext()) { - return values; - } - } - - return null; - } - }; + return () -> stream(providers).flatMap(s -> stream(s.get())).map(u -> (T) u).iterator(); } public static <K, V> MapBuilder<K, V> mapBuilder() {
