You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by "duco-lw (via GitHub)" <gi...@apache.org> on 2024/05/16 13:50:34 UTC
[I] SessionHelper#signalNegotiationEnd should pass reason through to listener [mina-sshd]
duco-lw opened a new issue, #504:
URL: https://github.com/apache/mina-sshd/issues/504
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:
```
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
Re: [I] SessionHelper#signalNegotiationEnd should pass reason through to listener [mina-sshd]
Posted by "tomaswolf (via GitHub)" <gi...@apache.org>.
tomaswolf commented on issue #504:
URL: https://github.com/apache/mina-sshd/issues/504#issuecomment-2115324310
Thanks for pointing this out.
> Would ... break anything?
No, it would not. Feel free to provide a PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
Re: [I] SessionHelper#signalNegotiationEnd should pass reason through to listener [mina-sshd]
Posted by "tomaswolf (via GitHub)" <gi...@apache.org>.
tomaswolf closed issue #504: SessionHelper#signalNegotiationEnd should pass reason through to listener
URL: https://github.com/apache/mina-sshd/issues/504
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org