ppalaga commented on code in PR #6241:
URL: https://github.com/apache/camel-quarkus/pull/6241#discussion_r1670565370


##########
extensions-support/bouncycastle/runtime/src/main/java/org/apache/camel/quarkus/support/bouncycastle/BouncyCastleRecorder.java:
##########
@@ -35,11 +34,28 @@ public class BouncyCastleRecorder {
 
     public void registerBouncyCastleProvider(List<String> 
cipherTransformations, ShutdownContext shutdownContext) {
         Provider provider = 
Security.getProvider(SecurityProviderUtils.BOUNCYCASTLE_PROVIDER_NAME);
+        if (provider == null) {
+            provider = 
Security.getProvider(SecurityProviderUtils.BOUNCYCASTLE_FIPS_PROVIDER_NAME);
+        }
         if (provider == null) {
             // TODO: Fix BuildStep execution order so that this is not required
             // https://github.com/apache/camel-quarkus/issues/3472
-            provider = new BouncyCastleProvider();
-            Security.addProvider(provider);
+            try {
+                provider = (Provider) 
Thread.currentThread().getContextClassLoader()
+                        
.loadClass(SecurityProviderUtils.BOUNCYCASTLE_PROVIDER_CLASS_NAME).getConstructor().newInstance();
+                Security.addProvider(provider);
+            } catch (Exception e) {

Review Comment:
   > My idea is that any failure would cause RuntimeException with the message 
`Neither BC nor BCFIPS provider can be registered` + exception message. Which 
is correct also with i.e, NPE
   
   Yeah, that's what the code currently does. My question is whether "any 
failure" including NPEs inside the try block should be caught. If we want to 
detect the availability of bcprov/bcfips by loading some of its classes, then 
we should catch only  exceptions related to classloading. Other exceptions 
which may signal other unrelated error conditions in the code should IMO be 
left uncaught so that users see the real issue including the stack trace. Note 
that the stack traces of both exceptions are lost with the current 
implementation. That's IMO fine for the classloading exceptions (because those 
are kinda expected) but not fine for everything else. 



-- 
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: commits-unsubscr...@camel.apache.org

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

Reply via email to