RFR: 8349533: Refactor validator tests shell files to java [v2]
Mikhail Yankelevich
duke at openjdk.org
Mon Feb 24 15:28:54 UTC 2025
On Fri, 21 Feb 2025 18:31:04 GMT, Weijun Wang <weijun at openjdk.org> wrote:
>> Mikhail Yankelevich has updated the pull request incrementally with one additional commit since the last revision:
>>
>> keyStore is not used to delete, cleanup of the calls, minor refactoring
>
> test/jdk/sun/security/validator/CertReplace.java line 28:
>
>> 26: */
>> 27:
>> 28: import java.io.FileInputStream;
>
> Remove the "This test is called by certreplace.sh" line above.
Changed in the next commit
> test/jdk/sun/security/validator/CertReplace.java line 82:
>
>> 80: final String intAliase = "int";
>> 81: final String userAlias = "user";
>> 82: final String caAlias = "ca";
>
> Do you really believe creating these variables help the program looking better? There are so many string concatenations in the keytool commands. If it were me, I would remove every variable for a string literal except for `ktBaseParameters`.
>
> On the other hand, it's OK to put `"changeit".toCharArray()` into a variable.
Changed in the next commit
> test/jdk/sun/security/validator/CertReplace.java line 120:
>
>> 118: new FileInputStream(intCertFileName)
>> 119: )
>> 120: };
>
> Use `CertUtils.getCertFromStream`. Also, it's better to close the file. Maybe Windows dislikes open files.
Done in the next commit
> test/jdk/sun/security/validator/CertReplace.java line 147:
>
>> 145: "-selfcert -alias " + caAlias);
>> 146: keyStore.deleteEntry(intAliase);
>> 147: keyStore.deleteEntry(userAlias);
>
> Call `keyStore.store` to actually remove the 2 entries inside the keystore file. But this needs to be done before the `-selfcert` call, otherwise, the old cert would overwrite the new one.
Done in the next commit
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23727#discussion_r1967869438
PR Review Comment: https://git.openjdk.org/jdk/pull/23727#discussion_r1967870760
PR Review Comment: https://git.openjdk.org/jdk/pull/23727#discussion_r1967872033
PR Review Comment: https://git.openjdk.org/jdk/pull/23727#discussion_r1967871506
More information about the security-dev
mailing list