svn commit: r1813964 - /ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java

[Deepak Dixit-5]

Author: deepak
Date: Wed Nov  1 10:43:14 2017
New Revision: 1813964

URL: http://svn.apache.org/viewvc?rev=1813964&view=rev
Log:
Fixed: doDecrypt method may return ClassNotFoundException, BadPaddingException,so instead of handling GeneralException used Exception class to handle all exception

Modified:
    ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java

Modified: ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java?rev=1813964&r1=1813963&r2=1813964&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java Wed Nov  1 10:43:14 2017
@@ -124,7 +124,7 @@ public final class EntityCrypto {
     public Object decrypt(String keyName, EncryptMethod encryptMethod, String encryptedString) throws EntityCryptoException {
         try {
             return doDecrypt(keyName, encryptMethod, encryptedString, handlers[0]);
-        } catch (GeneralException e) {
+        } catch (Exception e) {
             Debug.logInfo("Decrypt with DES key from standard key name hash failed, trying old/funny variety of key name hash", module);
             for (int i = 1; i < handlers.length; i++) {
                 try {

[Jacques Le Roux]

Hi Deepak,

It's minor, but instead of hiding a possible RuntimeException by catching Exception here I'd rather follow this FindBugs advice

---

This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try

{ ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

try { ... } catch (RuntimeException e) { throw e; } catch (Exception e) { ... deal with all ...}

---

I suggest to use this late solution, as it has for example been done for GroovyUtil.java in r1812059

http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/GroovyUtil.java?r1=1812059&r2=1812058&pathrev=1812059

Thanks

Jacques

Le 01/11/2017 à 11:43, [hidden email] a écrit :

Author: deepak
Date: Wed Nov  1 10:43:14 2017
New Revision: 1813964

URL: http://svn.apache.org/viewvc?rev=1813964&view=rev
Log:
Fixed: doDecrypt method may return ClassNotFoundException, BadPaddingException,so instead of handling GeneralException used Exception class to handle all exception

Modified:
    ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java

Modified: ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java?rev=1813964&r1=1813963&r2=1813964&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java Wed Nov  1 10:43:14 2017
@@ -124,7 +124,7 @@ public final class EntityCrypto {
     public Object decrypt(String keyName, EncryptMethod encryptMethod, String encryptedString) throws EntityCryptoException {
         try {
             return doDecrypt(keyName, encryptMethod, encryptedString, handlers[0]);
-        } catch (GeneralException e) {
+        } catch (Exception e) {
             Debug.logInfo("Decrypt with DES key from standard key name hash failed, trying old/funny variety of key name hash", module);
             for (int i = 1; i < handlers.length; i++) {
                 try {

[Jacques Le Roux]

Done at r1814155

Jacques

Le 01/11/2017 à 14:08, Jacques Le Roux a écrit :

Hi Deepak,

It's minor, but instead of hiding a possible RuntimeException by catching Exception here I'd rather follow this FindBugs advice

---

This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try

{ ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

try { ... } catch (RuntimeException e) { throw e; } catch (Exception e) { ... deal with all ...}

---

I suggest to use this late solution, as it has for example been done for GroovyUtil.java in r1812059

http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/GroovyUtil.java?r1=1812059&r2=1812058&pathrev=1812059

Thanks

Jacques

Le 01/11/2017 à 11:43, [hidden email] a écrit :

Author: deepak
Date: Wed Nov  1 10:43:14 2017
New Revision: 1813964

URL: http://svn.apache.org/viewvc?rev=1813964&view=rev
Log:
Fixed: doDecrypt method may return ClassNotFoundException, BadPaddingException,so instead of handling GeneralException used Exception class to handle all exception

Modified:
    ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java

Modified: ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java?rev=1813964&r1=1813963&r2=1813964&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityCrypto.java Wed Nov  1 10:43:14 2017
@@ -124,7 +124,7 @@ public final class EntityCrypto {
     public Object decrypt(String keyName, EncryptMethod encryptMethod, String encryptedString) throws EntityCryptoException {
         try {
             return doDecrypt(keyName, encryptMethod, encryptedString, handlers[0]);
-        } catch (GeneralException e) {
+        } catch (Exception e) {
             Debug.logInfo("Decrypt with DES key from standard key name hash failed, trying old/funny variety of key name hash", module);
             for (int i = 1; i < handlers.length; i++) {
                 try {