Hi Deepak,

The problem as I see it is that if a RuntimeException is thrown by the 1st version of doDecrypt() then maybe the 2nd version of doDecrypt() will do so. Maybe I'm wrong here, I did not check each method body.

Actually I think it's more a theoretical issue than a real one because among the possible the possible RuntimeException subclasses[1], even if I did not check them all, I hardly see one being thrown here. But maybe a NPE in some cases? In another word is more to follow a pattern and use it everywhere in our code than about this peculiar issue. You may revert if you want but beware of NPE ;)

Jacques
[1] https://docs.oracle.com/javase/7/docs/api/java/lang/RuntimeException.html


Le 07/11/2017 à 11:19, Deepak Dixit a écrit :
Hi Jacques,

I think we don't need to handle the RuntimeException, as doDecrypt fails to
decrypt then it will try with old pattern, I think this is for backward
compatibility.

Here is the comment from code
try using the old/bad hex encoding approach; this is another path the code
may take, ie if there is an exception thrown in decrypt


Thanks & Regards
--
Deepak Dixit
www.hotwaxsystems.com
www.hotwax.co

On Fri, Nov 3, 2017 at 5:15 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

Hi Deepak,

Actually the rule is quite simple. If, for a reason, you decide to catch a
java.lang.Exception, you need to catch before the
java.lang.RuntimeException.

This is because java.lang.RuntimeException is a java.lang.Exception so you
would hide possible runtime issues by only catching Exception

https://www.tutorialspoint.com/java/images/exceptions1.jpg

HTH

Jacques


Le 03/11/2017 à 11:17, Deepak Dixit a écrit :

Thanks Jacques,
I was engage in other work so did not get a chance to test your
suggestion... :)

Thanks & Regards
--
Deepak Dixit
www.hotwaxsystems.com
www.hotwax.co

On Fri, Nov 3, 2017 at 3:19 PM, Jacques Le Roux <
jacques.le.r...@les7arts.com> wrote:

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, dee...@apache.org 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/fra
mework/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 {









Reply via email to