On Fri, 21 Feb 2025 16:57:32 GMT, Mikhail Yankelevich <d...@openjdk.org> wrote:
>> Changed shell files to be java tests: >> * ./validator/certreplace.sh >> * ./validator/samedn.sh > > 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. test/jdk/sun/security/validator/CertReplace.java line 64: > 62: * > 63: * @run main/othervm CertReplace samedn.jks samedn1.certs > 64: * @run main/othervm CertReplace samedn.jks samedn2.certs Must these tests run in `othervm`? They haven't changed any static VM properties. 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. 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. 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. test/jdk/sun/security/validator/CertReplace.java line 197: > 195: > 196: // 3. Remove user for cacerts > 197: keyStore.deleteEntry(userAlias); Call `keyStore.store` to write to the keystore. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23727#discussion_r1965994249 PR Review Comment: https://git.openjdk.org/jdk/pull/23727#discussion_r1966022164 PR Review Comment: https://git.openjdk.org/jdk/pull/23727#discussion_r1965999262 PR Review Comment: https://git.openjdk.org/jdk/pull/23727#discussion_r1966008483 PR Review Comment: https://git.openjdk.org/jdk/pull/23727#discussion_r1966005851 PR Review Comment: https://git.openjdk.org/jdk/pull/23727#discussion_r1966016971
