-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
8341927: Remove hardcoded SunJCE provider #21551
base: master
Are you sure you want to change the base?
Conversation
👋 Welcome back mdonovan! A progress list of the required criteria for merging this PR into |
❗ This change is not yet ready to be integrated. |
Webrevs
|
@@ -84,12 +84,14 @@ private int testDefault(int testnum) throws Exception { | |||
|
|||
private int testStringProvider(int testnum) throws Exception { | |||
// get an instance of JavaLoginConfig from SUN | |||
Configuration c = Configuration.getInstance(JAVA_CONFIG, null, "SUN"); | |||
Configuration c = Configuration.getInstance(JAVA_CONFIG, null, | |||
System.getProperty("test.provider.name","SUN")); |
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.
System.getProperty("test.provider.name","SUN")); | |
System.getProperty("test.provider.name", "SUN")); |
doTest(c, testnum++); | ||
|
||
// get an instance of JavaLoginConfig from SunRsaSign | ||
try { | ||
c = Configuration.getInstance(JAVA_CONFIG, null, "SunRsaSign"); | ||
c = Configuration.getInstance(JAVA_CONFIG, null, | ||
System.getProperty("test.provider.name","SunRsaSign")); |
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.
System.getProperty("test.provider.name","SunRsaSign")); | |
System.getProperty("test.provider.name", "SunRsaSign")); |
You are changing more than SunJCE providers, so the title of this bug should not be specific to SunJCE. Suggest: "Replace hardcoded security providers with new test.provider.name system property". Are there any cases where a test has more than one hardcoded provider? |
In this PR, I removed hard-coded security providers and replaced them with a system property, test.provider.name. If the property is not specified, the provider originally used in the test is used:
Cipher c = Cipher.getInstance("AES/GCM/NoPadding", System.getProperty("test.provider.name", "SunJCE"));
Progress
Issue
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/21551/head:pull/21551
$ git checkout pull/21551
Update a local copy of the PR:
$ git checkout pull/21551
$ git pull https://git.openjdk.org/jdk.git pull/21551/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 21551
View PR using the GUI difftool:
$ git pr show -t 21551
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/21551.diff
Webrev
Link to Webrev Comment