-
Notifications
You must be signed in to change notification settings - Fork 123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for standard test vectors. #127
Conversation
Now handles standard test vectors when the ZIP containing them is pointed to by the testVectorZip system property. Example: `mvn install -Dgpg.skip=true '-DtestVectorZip=https://github.com/awslabs/aws-encryption-sdk-test-vectors/raw/master/vectors/awses-decrypt/python-1.3.8.zip'` This also adds them to our standard travis build.
import java.util.zip.ZipEntry; | ||
|
||
@RunWith(Parameterized.class) | ||
public class NewCompatTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason the class name is abbreviated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just was matching the pattern of the existing XCompatDecryptTest
. I'll rename it. You're right.
byte[] plaintext = crypto.decryptData(testCase.mkp, cachedData.get(testCase.ciphertextPath)).getResult(); | ||
final byte[] expectedPlaintext = cachedData.get(testCase.plaintextPath); | ||
|
||
Assert.assertArrayEquals(expectedPlaintext, plaintext); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to always statically import Assert methods for readability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I generally do if they're used more than once. As there is only a single use of this method, I (mildly) prefer to leave it fully-qualified.
} | ||
} | ||
|
||
@SuppressWarnings("unchecked") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this suppression is necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needed by a prior iteration of this method. Removed.
} | ||
|
||
@SuppressWarnings("unchecked") | ||
private static Map<String, KeyEntry> parseKeyManifest(final Map<String, Object> keysManifest) throws IOException, GeneralSecurityException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IOException isn't thrown in this method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
final JarURLConnection jarConnection = (JarURLConnection) new URL("jar:" + zipPath + "!/").openConnection(); | ||
|
||
try (JarFile jar = jarConnection.getJarFile()) { | ||
final ObjectMapper mapper = new ObjectMapper(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see mapper being used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needed by a prior version of this method. Removed.
Assert.assertArrayEquals(expectedPlaintext, plaintext); | ||
} | ||
|
||
@Parameterized.Parameters(name="Compatability Test: {0}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: "Compatibility"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I can never spell this word on my first try.
Description of changes:
Now handles standard test vectors when the ZIP containing them is
pointed to by the testVectorZip system property.
Example:
mvn install -Dgpg.skip=true '-DtestVectorZip=https://github.com/awslabs/aws-encryption-sdk-test-vectors/raw/master/vectors/awses-decrypt/python-1.3.8.zip'
This also adds them to our standard travis build.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: