You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "LuciferYang (via GitHub)" <gi...@apache.org> on 2023/10/22 10:08:10 UTC

[PR] [CRYPTO-169] Add protection for the `ExceptionInInitializerError` scenario to the `CryptoRandomFactory#getCryptoRandom(Properties)` method [commons-crypto]

LuciferYang opened a new pull request, #258:
URL: https://github.com/apache/commons-crypto/pull/258

   In version 1.2.0 of `commons-crypto`, the static initialization code block of `o.a.c.crypto.random.OpenSslCryptoRandom` may throw a `GeneralSecurityException` wrapped by `IllegalStateException`, which ultimately gets wrapped into `j.l.ExceptionInInitializerError` by Java's own mechanism.
   
   This results in behavior differences when running the statement 
   
   ```java
   org.apache.commons.crypto.random.CryptoRandomFactory#getCryptoRandom(new java.util.Properties());
   ```
   on platforms that do not support `OpenSslCryptoRandom` compared to when using `commons-crypto` 1.1.0 (e.g. Apple Silicon)
   
   - `commons-crypto` 1.1.0
   
   After `OpenSslCryptoRandom` initialization fails, it tries to initialize `JavaCryptoRandom`, and `JavaCryptoRandom` can be successfully initialized and return results.
   
   - `commons-crypto` 1.2.0
   
   After `OpenSslCryptoRandom` initialization fails, it throws an `ExceptionInInitializerError`. Since the `CryptoRandomFactory#getCryptoRandom` method does not catch `ExceptionInInitializerError` and perform fault tolerance, `ExceptionInInitializerError` continues to be thrown upward, losing the opportunity to try to initialize `JavaCryptoRandom`.
   
   Therefore, this PR adds the catch and fault tolerance for `ExceptionInInitializerError` in the `CryptoRandomFactory#getCryptoRandom` method to keep it behaving similarly to `commons-crypto` 1.1.0.


-- 
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: issues-unsubscribe@commons.apache.org

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

Re: [PR] [CRYPTO-169] Add protection for the `ExceptionInInitializerError` scenario to the `CryptoRandomFactory#getCryptoRandom(Properties)` method [commons-crypto]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #258:
URL: https://github.com/apache/commons-crypto/pull/258


-- 
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: issues-unsubscribe@commons.apache.org

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

Re: [PR] [CRYPTO-169] Add protection for the `ExceptionInInitializerError` scenario to the `CryptoRandomFactory#getCryptoRandom(Properties)` method [commons-crypto]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #258:
URL: https://github.com/apache/commons-crypto/pull/258#issuecomment-1774092634

   > Thanks @garydgregory ~
   
   YW! 😄 


-- 
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: issues-unsubscribe@commons.apache.org

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

