[
https://issues.apache.org/jira/browse/SSHD-860?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16680201#comment-16680201
]
Thomas Wolf edited comment on SSHD-860 at 11/8/18 7:10 PM:
-----------------------------------------------------------
{quote}I am not sure I understand how this happens
{quote}
It means that somewhere along the line, something still uses a stream operation
to obtain an iterator. An iterator on a stream will be such a spliterator, and
those buffer.
{quote}Can you indicate the code that uses these "newfangled spliterators" ?
{quote}
As far as I see, the culprit is this call chain:
* UserAuthPublicKeyIterator.initializeSessionIdentities()
* ClientSession.providerOf(session)
*
KeyIdentityProvider.resolveKeyIdentityProvider(session.getRegisteredIdentities(),
session.getKeyPairProvider())
* KeyIdentityProvider.multiProvider(identities, keys)
* KeyIdentityProvider.multiProvider(GenericUtils.asList(providers))
* KeyIdentityProvider.wrapKeyPairs(iterableOf(providers))
** iterableOf(providers) calls
*** GenericUtils.multiIterableSuppliers(GenericUtils.wrapIterable(providers, p
-> p::loadKeys))
*** GenericUtils.wrapIterable(iter, mapper) returns {{() ->
GenericUtils.wrapIterator(iter, mapper)}}
**** GenericUtils.wrapIterator(iter, mapper) does {{return
stream(iter).map(mapper).iterator()}}
*** GenericUtils.multiIterableSuppliers(...) does {{return () ->
stream(providers).flatMap(s ->
stream(s.get())).map(Function.identity()).iterator()}}
I'm not sure which of the last two is the real problem here; tracing this
through debugging is highly confusing once it enters hasNext(). Looks to me we
have in the end a spliterator over a stream of spliterators (a spliterator of
spliterators). Which also explains why in my JGit code it doesn't occur; I made
a point of not using any stream operations in these key iterators.
was (Author: wolft):
{quote}I am not sure I understand how this happens
{quote}
It means that somewhere along the line, something still uses a stream operation
to obtain an iterator. An iterator on a stream will be such a spliterator, and
those buffer.
{quote}Can you indicate the code that uses these "newfangled spliterators" ?
{quote}
As far as I see, the culprit is this call chain:
* UserAuthPublicKeyIterator.initializeSessionIdentities()
* ClientSession.providerOf(session)
*
KeyIdentityProvider.resolveKeyIdentityProvider(session.getRegisteredIdentities(),
session.getKeyPairProvider())
* KeyIdentityProvider.multiProvider(identities, keys)
* KeyIdentityProvider.multiProvider(GenericUtils.asList(providers))
* KeyIdentityProvider.wrapKeyPairs(iterableOf(providers))
** iterableOf(providers) calls
*** GenericUtils.multiIterableSuppliers(GenericUtils.wrapIterable(providers, p
-> p::loadKeys)), which returns {{() -> GenericUtils.wrapIterator(iter,
mapper)}}
**** GenericUtils.wrapIterator(iter, mapper) does {{return
stream(iter).map(mapper).iterator()}}
*** GenericUtils.multiIterableSuppliers(...) does {{return () ->
stream(providers).flatMap(s ->
stream(s.get())).map(Function.identity()).iterator()}}
I'm not sure which of the last two is the real problem here; tracing this
through debugging is highly confusing once it enters hasNext(). Looks to me we
have in the end a spliterator over a stream of spliterators (a spliterator of
spliterators). Which also explains why in my JGit code it doesn't occur; I made
a point of not using any stream operations in these key iterators.
> org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator pre-loads client
> identities (private key files)
> ------------------------------------------------------------------------------------------------------------
>
> Key: SSHD-860
> URL: https://issues.apache.org/jira/browse/SSHD-860
> Project: MINA SSHD
> Issue Type: Bug
> Affects Versions: 2.0.0, 2.1.0, 2.1.1
> Reporter: Thomas Wolf
> Assignee: Goldstein Lyor
> Priority: Major
> Fix For: 2.1.1
>
>
> {{org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator}} loads and
> caches in memory private keys prematurely. Keys are loaded even if they are
> not used at all in the end. In other words, incremental loading of keys
> doesn't work.
> This is bad for two reasons:
> # Private keys should be kept in memory only if and when needed. Loading
> completely unused private keys must be avoided.
> # With encrypted private keys, the user may end up being asked for
> passphrases for keys that are not used at all in the end, which is highly
> disruptive.
> {{org.apache.sshd.client.auth.pubkey.UserAuthPublicKeyIterator}} does in its
> constructor:
> {code:java}
> Iterator<? extends PublicKeyIdentity> current;
> Collection<Stream<? extends PublicKeyIdentity>> identities = new
> LinkedList<>();
> ...
> identities.add(Stream.of(ClientSession.providerOf(session))
> .map(KeyIdentityProvider::loadKeys)
> .flatMap(GenericUtils::stream)
> .map(kp -> new KeyPairIdentity(signatureFactories, session, kp)));
> current = identities.stream().flatMap(r -> r).iterator();
> {code}
> The final {{current}} iterator uses some internal buffer (size unknown;
> didn't try to determine it) and will pre-fill this buffer completely. So with
> buffer size _N_ it'll pre-load the first _N_ keys from the combined identity
> stream. If the first key authenticates successfully, loading all the others
> must not be done.
> See my [test
> case|https://github.com/tomaswolf/mina-sshd/blob/SSHD-860/sshd-core/src/test/java/org/apache/sshd/client/ClientKeyLoadTest.java]
> showing this faulty behavior. It does exactly the same as the
> {{UserAuthPublicKeyIterator}} constructor, using two iterators with two
> identity files each, and then tests the resulting iterator. The first
> {{hasNext()/next()}} call on the {{current}} iterator _loads all four keys!_
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
