You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/04/21 21:31:22 UTC
[GitHub] [commons-crypto] geoffreyblake opened a new pull request #97: Additional unit tests for JNA, Cipher, Random, Utils testing error inputs
geoffreyblake opened a new pull request #97:
URL: https://github.com/apache/commons-crypto/pull/97
Add unit tests that exercise error paths for JNA, Cipher, Random, and Utils parts of the package. Raises coverage overall by a few percentage points. These tests touch some of the error checking deep in the library as much as possible.
Tested on MacOS AMD64, RHEL8 x86 and RHEL8 aarch64.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-crypto] geoffreyblake commented on issue #97: Additional unit tests for JNA, Cipher, Random, Utils testing error inputs
Posted by GitBox <gi...@apache.org>.
geoffreyblake commented on issue #97:
URL: https://github.com/apache/commons-crypto/pull/97#issuecomment-618402486
Not at all. Saves me the time of making the changes and pushing. It appears the checks are passing, the code coverage has gone up too, looks ready to merge.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-crypto] geoffreyblake commented on a change in pull request #97: Additional unit tests for JNA, Cipher, Random, Utils testing error inputs
Posted by GitBox <gi...@apache.org>.
geoffreyblake commented on a change in pull request #97:
URL: https://github.com/apache/commons-crypto/pull/97#discussion_r413105005
##########
File path: src/test/java/org/apache/commons/crypto/cipher/OpenSslCipherTest.java
##########
@@ -136,6 +137,15 @@ public void testInvalidIV() throws Exception {
cipher.init(OpenSsl.ENCRYPT_MODE, KEY, new IvParameterSpec(invalidIV));
}
+ @Test(expected = InvalidAlgorithmParameterException.class, timeout = 120000)
+ public void testInvalidIVClass() throws Exception {
+ final OpenSsl cipher = OpenSsl.getInstance("AES/CTR/NoPadding");
+ Assert.assertNotNull(cipher);
+
+ cipher.init(OpenSsl.ENCRYPT_MODE, KEY, new GCMParameterSpec(IV.length, IV));
+ Assert.fail("Should have caught an InvalidAlgorithmParameterException");
Review comment:
Fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-crypto] coveralls edited a comment on issue #97: Additional unit tests for JNA, Cipher, Random, Utils testing error inputs
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #97:
URL: https://github.com/apache/commons-crypto/pull/97#issuecomment-617451912
[](https://coveralls.io/builds/30273766)
Coverage increased (+0.9%) to 76.919% when pulling **19f135852a960e6e9446de75228c800c26492210 on geoffreyblake:random_units** into **24897862c41a504a6987a65727669268019f6f2f on apache:master**.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-crypto] vanzin commented on a change in pull request #97: Additional unit tests for JNA, Cipher, Random, Utils testing error inputs
Posted by GitBox <gi...@apache.org>.
vanzin commented on a change in pull request #97:
URL: https://github.com/apache/commons-crypto/pull/97#discussion_r412565939
##########
File path: src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java
##########
@@ -147,6 +153,75 @@ public void cryptoTest() throws Exception {
}
}
+ @Test
+ public void testNullTransform() throws Exception {
+ for (String transform : transformations) {
Review comment:
`transform` is not used in the body, is the loop really needed?
(I guess not. Then you can also use `@Test(expected=RuntimeException.class)`.)
##########
File path: src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java
##########
@@ -147,6 +153,75 @@ public void cryptoTest() throws Exception {
}
}
+ @Test
+ public void testNullTransform() throws Exception {
+ for (String transform : transformations) {
+ try {
+ final CryptoCipher cipher = getCipher(null);
+ Assert.fail("Expected RuntimeException via NullPointerException");
+ } catch (final RuntimeException e) {
+ }
+ }
+ }
+
+ @Test
+ public void testInvalidTransform() throws Exception {
+ for (String transform : transformations) {
+ try {
+ final CryptoCipher cipher = getCipher("AES/CBR/NoPadding/garbage/garbage");
+ Assert.fail("Expected RuntimeException via a InvocationTargetException");
+ } catch (final RuntimeException e) {
+ }
+ }
+ }
+
+ @Test
+ public void testInvalidKey() throws Exception {
+ for (String transform : transformations) {
+ try {
+ final CryptoCipher cipher = getCipher(transform);
+ Assert.assertNotNull(cipher);
+
+ final byte[] invalidKey = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06,
+ 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x11 };
+ cipher.init(OpenSsl.ENCRYPT_MODE, new SecretKeySpec(invalidKey, "AES"), new IvParameterSpec(IV));
+ Assert.fail("Expected InvalidKeyException");
+ } catch (final InvalidKeyException ike) {
+ }
+ }
+ }
+
+ //@Test(expected = InvalidAlgorithmParameterException.class, timeout = 120000)
Review comment:
delete
##########
File path: src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java
##########
@@ -147,6 +153,75 @@ public void cryptoTest() throws Exception {
}
}
+ @Test
+ public void testNullTransform() throws Exception {
+ for (String transform : transformations) {
+ try {
+ final CryptoCipher cipher = getCipher(null);
+ Assert.fail("Expected RuntimeException via NullPointerException");
+ } catch (final RuntimeException e) {
+ }
+ }
+ }
+
+ @Test
+ public void testInvalidTransform() throws Exception {
+ for (String transform : transformations) {
Review comment:
Same question.
##########
File path: src/test/java/org/apache/commons/crypto/utils/UtilsTest.java
##########
@@ -41,4 +42,13 @@ public void testSplitOmitEmptyLine() {
public void testSplitNull() {
Assert.assertEquals(Collections.<String> emptyList(), Utils.splitClassNames(null, ","));
}
+ @Test
Review comment:
nit: add blank line between methods
##########
File path: src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java
##########
@@ -147,6 +153,75 @@ public void cryptoTest() throws Exception {
}
}
+ @Test
+ public void testNullTransform() throws Exception {
+ for (String transform : transformations) {
+ try {
+ final CryptoCipher cipher = getCipher(null);
+ Assert.fail("Expected RuntimeException via NullPointerException");
+ } catch (final RuntimeException e) {
+ }
+ }
+ }
+
+ @Test
+ public void testInvalidTransform() throws Exception {
+ for (String transform : transformations) {
+ try {
+ final CryptoCipher cipher = getCipher("AES/CBR/NoPadding/garbage/garbage");
+ Assert.fail("Expected RuntimeException via a InvocationTargetException");
+ } catch (final RuntimeException e) {
+ }
+ }
+ }
+
+ @Test
+ public void testInvalidKey() throws Exception {
+ for (String transform : transformations) {
+ try {
+ final CryptoCipher cipher = getCipher(transform);
+ Assert.assertNotNull(cipher);
+
+ final byte[] invalidKey = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06,
+ 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x11 };
+ cipher.init(OpenSsl.ENCRYPT_MODE, new SecretKeySpec(invalidKey, "AES"), new IvParameterSpec(IV));
+ Assert.fail("Expected InvalidKeyException");
+ } catch (final InvalidKeyException ike) {
+ }
+ }
+ }
+
+ //@Test(expected = InvalidAlgorithmParameterException.class, timeout = 120000)
+ @Test
+ public void testInvalidIV() throws Exception {
+ for (String transform : transformations) {
+ try {
+ final CryptoCipher cipher = getCipher(transform);
+ Assert.assertNotNull(cipher);
+
+ final byte[] invalidIV = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06,
+ 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x11 };
+ cipher.init(OpenSsl.ENCRYPT_MODE, new SecretKeySpec(KEY, "AES"), new IvParameterSpec(invalidIV));
+ Assert.fail("Expected InvalidAlgorithmParameterException");
+ } catch (final InvalidAlgorithmParameterException iape) {
+ }
+ }
+ }
+
+ @Test
+ public void testInvalidIVClass() throws Exception {
+ for (String transform : transformations) {
+ try {
+ final CryptoCipher cipher = getCipher(transform);
Review comment:
Funky indentation.
##########
File path: src/test/java/org/apache/commons/crypto/cipher/OpenSslCipherTest.java
##########
@@ -136,6 +137,15 @@ public void testInvalidIV() throws Exception {
cipher.init(OpenSsl.ENCRYPT_MODE, KEY, new IvParameterSpec(invalidIV));
}
+ @Test(expected = InvalidAlgorithmParameterException.class, timeout = 120000)
+ public void testInvalidIVClass() throws Exception {
+ final OpenSsl cipher = OpenSsl.getInstance("AES/CTR/NoPadding");
+ Assert.assertNotNull(cipher);
+
+ cipher.init(OpenSsl.ENCRYPT_MODE, KEY, new GCMParameterSpec(IV.length, IV));
+ Assert.fail("Should have caught an InvalidAlgorithmParameterException");
Review comment:
Unnecessary (`@Test` tag covers it).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-crypto] geoffreyblake commented on a change in pull request #97: Additional unit tests for JNA, Cipher, Random, Utils testing error inputs
Posted by GitBox <gi...@apache.org>.
geoffreyblake commented on a change in pull request #97:
URL: https://github.com/apache/commons-crypto/pull/97#discussion_r413105247
##########
File path: src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java
##########
@@ -147,6 +153,75 @@ public void cryptoTest() throws Exception {
}
}
+ @Test
+ public void testNullTransform() throws Exception {
+ for (String transform : transformations) {
+ try {
+ final CryptoCipher cipher = getCipher(null);
+ Assert.fail("Expected RuntimeException via NullPointerException");
+ } catch (final RuntimeException e) {
+ }
+ }
+ }
+
+ @Test
+ public void testInvalidTransform() throws Exception {
+ for (String transform : transformations) {
+ try {
+ final CryptoCipher cipher = getCipher("AES/CBR/NoPadding/garbage/garbage");
+ Assert.fail("Expected RuntimeException via a InvocationTargetException");
+ } catch (final RuntimeException e) {
+ }
+ }
+ }
+
+ @Test
+ public void testInvalidKey() throws Exception {
+ for (String transform : transformations) {
+ try {
+ final CryptoCipher cipher = getCipher(transform);
+ Assert.assertNotNull(cipher);
+
+ final byte[] invalidKey = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06,
+ 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x11 };
+ cipher.init(OpenSsl.ENCRYPT_MODE, new SecretKeySpec(invalidKey, "AES"), new IvParameterSpec(IV));
+ Assert.fail("Expected InvalidKeyException");
+ } catch (final InvalidKeyException ike) {
+ }
+ }
+ }
+
+ //@Test(expected = InvalidAlgorithmParameterException.class, timeout = 120000)
+ @Test
+ public void testInvalidIV() throws Exception {
+ for (String transform : transformations) {
+ try {
+ final CryptoCipher cipher = getCipher(transform);
+ Assert.assertNotNull(cipher);
+
+ final byte[] invalidIV = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06,
+ 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x11 };
+ cipher.init(OpenSsl.ENCRYPT_MODE, new SecretKeySpec(KEY, "AES"), new IvParameterSpec(invalidIV));
+ Assert.fail("Expected InvalidAlgorithmParameterException");
+ } catch (final InvalidAlgorithmParameterException iape) {
+ }
+ }
+ }
+
+ @Test
+ public void testInvalidIVClass() throws Exception {
+ for (String transform : transformations) {
+ try {
+ final CryptoCipher cipher = getCipher(transform);
Review comment:
Fixed all the odd formatting, redundant code concerns.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-crypto] coveralls commented on issue #97: Additional unit tests for JNA, Cipher, Random, Utils testing error inputs
Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #97:
URL: https://github.com/apache/commons-crypto/pull/97#issuecomment-617451912
[](https://coveralls.io/builds/30241242)
Coverage increased (+9.7%) to 74.671% when pulling **62fca451b0f58f04433f830dc6e8c3c1f1f86eaa on geoffreyblake:random_units** into **47b625c2bd13a101e3a7ca0c003dd78e28535a4a on apache:master**.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-crypto] vanzin commented on a change in pull request #97: Additional unit tests for JNA, Cipher, Random, Utils testing error inputs
Posted by GitBox <gi...@apache.org>.
vanzin commented on a change in pull request #97:
URL: https://github.com/apache/commons-crypto/pull/97#discussion_r413413011
##########
File path: src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java
##########
@@ -147,6 +153,62 @@ public void cryptoTest() throws Exception {
}
}
+ @Test(expected = RuntimeException.class)
+ public void testNullTransform() throws Exception {
+ final CryptoCipher cipher = getCipher(null);
Review comment:
```suggestion
getCipher(null);
```
##########
File path: src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java
##########
@@ -147,6 +153,62 @@ public void cryptoTest() throws Exception {
}
}
+ @Test(expected = RuntimeException.class)
+ public void testNullTransform() throws Exception {
+ final CryptoCipher cipher = getCipher(null);
+ }
+
+ @Test(expected = RuntimeException.class)
+ public void testInvalidTransform() throws Exception {
+ final CryptoCipher cipher = getCipher("AES/CBR/NoPadding/garbage/garbage");
Review comment:
```suggestion
getCipher("AES/CBR/NoPadding/garbage/garbage");
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-crypto] geoffreyblake commented on a change in pull request #97: Additional unit tests for JNA, Cipher, Random, Utils testing error inputs
Posted by GitBox <gi...@apache.org>.
geoffreyblake commented on a change in pull request #97:
URL: https://github.com/apache/commons-crypto/pull/97#discussion_r413104706
##########
File path: src/test/java/org/apache/commons/crypto/utils/UtilsTest.java
##########
@@ -41,4 +42,13 @@ public void testSplitOmitEmptyLine() {
public void testSplitNull() {
Assert.assertEquals(Collections.<String> emptyList(), Utils.splitClassNames(null, ","));
}
+ @Test
Review comment:
Fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-crypto] vanzin commented on issue #97: Additional unit tests for JNA, Cipher, Random, Utils testing error inputs
Posted by GitBox <gi...@apache.org>.
vanzin commented on issue #97:
URL: https://github.com/apache/commons-crypto/pull/97#issuecomment-618100287
Hey I can commit my own suggestions! Hope you don't mind me doing that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-crypto] vanzin commented on pull request #97: Additional unit tests for JNA, Cipher, Random, Utils testing error inputs
Posted by GitBox <gi...@apache.org>.
vanzin commented on pull request #97:
URL: https://github.com/apache/commons-crypto/pull/97#issuecomment-618782959
Merging.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-crypto] coveralls edited a comment on issue #97: Additional unit tests for JNA, Cipher, Random, Utils testing error inputs
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #97:
URL: https://github.com/apache/commons-crypto/pull/97#issuecomment-617451912
[](https://coveralls.io/builds/30263030)
Coverage increased (+0.9%) to 76.919% when pulling **a20a1b8ce13e4fcc784d2e237e3768c626be1c23 on geoffreyblake:random_units** into **24897862c41a504a6987a65727669268019f6f2f on apache:master**.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org