geoffreyblake commented on a change in pull request #99:
URL: https://github.com/apache/commons-crypto/pull/99#discussion_r416643423
##########
File path:
src/test/java/org/apache/commons/crypto/stream/AbstractCipherStreamTest.java
##########
@@ -197,46 +224,236 @@ protected void doByteBufferRead(final String
cipherClass, final boolean withChan
getCipher(cipherClass), smallBufferSize, iv, withChannel);
buf.clear();
byteBufferReadCheck(in, buf, 11);
+ in.close();
+
+ // Direct buffer, default buffer size, initial buffer position is 0,
final read
+ in = getCryptoInputStream(new ByteArrayInputStream(encData),
+ getCipher(cipherClass), smallBufferSize, iv, withChannel);
+ buf.clear();
+ byteBufferFinalReadCheck(in, buf, 0);
+ in.close();
+
+ // Default buffer size, initial buffer position is 0, insufficient
dest buffer length
+ in = getCryptoInputStream(new ByteArrayInputStream(encData),
+ getCipher(cipherClass), defaultBufferSize, iv, withChannel);
+ buf = ByteBuffer.allocate(100);
+ byteBufferReadCheck(in, buf, 0);
+ in.close();
+
+ // Default buffer size, initial buffer position is 0
+ in = getCryptoInputStream(transformation, props, new
ByteArrayInputStream(encData), key,
+ new IvParameterSpec(iv), withChannel);
+ buf = ByteBuffer.allocate(dataLen + 100);
+ byteBufferReadCheck(in, buf, 0);
+ in.close();
+
+ // Default buffer size, initial buffer position is not 0
+ in = getCryptoInputStream(transformation, props, new
ByteArrayInputStream(encData), key,
+ new IvParameterSpec(iv), withChannel);
+ buf.clear();
+ byteBufferReadCheck(in, buf, 11);
+ in.close();
+
+ // Small buffer size, initial buffer position is 0
+ in = getCryptoInputStream(transformation, props, new
ByteArrayInputStream(encData), key,
+ new IvParameterSpec(iv), withChannel);
+ buf.clear();
+ byteBufferReadCheck(in, buf, 0);
+ in.close();
+
+ // Small buffer size, initial buffer position is not 0
+ in = getCryptoInputStream(transformation, props, new
ByteArrayInputStream(encData), key,
+ new IvParameterSpec(iv), withChannel);
+ buf.clear();
+ byteBufferReadCheck(in, buf, 11);
+ in.close();
+
+ // Direct buffer, default buffer size, initial buffer position is 0
+ in = getCryptoInputStream(transformation, props, new
ByteArrayInputStream(encData), key,
+ new IvParameterSpec(iv), withChannel);
+ buf = ByteBuffer.allocateDirect(dataLen + 100);
+ byteBufferReadCheck(in, buf, 0);
+ in.close();
+
+ // Direct buffer, default buffer size, initial buffer position is not 0
+ in = getCryptoInputStream(transformation, props, new
ByteArrayInputStream(encData), key,
+ new IvParameterSpec(iv), withChannel);
+ buf.clear();
+ byteBufferReadCheck(in, buf, 11);
+ in.close();
+
+ // Direct buffer, small buffer size, initial buffer position is 0
+ in = getCryptoInputStream(transformation, props, new
ByteArrayInputStream(encData), key,
+ new IvParameterSpec(iv), withChannel);
+ buf.clear();
+ byteBufferReadCheck(in, buf, 0);
+ in.close();
+
+ // Direct buffer, small buffer size, initial buffer position is not 0
+ in = getCryptoInputStream(transformation, props, new
ByteArrayInputStream(encData), key,
+ new IvParameterSpec(iv), withChannel);
+ buf.clear();
+ byteBufferReadCheck(in, buf, 11);
+ in.close();
+
+ // Direct buffer, default buffer size, initial buffer position is 0,
final read
+ in = getCryptoInputStream(transformation, props, new
ByteArrayInputStream(encData), key,
+ new IvParameterSpec(iv), withChannel);
+ buf.clear();
+ byteBufferFinalReadCheck(in, buf, 0);
+ in.close();
+
+ // Default buffer size, initial buffer position is 0, insufficient
dest buffer length
+ in = getCryptoInputStream(transformation, props, new
ByteArrayInputStream(encData), key,
+ new IvParameterSpec(iv), withChannel);
+ buf = ByteBuffer.allocate(100);
+ byteBufferReadCheck(in, buf, 0);
in.close();
}
protected void doByteBufferWrite(final String cipherClass,
- final ByteArrayOutputStream baos, final boolean withChannel)
throws Exception {
+ final ByteArrayOutputStream baos, final boolean withChannel)
+ throws Exception {
if (AbstractCipherTest.OPENSSL_CIPHER_CLASSNAME.equals(cipherClass)) {
if (!Crypto.isNativeCodeLoaded()) {
return; // Skip this test if no JNI
}
}
baos.reset();
- final CryptoOutputStream out = getCryptoOutputStream(baos,
+ CryptoOutputStream out = getCryptoOutputStream(baos,
getCipher(cipherClass), defaultBufferSize, iv, withChannel);
- ByteBuffer buf = ByteBuffer.allocateDirect(dataLen / 2);
- buf.put(data, 0, dataLen / 2);
- buf.flip();
- final int n1 = out.write(buf);
-
- buf.clear();
- buf.put(data, n1, dataLen / 3);
- buf.flip();
- final int n2 = out.write(buf);
-
- buf.clear();
- buf.put(data, n1 + n2, dataLen - n1 - n2);
- buf.flip();
- final int n3 = out.write(buf);
+ doByteBufferWrite(out, withChannel);
+
+ baos.reset();
+ CryptoCipher cipher = getCipher(cipherClass);
+ String transformation = cipher.getAlgorithm();
+ out = getCryptoOutputStream(transformation, props, baos, key,
+ new IvParameterSpec(iv), withChannel);
+ doByteBufferWrite(out, withChannel);
+ out.write(1);
+ Assert.assertTrue(out.isOpen());
+
+ out = getCryptoOutputStream(transformation, props, baos, key,
+ new IvParameterSpec(iv), withChannel);
+ out.close();
+ Assert.assertTrue(!out.isOpen());
+ }
- Assert.assertEquals(dataLen, n1 + n2 + n3);
+ protected void doExceptionTest(final String cipherClass,
ByteArrayOutputStream baos,
+ final boolean withChannel) throws IOException {
+ if (AbstractCipherTest.OPENSSL_CIPHER_CLASSNAME.equals(cipherClass)) {
+ if (!Crypto.isNativeCodeLoaded()) {
+ return; // Skip this test if no JNI
+ }
+ }
+
+ InputStream in = null;
+ OutputStream out = null;
+ try { // Test InvalidAlgorithmParameters
+ in = getCryptoInputStream(transformation, props, new
ByteArrayInputStream(encData),
+ new SecretKeySpec(key, "AES"), new GCMParameterSpec(0, new
byte[0]),
+ withChannel);
+ Assert.fail("Expected IOException.");
+ } catch (IOException ex) {
+ Assert.assertEquals(ex.getMessage(),"Illegal parameters");
+ }
+
+ try { // Test InvalidAlgorithmParameters
+ out = getCryptoOutputStream(transformation, props, baos,
+ new SecretKeySpec(key, "AES"), new GCMParameterSpec(0,
+ new byte[0]), withChannel);
+ Assert.fail("Expected IOException.");
+ } catch (IOException ex) {
+ Assert.assertEquals(ex.getMessage(),"Illegal parameters");
+ }
+
+ try { // Test Invalid Key
+ in = getCryptoInputStream(transformation,props, new
ByteArrayInputStream(encData),
+ new SecretKeySpec(new byte[10], "AES"), new
IvParameterSpec(iv), withChannel);
+ Assert.fail("Expected IOException for Invalid Key");
+ } catch (IOException ex) {
+ Assert.assertNotNull(ex);
+ }
+
+ try { // Test Invalid Key
+ out = getCryptoOutputStream(transformation, props, baos, new
byte[10],
+ new IvParameterSpec(iv), withChannel);
+ Assert.fail("Expected IOException for Invalid Key");
+ } catch (IOException ex) {
+ Assert.assertNotNull(ex);
+ }
+
+ try { // Test closed stream.
+ in = getCryptoInputStream(new ByteArrayInputStream(encData),
+ getCipher(cipherClass), defaultBufferSize, iv,
withChannel);
+ in.close();
+ in.close(); // Don't throw exception.
+ in.read(); // Throw exception.
Review comment:
I see. Then should we split this test into 2 try blocks? If someone
removes that path you're testing with the 2nd close, the test will still pass
because you'd catch an IOException from the 2nd close failing instead of from
the read() call.
To be honest, I'd expect a double close() to throw and exception, but
obviously not in this case.
##########
File path:
src/test/java/org/apache/commons/crypto/stream/CtrCryptoStreamTest.java
##########
@@ -52,4 +75,115 @@ protected CtrCryptoOutputStream getCryptoOutputStream(
}
return new CtrCryptoOutputStream(baos, cipher, bufferSize, key, iv);
}
+
+ @Override
+ protected CtrCryptoOutputStream getCryptoOutputStream(final String
transformation,
+ final Properties props, final ByteArrayOutputStream baos, final
byte[] key,
+ final AlgorithmParameterSpec params, final boolean withChannel)
throws IOException {
+ if (withChannel) {
+ return new CtrCryptoOutputStream(props, Channels.newChannel(baos),
key,
+ ((IvParameterSpec)params).getIV());
+ }
+ return new CtrCryptoOutputStream(props, baos,
key,((IvParameterSpec)params).getIV());
+ }
+
+ @Override
+ protected void doFieldGetterTest(final String cipherClass, final
ByteArrayOutputStream baos,
+ final boolean withChannel) throws Exception {
+ if (AbstractCipherTest.OPENSSL_CIPHER_CLASSNAME.equals(cipherClass)) {
+ if (!Crypto.isNativeCodeLoaded()) {
+ return; // Skip this test if no JNI
+ }
+ }
+
+ StreamInput streamInput = new StreamInput(new
ByteArrayInputStream(encData), 0);
+ try {
+ streamInput.seek(0);
+ Assert.fail("Expected UnsupportedOperationException.");
+ } catch (UnsupportedOperationException ex) {
+ Assert.assertEquals(ex.getMessage(), "Seek is not supported by
this implementation");
Review comment:
I agree it helps with readability. My reservation is this is a brittle
test if the generation method changes in the future, all these tests have to be
changed.
----------------------------------------------------------------
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:
[email protected]
