[GitHub] [jmeter] vlsi edited a comment on pull request #669: 65461: support multiple keys in the client keystore (select the matching one by keyType)

[GitBox <gi...@apache.org>]

vlsi edited a comment on pull request #669:
URL: https://github.com/apache/jmeter/pull/669#issuecomment-1001949871


   I did try to review this, and my conclusion is as follows:
   1) I would like to decline the patch, as adding more and more stuff to JMeter's SSL makes the stuff harder and harder to follow
   2) There are no tests to cover the change. I agree SSL-related tests was not there, however, the code is already hard to follow
   3) I'm inclined that `JmeterKeyStore` should be **removed**, and we should use the default `KeyStore` implementation
   4) `SSLManager.getInstance()` is singleton, so having different keystores for different threads would be problematic
   5) JMeter's "first key index", and "last key index" parameters are really obscure. I would like to just drop them.


-- 
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@jmeter.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org