tomaswolf commented on code in PR #507: URL: https://github.com/apache/mina-sshd/pull/507#discussion_r1617835398
########## CHANGES.md: ########## @@ -58,7 +59,34 @@ NTRU Prime sntrup761 and X25519 with SHA-512: sntrup761x25519-sha512](https://ww ## Behavioral changes and enhancements -* [GH-468](https://github.com/apache/mina-sshd/issues/468) SFTP: validate length of data received: must not be more than requested +### [GH-461](https://github.com/apache/mina-sshd/issues/461) Fix heartbeats with `wantReply=true` + +The client-side heartbeat mechanism has been changed. Such heartbeats are configured with an interval +(`CoreModuleProperties.HEARTBEAT_INTERVAL`) > 0. Previously they could also be configured with a timeout +(`CoreModuleProperties.HEARTBEAT_REPLY_WAIT`). If the timeout was <= 0, the client would just send a +heartbeat request, but not expect any answer. However, with a timeout > 0, it would send the request +with a flag telling the server to send back a reply, and then wait for the given duration for the +reply to arrive, and terminate the connection if no reply was forthcoming. + +That could cause trouble if the timeout was fairly long, and the server was slow to respond. If the +timeout was longer than the interval it could also delay subsequent heartbeats. + +Newly, `CoreModuleProperties.HEARTBEAT_REPLY_WAIT` is _deprecated_. There is a new configuration property +`CoreModuleProperties.HEARTBEAT_NO_REPLY_MAX` instead. It defines a limit for how many heartbeats without +reply a session survives. If the value is <= 0, the client still sends heartbeats without expecting +any answer. If > 0, it will request a reply from the server for each heartbeat message, and it will +terminate the connection once there are `CoreModuleProperties.HEARTBEAT_NO_REPLY_MAX` previous heartbeats +for which no reply was received yet. + +This new way to configure heartbeats corresponds to the configuration in OpenSSH via the +`ServerAliveInterval` and `ServerAliveCountMax` options. + +For compatibility with old configurations that do define `CoreModuleProperties.HEARTBEAT_REPLY_WAIT` > 0, +the new code maps this to the new configuration (but only if `CoreModuleProperties.HEARTBEAT_INTERVAL` > 0 +and the new property `CoreModuleProperties.HEARTBEAT_NO_REPLY_MAX` has _not_ been set) by setting +`CoreModuleProperties.HEARTBEAT_NO_REPLY_MAX = (CoreModuleProperties.HEARTBEAT_REPLY_WAIT / CoreModuleProperties.HEARTBEAT_INTERVAL) + 1`. Review Comment: Done. I've taken this as inspiration and have rephrased the description along these lines. -- 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-unsubscr...@mina.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
