coheigea commented on code in PR #218:
URL: 
https://github.com/apache/santuario-xml-security-java/pull/218#discussion_r1348907890


##########
src/main/java/org/apache/xml/security/stax/ext/XMLSecurityConstants.java:
##########
@@ -73,17 +74,34 @@ protected XMLSecurityConstants() {
     }
 
     /**
-     * Generate bytes of the given length using the supplied algorithm in 
RANDOM_ALGORITHM_KEY or,
-     * if not specified, use SecureRandom instance from default constructor. 
The SecureRandom
+     * Generate bytes of the given length using the algorithm supplied in 
{@value #RANDOM_ALGORITHM_KEY} or,
+     * if not specified, use a {@code SecureRandom} instance from default 
constructor. The {@code SecureRandom}
      * instance that backs this method is cached for efficiency.
      *
-     * @return a byte array of the given length
+     * @return a new byte array of the given length
      * @throws XMLSecurityException
      */
     public static byte[] generateBytes(int length) throws XMLSecurityException 
{
+
+        SecureRandom rnd = SECURE_RANDOM;
+        if (rnd == null) {
+            synchronized (SECURE_RANDOM_LOCK) {
+                rnd = SECURE_RANDOM;
+                if (rnd == null) {
+                    try {
+                        String prngAlgorithm = 
System.getProperty("org.apache.xml.security.securerandom.algorithm");
+                        SECURE_RANDOM = rnd = prngAlgorithm != null ? 
SecureRandom.getInstance(prngAlgorithm)
+                                : new SecureRandom();
+                    } catch (NoSuchAlgorithmException e) {
+                        throw new RuntimeException(e);

Review Comment:
   I'm not so keen on the synchronized block, would it be possible to use the 
"Lazy Initialization Holder Class Idiom" (i.e. 
https://dzone.com/articles/another-singleton-implementation), would this work 
with Quarkus?



##########
src/main/java/org/apache/xml/security/stax/ext/XMLSecurityConstants.java:
##########
@@ -73,17 +74,34 @@ protected XMLSecurityConstants() {
     }
 
     /**
-     * Generate bytes of the given length using the supplied algorithm in 
RANDOM_ALGORITHM_KEY or,
-     * if not specified, use SecureRandom instance from default constructor. 
The SecureRandom
+     * Generate bytes of the given length using the algorithm supplied in 
{@value #RANDOM_ALGORITHM_KEY} or,
+     * if not specified, use a {@code SecureRandom} instance from default 
constructor. The {@code SecureRandom}
      * instance that backs this method is cached for efficiency.
      *
-     * @return a byte array of the given length
+     * @return a new byte array of the given length
      * @throws XMLSecurityException
      */
     public static byte[] generateBytes(int length) throws XMLSecurityException 
{
+
+        SecureRandom rnd = SECURE_RANDOM;
+        if (rnd == null) {
+            synchronized (SECURE_RANDOM_LOCK) {
+                rnd = SECURE_RANDOM;
+                if (rnd == null) {
+                    try {
+                        String prngAlgorithm = 
System.getProperty("org.apache.xml.security.securerandom.algorithm");

Review Comment:
   Couldn't we keep the System.getProperty as a static variable read at 
initialization?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to