RFR: 8359388: Stricter checking for cipher transformations

[Valerie Peng]

On Mon, 16 Jun 2025 13:49:38 GMT, Mikhail Yankelevich <myankelevich at 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~
>
> src/java.base/share/classes/javax/crypto/Cipher.java line 348:
> 
>> 346:                 transformation.substring(0, endIdx).trim());
>> 347:         if (algo.isEmpty()) {
>> 348:             throw new NoSuchAlgorithmException("Invalid transformation: " +
> 
> I don't think this exception is tested. It could be by adding `test(" ", provider);` in [test/jdk/javax/crypto/Cipher/TestEmptyModePadding.java](https://github.com/openjdk/jdk/pull/25808/files#diff-5442ba4fbe85af64a99ca3c2d2b5795026be4c40e23af8337d52a64cefd6af35) on line 68

Sure, I can add it.

> test/jdk/javax/crypto/Cipher/TestEmptyModePadding.java line 75:
> 
>> 73:         try {
>> 74:             Cipher c = Cipher.getInstance(transformation, provider);
>> 75:             throw new RuntimeException("Expected NSAE not thrown");
> 
> Minor: Do you think it would be a good idea to print out `transformation` and `provider` data for debug? In `System.err` that is. Otherwise afaik the exception is thrown and no trace in system error, just in `System.out.println`, making debugging quite challenging 
> 
> Just something like this 
> ```suggestion 
>             System.err.println("Error while testing " + transformation);
>             throw new RuntimeException("Expected NSAE not thrown");
> 
> or just add it to the `RuntimeException` message

Ok, I will update the test to include the `transformation` info in the exception message. As for `provider`, it's somewhat related, but given that it stays the same for the test, I printed it out just once in the beginning of the test.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25808#discussion_r2150929140
PR Review Comment: https://git.openjdk.org/jdk/pull/25808#discussion_r2150927529