lgoldstein commented on PR #446: URL: https://github.com/apache/mina-sshd/pull/446#issuecomment-1872153855
> I cannot stop you from merging this, but I fundamentally disagree with this change. > Implementing strict KEX is about a 20, maybe 30-line change in AbstractSession Just to make clear - I am not offended in any way - quite the opposite. Like I said, I have tremendous respect for your work despite our differences in style and since you feel so strongly about it I will respect that - especially since you want to work on your own PR counter-proposal. I think this is a very **positive** outcome since we are both committed (no pun intended :-))) to the quality of our code. I do want to stress 2 things: 1. the importance of **thorough** unit tests - regardless of your opinion on the way I propose to fix the issue, my own experience is that the unit tests helped find some problems in the code I wrote. 2. I feel I must insist on having a "kill switch" for this feature (i.e., a `CoreModuleProperties` entry). Like I said, it is a major change in the protocol that not all clients/servers support (yet). There may be a bug in our implementation, or more importantly in theirs. We want to give our users the capability to disable it globally or for a specific session (e.g., one that attempts to communicate with an improperly/faulty implemented peer). As I have stated, if you prefer making it ENABLED by default I will not object, but IMO a kill switch we must have. -- 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