Re: [PR] [CRYPTO-169] Add protection for the `ExceptionInInitializerError` scenario to the `CryptoRandomFactory#getCryptoRandom(Properties)` method [commons-crypto]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #258:
URL: https://github.com/apache/commons-crypto/pull/258#issuecomment-1774052862

   ## [Codecov](https://app.codecov.io/gh/apache/commons-crypto/pull/258?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   > Merging [#258](https://app.codecov.io/gh/apache/commons-crypto/pull/258?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (e0fd653) into [master](https://app.codecov.io/gh/apache/commons-crypto/commit/c5d19d017d96e34a4589736aeb48bc1d59e204a6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (c5d19d0) will **decrease** coverage by `0.25%`.
   > The diff coverage is `0.00%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master     #258      +/-   ##
   ============================================
   - Coverage     74.69%   74.44%   -0.25%     
     Complexity      441      441              
   ============================================
     Files            38       38              
     Lines          1810     1816       +6     
     Branches        176      177       +1     
   ============================================
     Hits           1352     1352              
   - Misses          348      354       +6     
     Partials        110      110              
   ```
   
   
   | [Files](https://app.codecov.io/gh/apache/commons-crypto/pull/258?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [...che/commons/crypto/random/CryptoRandomFactory.java](https://app.codecov.io/gh/apache/commons-crypto/pull/258?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvY3J5cHRvL3JhbmRvbS9DcnlwdG9SYW5kb21GYWN0b3J5LmphdmE=) | `82.00% <0.00%> (-11.19%)` | :arrow_down: |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   


-- 
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: notifications-unsubscribe@commons.apache.org

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

Re: [PR] [CRYPTO-169] Add protection for the `ExceptionInInitializerError` scenario to the `CryptoRandomFactory#getCryptoRandom(Properties)` method [commons-crypto]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #258:
URL: https://github.com/apache/commons-crypto/pull/258#discussion_r1367880514


##########
src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java:
##########
@@ -141,4 +141,14 @@ public void testNull() {
         assertThrows(NullPointerException.class, () -> CryptoRandomFactory.getCryptoRandom(null));
     }
 
+    @Test
+    public void testExceptionInInitializerErrorRandom() throws GeneralSecurityException, IOException {

Review Comment:
   If there are no chagnes to `CryptoRandomFactory.java`, the new test case will fail:
   
   ```
   	java.lang.ExceptionInInitializerError
   	at java.base/java.lang.Class.forName0(Native Method)
   	at java.base/java.lang.Class.forName(Class.java:534)
   	at java.base/java.lang.Class.forName(Class.java:513)
   	at org.apache.commons.crypto.utils.ReflectionUtils.getClassByNameOrNull(ReflectionUtils.java:93)
   	at org.apache.commons.crypto.utils.ReflectionUtils.getClassByName(ReflectionUtils.java:64)
   	at org.apache.commons.crypto.random.CryptoRandomFactory.getCryptoRandom(CryptoRandomFactory.java:189)
   	at org.apache.commons.crypto.random.CryptoRandomFactoryTest.testExceptionInInitializerErrorRandom(CryptoRandomFactoryTest.java:150)
   	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
   	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
   	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
   Caused by: java.lang.IllegalStateException: java.security.GeneralSecurityException: ExceptionInInitializerErrorRandom init failed
   	at org.apache.commons.crypto.random.ExceptionInInitializerErrorRandom.<clinit>(ExceptionInInitializerErrorRandom.java:28)
   	... 10 more
   Caused by: java.security.GeneralSecurityException: ExceptionInInitializerErrorRandom init failed
   	at org.apache.commons.crypto.random.ExceptionInInitializerErrorRandom.check(ExceptionInInitializerErrorRandom.java:33)
   	at org.apache.commons.crypto.random.ExceptionInInitializerErrorRandom.<clinit>(ExceptionInInitializerErrorRandom.java:26)
   	... 10 more
   ```



-- 
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: issues-unsubscribe@commons.apache.org

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

Re: [PR] [CRYPTO-169] Add protection for the `ExceptionInInitializerError` scenario to the `CryptoRandomFactory#getCryptoRandom(Properties)` method [commons-crypto]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #258:
URL: https://github.com/apache/commons-crypto/pull/258#discussion_r1367880514


##########
src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java:
##########
@@ -141,4 +141,14 @@ public void testNull() {
         assertThrows(NullPointerException.class, () -> CryptoRandomFactory.getCryptoRandom(null));
     }
 
+    @Test
+    public void testExceptionInInitializerErrorRandom() throws GeneralSecurityException, IOException {

Review Comment:
   If there are no chagnes to `CryptoRandomFactory.java`, the new test case will fail:
   
   ```
   java.lang.ExceptionInInitializerError
   	at java.base/java.lang.Class.forName0(Native Method)
   	at java.base/java.lang.Class.forName(Class.java:534)
   	at java.base/java.lang.Class.forName(Class.java:513)
   	at org.apache.commons.crypto.utils.ReflectionUtils.getClassByNameOrNull(ReflectionUtils.java:93)
   	at org.apache.commons.crypto.utils.ReflectionUtils.getClassByName(ReflectionUtils.java:64)
   	at org.apache.commons.crypto.random.CryptoRandomFactory.getCryptoRandom(CryptoRandomFactory.java:189)
   	at org.apache.commons.crypto.random.CryptoRandomFactoryTest.testExceptionInInitializerErrorRandom(CryptoRandomFactoryTest.java:150)
   	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
   	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
   	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
   Caused by: java.lang.IllegalStateException: java.security.GeneralSecurityException: ExceptionInInitializerErrorRandom init failed
   	at org.apache.commons.crypto.random.ExceptionInInitializerErrorRandom.<clinit>(ExceptionInInitializerErrorRandom.java:28)
   	... 10 more
   Caused by: java.security.GeneralSecurityException: ExceptionInInitializerErrorRandom init failed
   	at org.apache.commons.crypto.random.ExceptionInInitializerErrorRandom.check(ExceptionInInitializerErrorRandom.java:33)
   	at org.apache.commons.crypto.random.ExceptionInInitializerErrorRandom.<clinit>(ExceptionInInitializerErrorRandom.java:26)
   	... 10 more
   ```



-- 
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: issues-unsubscribe@commons.apache.org

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

Re: [PR] [CRYPTO-169] Add protection for the `ExceptionInInitializerError` scenario to the `CryptoRandomFactory#getCryptoRandom(Properties)` method [commons-crypto]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #258:
URL: https://github.com/apache/commons-crypto/pull/258#discussion_r1367880514


##########
src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java:
##########
@@ -141,4 +141,14 @@ public void testNull() {
         assertThrows(NullPointerException.class, () -> CryptoRandomFactory.getCryptoRandom(null));
     }
 
+    @Test
+    public void testExceptionInInitializerErrorRandom() throws GeneralSecurityException, IOException {

Review Comment:
   
   
   
   	If there are no chagnes to `CryptoRandomFactory.java`, the new test case will fail:
   
   	```
   	java.lang.ExceptionInInitializerError
   	at java.base/java.lang.Class.forName0(Native Method)
   	at java.base/java.lang.Class.forName(Class.java:534)
   	at java.base/java.lang.Class.forName(Class.java:513)
   	at org.apache.commons.crypto.utils.ReflectionUtils.getClassByNameOrNull(ReflectionUtils.java:93)
   	at org.apache.commons.crypto.utils.ReflectionUtils.getClassByName(ReflectionUtils.java:64)
   	at org.apache.commons.crypto.random.CryptoRandomFactory.getCryptoRandom(CryptoRandomFactory.java:189)
   	at org.apache.commons.crypto.random.CryptoRandomFactoryTest.testExceptionInInitializerErrorRandom(CryptoRandomFactoryTest.java:150)
   	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
   	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
   	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
   Caused by: java.lang.IllegalStateException: java.security.GeneralSecurityException: ExceptionInInitializerErrorRandom init failed
   	at org.apache.commons.crypto.random.ExceptionInInitializerErrorRandom.<clinit>(ExceptionInInitializerErrorRandom.java:28)
   	... 10 more
   Caused by: java.security.GeneralSecurityException: ExceptionInInitializerErrorRandom init failed
   	at org.apache.commons.crypto.random.ExceptionInInitializerErrorRandom.check(ExceptionInInitializerErrorRandom.java:33)
   	at org.apache.commons.crypto.random.ExceptionInInitializerErrorRandom.<clinit>(ExceptionInInitializerErrorRandom.java:26)
   	... 10 more
   	```



-- 
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: issues-unsubscribe@commons.apache.org

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

Re: [PR] [CRYPTO-169] Add protection for the `ExceptionInInitializerError` scenario to the `CryptoRandomFactory#getCryptoRandom(Properties)` method [commons-crypto]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #258:
URL: https://github.com/apache/commons-crypto/pull/258#issuecomment-1774075484

   JIRA: https://issues.apache.org/jira/browse/CRYPTO-169


-- 
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: issues-unsubscribe@commons.apache.org

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

Re: [PR] [CRYPTO-169] Add protection for the `ExceptionInInitializerError` scenario to the `CryptoRandomFactory#getCryptoRandom(Properties)` method [commons-crypto]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #258:
URL: https://github.com/apache/commons-crypto/pull/258#issuecomment-1774075930

   Thanks @garydgregory  ~


-- 
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: issues-unsubscribe@commons.apache.org

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

Re: [PR] [CRYPTO-169] Add protection for the `ExceptionInInitializerError` scenario to the `CryptoRandomFactory#getCryptoRandom(Properties)` method [commons-crypto]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #258:
URL: https://github.com/apache/commons-crypto/pull/258#discussion_r1367880919


##########
src/test/java/org/apache/commons/crypto/random/ExceptionInInitializerErrorRandom.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.crypto.random;
+
+import java.io.IOException;
+import java.security.GeneralSecurityException;
+
+public class ExceptionInInitializerErrorRandom implements CryptoRandom {

Review Comment:
   This class is used to simulate scenarios where `OpenSslCryptoRandom` fails in the static code block `checkNative()` or `!OpenSslCryptoRandomNative.nextRandBytes(new byte[1])` is false.



-- 
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: issues-unsubscribe@commons.apache.org

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