geomacy commented on a change in pull request #1087: leave a note to tell user
this is not secure
URL: https://github.com/apache/brooklyn-server/pull/1087#discussion_r388632084
##########
File path:
utils/common/src/main/java/org/apache/brooklyn/util/crypto/SslTrustUtils.java
##########
@@ -39,7 +39,7 @@
return connection;
}
- /** trusts all SSL certificates */
+ /** trusts all SSL certificates, it is not secure*/
Review comment:
hi @YYTVicky thanks for the PR! One comment I would make is that if we are
going to warn users about something not being secure, we should explain _why_
it is not secure. The additional comment here would probably need to have more
explanation in it.
However, I'm not sure that it is right simply to say "it is not secure"
here. Even though this policy trusts all certificates, whether or not this is
secure depends on where and how you are using it. I can certainly imagine
situations in which this would be the wrong class to use. On the other hand
there may be situations in which you do want to trust all certificates (e.g. in
unit tests where you are not concerned with the trustworthiness of the
certificate). Maybe you could reword the comment along the lines of suggesting
that users should be aware of any security implications of trusting all certs
before using this class?
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
