On Fri, 21 Feb 2025 18:31:04 GMT, Weijun Wang <wei...@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
