You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2021/07/26 03:10:17 UTC

[GitHub] [activemq-artemis] jbertram opened a new pull request #3671: ARTEMIS-3302 swap deprecated X509Certificate

jbertram opened a new pull request #3671:
URL: https://github.com/apache/activemq-artemis/pull/3671


   Casting the result of getPeerCertificates() to X509Certificate[] mirrors
   what is done in the ActiveMQ "Classic" code-base.


-- 
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: gitbox-unsubscribe@activemq.apache.org

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



[GitHub] [activemq-artemis] jbertram commented on pull request #3671: ARTEMIS-3302 swap deprecated X509Certificate

Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3671:
URL: https://github.com/apache/activemq-artemis/pull/3671#issuecomment-887674273


   @gemmellr, I updated everything except `org.apache.activemq.transport.tcp.StubSSLSession` because it is actually implementing `javax.net.ssl.SSLSession` so it *must* use `javax.security.cert.X509Certificate[]`. This, of course, will cause a problem when it is eventually removed as that class will still work in older JDKs but will fail in the latest. It looks like the only place where `StubSSLSession` is used is `org.apache.activemq.transport.tcp.SslTransportTest`. I'm not sure if we should wait and remove that test now or later. What do you think?


-- 
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: gitbox-unsubscribe@activemq.apache.org

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



[GitHub] [activemq-artemis] asfgit closed pull request #3671: ARTEMIS-3302 swap deprecated X509Certificate

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3671:
URL: https://github.com/apache/activemq-artemis/pull/3671


   


-- 
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: gitbox-unsubscribe@activemq.apache.org

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



[GitHub] [activemq-artemis] gemmellr commented on pull request #3671: ARTEMIS-3302 swap deprecated X509Certificate

Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #3671:
URL: https://github.com/apache/activemq-artemis/pull/3671#issuecomment-888209490


   @jbertram Ah. I didnt actually look at every use so hadnt spotted that hehe.
   
   Looking at what StubSSLSession is doing I would probably just try to replace it, e.g with an mock, or faking one with a Proxy. Its not really implementing any but 1 of the methods, and not the one of concern as it is ironically using the intended replacement. That one test usage isnt even using the full functionality implemented in the stub, as its only used when verification is required, and so the stub is arguably even over-engineered. A replacement could just be doing a trivial 'return <value>' action for the interesting method.
   
   The test could even alternatively be implemented with a real SSLSession.


-- 
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: gitbox-unsubscribe@activemq.apache.org

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



[GitHub] [activemq-artemis] gemmellr commented on pull request #3671: ARTEMIS-3302 swap deprecated X509Certificate

Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #3671:
URL: https://github.com/apache/activemq-artemis/pull/3671#issuecomment-888222555


   So, actually looking at the wider usage and considering it a bit more....the particular use is in testing for a SslTransport class coming from the _activemq-client_ 5.14.0 dependency used in that module. Seems those tests arent even using the same version that the artemis-openwire-protocol actually does, since that is on 5.16.0.
   
   Its not clear to me that the class under test is actually used anywhere in the broker. If not it would seem like the stub and related test etc should just be straight up deleted because they would be extraneous already, not testing anything the artemis codebase could influence.


-- 
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: gitbox-unsubscribe@activemq.apache.org

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



[GitHub] [activemq-artemis] gemmellr commented on pull request #3671: ARTEMIS-3302 swap deprecated X509Certificate

Posted by GitBox <gi...@apache.org>.
gemmellr commented on pull request #3671:
URL: https://github.com/apache/activemq-artemis/pull/3671#issuecomment-887550638


   I think you may have missed some related instances, in the test modules I think, as the PR only touches 13 files but the related warnings are emitted for 16 files. I added a list on https://issues.apache.org/jira/browse/ARTEMIS-3302.
   
   (Should be more visible in logs of a fresh CI run, now that I fixed a similar problem with my own earlier changes on ARTEMIS 3304 via d9a44002c5fe177ccb612197c1db59e22d62c90e)


-- 
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: gitbox-unsubscribe@activemq.apache.org

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