CRYPTO-113 Improve error reporting by factories Project: http://git-wip-us.apache.org/repos/asf/commons-crypto/repo Commit: http://git-wip-us.apache.org/repos/asf/commons-crypto/commit/79502d45 Tree: http://git-wip-us.apache.org/repos/asf/commons-crypto/tree/79502d45 Diff: http://git-wip-us.apache.org/repos/asf/commons-crypto/diff/79502d45
Branch: refs/heads/CRYPTO-1.0.0 Commit: 79502d4544d42c020520d7f6cdb69ff9c1631554 Parents: bc7c116 Author: Sebb <s...@apache.org> Authored: Sun Jul 10 00:47:59 2016 +0100 Committer: Sebb <s...@apache.org> Committed: Sun Jul 10 00:47:59 2016 +0100 ---------------------------------------------------------------------- .../crypto/cipher/CryptoCipherFactory.java | 4 +- .../crypto/random/CryptoRandomFactory.java | 6 ++- .../crypto/random/CryptoRandomFactoryTest.java | 18 +++++++++ .../commons/crypto/random/FailingRandom.java | 39 ++++++++++++++++++++ .../crypto/random/OsCryptoRandomTest.java | 27 ++++++++++++++ 5 files changed, 92 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/commons-crypto/blob/79502d45/src/main/java/org/apache/commons/crypto/cipher/CryptoCipherFactory.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/crypto/cipher/CryptoCipherFactory.java b/src/main/java/org/apache/commons/crypto/cipher/CryptoCipherFactory.java index 7f63376..c34753a 100644 --- a/src/main/java/org/apache/commons/crypto/cipher/CryptoCipherFactory.java +++ b/src/main/java/org/apache/commons/crypto/cipher/CryptoCipherFactory.java @@ -148,6 +148,7 @@ public class CryptoCipherFactory { throw new IllegalArgumentException("No classname(s) provided"); } CryptoCipher cipher = null; + Exception lastException = null; StringBuilder errorMessage = new StringBuilder("CryptoCipher "); for (String klass : names) { @@ -159,6 +160,7 @@ public class CryptoCipherFactory { break; } } catch (Exception e) { + lastException = e; errorMessage.append("{" + klass + "}"); } } @@ -168,7 +170,7 @@ public class CryptoCipherFactory { } errorMessage.append(" is not available or transformation " + transformation + " is not supported."); - throw new GeneralSecurityException(errorMessage.toString()); + throw new GeneralSecurityException(errorMessage.toString(), lastException); } /** http://git-wip-us.apache.org/repos/asf/commons-crypto/blob/79502d45/src/main/java/org/apache/commons/crypto/random/CryptoRandomFactory.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/crypto/random/CryptoRandomFactory.java b/src/main/java/org/apache/commons/crypto/random/CryptoRandomFactory.java index aa046ec..853a079 100644 --- a/src/main/java/org/apache/commons/crypto/random/CryptoRandomFactory.java +++ b/src/main/java/org/apache/commons/crypto/random/CryptoRandomFactory.java @@ -188,6 +188,7 @@ public class CryptoRandomFactory { } StringBuilder errorMessage = new StringBuilder(); CryptoRandom random = null; + Exception lastException = null; for (String klassName : names) { try { final Class<?> klass = ReflectionUtils.getClassByName(klassName); @@ -196,10 +197,13 @@ public class CryptoRandomFactory { break; } } catch (ClassCastException e) { + lastException = e; errorMessage.append("Class: [" + klassName + "] is not a CryptoRandom."); } catch (ClassNotFoundException e) { + lastException = e; errorMessage.append("CryptoRandom: [" + klassName + "] not found."); } catch (Exception e) { + lastException = e; errorMessage.append("CryptoRandom: [" + klassName + "] failed with " + e.getMessage()); } } @@ -207,7 +211,7 @@ public class CryptoRandomFactory { if (random != null) { return random; } - throw new GeneralSecurityException(errorMessage.toString()); + throw new GeneralSecurityException(errorMessage.toString(), lastException); } /** http://git-wip-us.apache.org/repos/asf/commons-crypto/blob/79502d45/src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java b/src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java index 2901404..f4f1397 100644 --- a/src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java +++ b/src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java @@ -17,6 +17,7 @@ */ package org.apache.commons.crypto.random; +import java.lang.reflect.InvocationTargetException; import java.security.GeneralSecurityException; import java.util.Properties; @@ -122,4 +123,21 @@ public class CryptoRandomFactoryTest { CryptoRandomFactory.getCryptoRandom(props); } + @Test + public void testFailingRandom() { + Properties props = new Properties(); + props.setProperty(CryptoRandomFactory.CLASSES_KEY, FailingRandom.class.getName()); + try { + CryptoRandomFactory.getCryptoRandom(props); + Assert.fail("Expected GeneralSecurityException"); + } catch (GeneralSecurityException e) { + Throwable cause = e.getCause(); + Assert.assertEquals(RuntimeException.class, cause.getClass()); + cause = cause.getCause(); + Assert.assertEquals(InvocationTargetException.class, cause.getClass()); + cause = cause.getCause(); + Assert.assertEquals(UnsatisfiedLinkError.class, cause.getClass()); + } + } + } http://git-wip-us.apache.org/repos/asf/commons-crypto/blob/79502d45/src/test/java/org/apache/commons/crypto/random/FailingRandom.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/commons/crypto/random/FailingRandom.java b/src/test/java/org/apache/commons/crypto/random/FailingRandom.java new file mode 100644 index 0000000..ce19126 --- /dev/null +++ b/src/test/java/org/apache/commons/crypto/random/FailingRandom.java @@ -0,0 +1,39 @@ +/* + * 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.util.Properties; + +class FailingRandom implements CryptoRandom { + + // Should fail with NoSuchMethodException + FailingRandom(Properties props) { + NoSuchMethod(); + } + + @Override + public void close() throws IOException { + } + + @Override + public void nextBytes(byte[] bytes) { + } + + public static native void NoSuchMethod(); +} http://git-wip-us.apache.org/repos/asf/commons-crypto/blob/79502d45/src/test/java/org/apache/commons/crypto/random/OsCryptoRandomTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/commons/crypto/random/OsCryptoRandomTest.java b/src/test/java/org/apache/commons/crypto/random/OsCryptoRandomTest.java index 69f6d6a..6382771 100644 --- a/src/test/java/org/apache/commons/crypto/random/OsCryptoRandomTest.java +++ b/src/test/java/org/apache/commons/crypto/random/OsCryptoRandomTest.java @@ -19,10 +19,15 @@ package org.apache.commons.crypto.random; import static org.junit.Assert.fail; +import java.io.FileNotFoundException; +import java.lang.reflect.InvocationTargetException; import java.security.GeneralSecurityException; import java.util.Properties; import org.junit.Assume; +import org.junit.Test; + +import junit.framework.Assert; public class OsCryptoRandomTest extends AbstractRandomTest { @@ -41,4 +46,26 @@ public class OsCryptoRandomTest extends AbstractRandomTest { } return random; } + + @Test + public void testInvalidRansom() { + Properties props = new Properties(); + props.setProperty(CryptoRandomFactory.CLASSES_KEY, OsCryptoRandom.class.getName()); + // Invalid device + props.setProperty(CryptoRandomFactory.DEVICE_FILE_PATH_KEY, ""); + try { + CryptoRandomFactory.getCryptoRandom(props); + fail("Expected GeneralSecurityException"); + } catch (GeneralSecurityException e) { + Throwable cause; + cause = e.getCause(); + Assert.assertEquals(RuntimeException.class, cause.getClass()); + cause = cause.getCause(); + Assert.assertEquals(InvocationTargetException.class, cause.getClass()); + cause = cause.getCause(); + Assert.assertEquals(RuntimeException.class, cause.getClass()); + cause = cause.getCause(); + Assert.assertEquals(FileNotFoundException.class, cause.getClass()); + } + } }
