Closed duco-lw closed 4 months ago
https://github.com/apache/mina-sshd/blob/71b842f759f9879d7638bed175e5be006d9c0f46/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java#L1149
The reason is not passed through to the listener:
reason
protected void signalNegotiationEnd( SessionListener listener, Map<KexProposalOption, String> c2sOptions, Map<KexProposalOption, String> s2cOptions, Map<KexProposalOption, String> negotiatedGuess, Throwable reason) { if (listener == null) { return; } listener.sessionNegotiationEnd(this, c2sOptions, s2cOptions, negotiatedGuess, null); }
The doc for this says the negotiation is successful if this is null:
/** * Signals the end of the negotiation options handling * * @param session The referenced {@link Session} * @param clientProposal The client proposal options (un-modifiable) * @param serverProposal The server proposal options (un-modifiable) * @param negotiatedOptions The successfully negotiated options so far - even if exception occurred (un-modifiable) * @param reason Negotiation end reason - {@code null} if successful */ default void sessionNegotiationEnd( Session session, Map<KexProposalOption, String> clientProposal, Map<KexProposalOption, String> serverProposal, Map<KexProposalOption, String> negotiatedOptions, Throwable reason) { // ignored }
https://github.com/apache/mina-sshd/blob/71b842f759f9879d7638bed175e5be006d9c0f46/sshd-core/src/main/java/org/apache/sshd/common/session/SessionListener.java#L151
This looks like this will always be "successful", even in the error case?
} catch (IOException | RuntimeException | Error e) { signalNegotiationEnd(c2sOptions, s2cOptions, negotiatedGuess, e); throw e; }
https://github.com/apache/mina-sshd/blob/71b842f759f9879d7638bed175e5be006d9c0f46/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java#L2259
Would changing the line from
listener.sessionNegotiationEnd(this, c2sOptions, s2cOptions, negotiatedGuess, null);
to
listener.sessionNegotiationEnd(this, c2sOptions, s2cOptions, negotiatedGuess, reason);
break anything?
Thanks for pointing this out.
Would ... break anything?
No, it would not. Feel free to provide a PR.
https://github.com/apache/mina-sshd/blob/71b842f759f9879d7638bed175e5be006d9c0f46/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/SessionHelper.java#L1149
The
reason
is not passed through to the listener:The doc for this says the negotiation is successful if this is null:
https://github.com/apache/mina-sshd/blob/71b842f759f9879d7638bed175e5be006d9c0f46/sshd-core/src/main/java/org/apache/sshd/common/session/SessionListener.java#L151
This looks like this will always be "successful", even in the error case?
https://github.com/apache/mina-sshd/blob/71b842f759f9879d7638bed175e5be006d9c0f46/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java#L2259
Would changing the line from
to
break anything?