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