You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by GitBox <gi...@apache.org> on 2020/02/27 13:04:55 UTC
[GitHub] [mina-sshd] lgoldstein opened a new pull request #114: SSHD-968
interpret SSH_MSG_UNIMPLEMENTED response to a global request as its failure
lgoldstein opened a new pull request #114: SSHD-968 interpret SSH_MSG_UNIMPLEMENTED response to a global request as its failure
URL: https://github.com/apache/mina-sshd/pull/114
The main flow is as follows:
* when `AbstractSession#preProcessEncodeBuffer` override is called as part of sending the global request, it checks if the buffer about to be encoded and written is a `SSH_MSG_GLOBAL_REQUEST` command. If so, it stores the outgoing message sequence number in case `SSH_MSG_UNIMPLEMENTED` is reported for it.
* `AbstractSession#request` resets the tracked global request outgoing sequence number if either a response was received or timeout expired.
* `AbstractSession#doInvokeUnimplementedMessageHandler` override checks if the reported message number matches the currently tracked global request message and converts it to a message failure signal instead (thus releasing the `AbstractSession#request` loop waiting for it).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] gnodet commented on issue #114: SSHD-968 interpret
SSH_MSG_UNIMPLEMENTED response to a global request as its failure
Posted by GitBox <gi...@apache.org>.
gnodet commented on issue #114: SSHD-968 interpret SSH_MSG_UNIMPLEMENTED response to a global request as its failure
URL: https://github.com/apache/mina-sshd/pull/114#issuecomment-592077920
No objections, go for it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein commented on issue #114: SSHD-968 interpret
SSH_MSG_UNIMPLEMENTED response to a global request as its failure
Posted by GitBox <gi...@apache.org>.
lgoldstein commented on issue #114: SSHD-968 interpret SSH_MSG_UNIMPLEMENTED response to a global request as its failure
URL: https://github.com/apache/mina-sshd/pull/114#issuecomment-591989830
>> I thought you wanted a less invasive fix ;-)
I did not mean less invasive as much as less aware of the purpose of the changes. It is less invasive as far as existing methods since it only introduces new ones.
>> I don't really like the artificial split with the introduction of the preProcessEncodeBuffer method as it's only called from AbstractSession. encode, while the preProcessEncodeBuffer is defined in the SessionHelper and overriden in the AbstractSession. This looks rather complex instead of simply adding the few needed lines in the existing encode method (efdb7c3#diff-916a03c44b5846431681a4411891d952R843-R849).
True, but it was the only way that I could come up with - IMO `encode` already does so much (lots of code for one method) so I thought it would make sense to split it rather than add to it. Anyway, like I said it opens up the door for future fixes/extension - e.g., perhaps one of the derived classes from `AbsractSession` needs something else.
>> Anyway, if you really wanted something simple, we could go for gnodet@cdefec7 which is much simpler ;-)
True, but IMO this fix while more complex feels more robust. Any objections to merging it ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] gnodet commented on issue #114: SSHD-968 interpret
SSH_MSG_UNIMPLEMENTED response to a global request as its failure
Posted by GitBox <gi...@apache.org>.
gnodet commented on issue #114: SSHD-968 interpret SSH_MSG_UNIMPLEMENTED response to a global request as its failure
URL: https://github.com/apache/mina-sshd/pull/114#issuecomment-591986308
I thought you wanted a less invasive fix ;-)
I don't really like the artificial split with the introduction of the `preProcessEncodeBuffer` method as it's only called from `AbstractSession. encode`, while the `preProcessEncodeBuffer` is defined in the `SessionHelper` and overriden in the `AbstractSession`. This looks rather complex instead of simply adding the few needed lines in the existing `encode` method (https://github.com/apache/mina-sshd/pull/114/commits/efdb7c3de1e32592c91536cf699fb3a0bfc48462#diff-916a03c44b5846431681a4411891d952R843-R849).
Anyway, if you really wanted something simple, we could go for https://github.com/gnodet/mina-sshd/commit/cdefec7ebd5214694a4c578bc01066c6f3c6cbde which is much simpler ;-)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] lgoldstein closed pull request #114: SSHD-968 interpret
SSH_MSG_UNIMPLEMENTED response to a global request as its failure
Posted by GitBox <gi...@apache.org>.
lgoldstein closed pull request #114: SSHD-968 interpret SSH_MSG_UNIMPLEMENTED response to a global request as its failure
URL: https://github.com/apache/mina-sshd/pull/114
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org