Repository: commons-crypto Updated Branches: refs/heads/master b1d66b82a -> ee2136e54
Add tests to show that IllegalArgumentException is being thrown Fix bug in CryptoCipherFactory - did not throw IAE because errorMessage buffer is not initially empty Simplify code by checking the list size Project: http://git-wip-us.apache.org/repos/asf/commons-crypto/repo Commit: http://git-wip-us.apache.org/repos/asf/commons-crypto/commit/ee2136e5 Tree: http://git-wip-us.apache.org/repos/asf/commons-crypto/tree/ee2136e5 Diff: http://git-wip-us.apache.org/repos/asf/commons-crypto/diff/ee2136e5 Branch: refs/heads/master Commit: ee2136e547836f6b9c69d5636c34e0d825bcd3f9 Parents: b1d66b8 Author: Sebb <[email protected]> Authored: Wed Jul 6 00:12:35 2016 +0100 Committer: Sebb <[email protected]> Committed: Wed Jul 6 00:12:35 2016 +0100 ---------------------------------------------------------------------- .../commons/crypto/cipher/CryptoCipherFactory.java | 12 +++++++----- .../commons/crypto/random/CryptoRandomFactory.java | 11 ++++++----- .../commons/crypto/cipher/CryptoCipherFactoryTest.java | 10 ++++++++++ .../commons/crypto/random/CryptoRandomFactoryTest.java | 9 +++++++++ 4 files changed, 32 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/commons-crypto/blob/ee2136e5/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 5d5b789..08eea45 100644 --- a/src/main/java/org/apache/commons/crypto/cipher/CryptoCipherFactory.java +++ b/src/main/java/org/apache/commons/crypto/cipher/CryptoCipherFactory.java @@ -18,6 +18,7 @@ package org.apache.commons.crypto.cipher; import java.security.GeneralSecurityException; +import java.util.List; import java.util.Properties; import org.apache.commons.crypto.Crypto; @@ -130,15 +131,19 @@ public class CryptoCipherFactory { * @param transformation algorithm/mode/padding * @return CryptoCipher the cipher (defaults to OpenSslCipher) * @throws GeneralSecurityException if cipher initialize failed - * @throws IllegalArgumentException if no classname(s) + * @throws IllegalArgumentException if no classname(s) were provided */ public static CryptoCipher getCryptoCipher(String transformation, Properties props) throws GeneralSecurityException { + final List<String> names = Utils.splitClassNames(getCipherClassString(props), ","); + if (names.size() == 0) { + throw new IllegalArgumentException("No classname(s) provided"); + } CryptoCipher cipher = null; StringBuilder errorMessage = new StringBuilder("CryptoCipher "); - for (String klass : Utils.splitClassNames(getCipherClassString(props), ",")) { + for (String klass : names) { try { Class<?> cls = ReflectionUtils.getClassByName(klass); cipher = ReflectionUtils.newInstance(cls.asSubclass @@ -154,9 +159,6 @@ public class CryptoCipherFactory { if (cipher != null) { return cipher; } - if (errorMessage.length() == 0) { - throw new IllegalArgumentException("No classname(s) provided"); - } errorMessage.append(" is not available or transformation " + transformation + " is not supported."); throw new GeneralSecurityException(errorMessage.toString()); http://git-wip-us.apache.org/repos/asf/commons-crypto/blob/ee2136e5/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 1929685..aa046ec 100644 --- a/src/main/java/org/apache/commons/crypto/random/CryptoRandomFactory.java +++ b/src/main/java/org/apache/commons/crypto/random/CryptoRandomFactory.java @@ -18,6 +18,7 @@ package org.apache.commons.crypto.random; import java.security.GeneralSecurityException; +import java.util.List; import java.util.Properties; import org.apache.commons.crypto.Crypto; @@ -181,10 +182,13 @@ public class CryptoRandomFactory { */ public static CryptoRandom getCryptoRandom(Properties props) throws GeneralSecurityException { + final List<String> names = Utils.splitClassNames(getRandomClassString(props), ","); + if (names.size() == 0) { + throw new IllegalArgumentException("No classname(s) provided"); + } StringBuilder errorMessage = new StringBuilder(); CryptoRandom random = null; - for (String klassName : Utils.splitClassNames( - getRandomClassString(props), ",")) { + for (String klassName : names) { try { final Class<?> klass = ReflectionUtils.getClassByName(klassName); random = (CryptoRandom) ReflectionUtils.newInstance(klass, props); @@ -203,9 +207,6 @@ public class CryptoRandomFactory { if (random != null) { return random; } - if (errorMessage.length() == 0) { - throw new IllegalArgumentException("No classname(s) provided"); - } throw new GeneralSecurityException(errorMessage.toString()); } http://git-wip-us.apache.org/repos/asf/commons-crypto/blob/ee2136e5/src/test/java/org/apache/commons/crypto/cipher/CryptoCipherFactoryTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/commons/crypto/cipher/CryptoCipherFactoryTest.java b/src/test/java/org/apache/commons/crypto/cipher/CryptoCipherFactoryTest.java index 2544717..f47e7e0 100644 --- a/src/test/java/org/apache/commons/crypto/cipher/CryptoCipherFactoryTest.java +++ b/src/test/java/org/apache/commons/crypto/cipher/CryptoCipherFactoryTest.java @@ -63,4 +63,14 @@ public class CryptoCipherFactoryTest { Properties properties = new Properties(); CryptoCipherFactory.getCryptoCipher("AES/Invalid/NoPadding", properties); } + + @Test(expected = IllegalArgumentException.class) + public void testNoCipher() throws Exception { + Properties properties = new Properties(); + // An empty string currently means use the default + // However the splitter drops empty fields + properties.setProperty(CryptoCipherFactory.CLASSES_KEY, ","); + CryptoCipherFactory.getCryptoCipher("AES/CBC/NoPadding", properties); + } + } http://git-wip-us.apache.org/repos/asf/commons-crypto/blob/ee2136e5/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 662c147..0d93aeb 100644 --- a/src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java +++ b/src/test/java/org/apache/commons/crypto/random/CryptoRandomFactoryTest.java @@ -106,4 +106,13 @@ public class CryptoRandomFactoryTest { } } + @Test(expected=IllegalArgumentException.class) + public void testNoClasses() throws Exception { + final Properties props = new Properties(); + // An empty string currently means use the default + // However the splitter drops empty fields + props.setProperty(CryptoRandomFactory.CLASSES_KEY, ","); + CryptoRandomFactory.getCryptoRandom(props); + } + }
