On Mon, 16 Jun 2025 22:20:10 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> Based on the javadoc of `javax.crypto.Cipher` class, the cipher >> transformation should be either "algorithm/mode/padding" or >> "algorithm". When parsing the transformation, space(s) is trimmed off and >> empty strings are considered as "unspecified". This PR adds checks to ensure >> that transformations with empty "mode" and/or "padding" value in the >> "algorithm/mode/padding" form leads to `NoSuchAlgorithmException`. This >> reverts some changes made in >> [https://bugs.openjdk.org/browse/JDK-8358159](https://bugs.openjdk.org/browse/JDK-8358159) >> which allows empty mode and/or padding in the transformations. >> >> >> Thanks in advance for the review~ > > Valerie Peng has updated the pull request incrementally with one additional > commit since the last revision: > > Updated test per Mikhail's review comments. Thank you for your changes! Just a few super minor questions test/jdk/javax/crypto/Cipher/TestEmptyModePadding.java line 30: > 28: * @summary test that the Cipher.getInstance() would reject improper > 29: * transformations with empty mode and/or padding. > 30: * @run main TestEmptyModePadding minor: Is `@run` needed here? It's fine to leave it here, if you prefer it this way though. test/jdk/javax/crypto/Cipher/TestEmptyModePadding.java line 37: > 35: import javax.crypto.*; > 36: > 37: public class TestEmptyModePadding { Could you please change the imports to not use wildcard imports import java.security.NoSuchAlgorithmException; import java.security.Provider; import java.security.Security; import javax.crypto.Cipher; test/jdk/javax/crypto/Cipher/TestEmptyModePadding.java line 45: > 43: System.out.println("Testing against " + provider.getName()); > 44: > 45: String[] testTransformations = { minor: Do you think it would be easier to read if each entry was a separate line? test/jdk/javax/crypto/Cipher/TestEmptyModePadding.java line 69: > 67: for (String t : testTransformations) { > 68: test(t, provider); > 69: }; nit, `;` after for loop Suggestion: } ------------- PR Review: https://git.openjdk.org/jdk/pull/25808#pullrequestreview-2935056506 PR Review Comment: https://git.openjdk.org/jdk/pull/25808#discussion_r2151902251 PR Review Comment: https://git.openjdk.org/jdk/pull/25808#discussion_r2151902337 PR Review Comment: https://git.openjdk.org/jdk/pull/25808#discussion_r2151906412 PR Review Comment: https://git.openjdk.org/jdk/pull/25808#discussion_r2151894079
