RFR: 8318240: [AIX] Cleaners.java test failure
When the test sets the sun.security.jgss.native and sun.security.nativegss.debug as true, the test fails on AIX due to missing Kerberos GSS API dynamic library` libgssapi_krb5.so` JBS Issue : [JDK-8318240](https://bugs.openjdk.org/browse/JDK-8318240) - Commit messages: - fix for Cleaners.java Changes: https://git.openjdk.org/jdk/pull/16212/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16212&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8318240 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16212.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16212/head:pull/16212 PR: https://git.openjdk.org/jdk/pull/16212
Re: RFR: 8318240: [AIX] Cleaners.java test failure
On Thu, 19 Oct 2023 11:53:30 GMT, Matthias Baesken wrote: >> When the test sets the sun.security.jgss.native and >> sun.security.nativegss.debug as true, the test fails on AIX due to missing >> Kerberos GSS API dynamic library` libgssapi_krb5.so` >> >> JBS Issue : [JDK-8318240](https://bugs.openjdk.org/browse/JDK-8318240) > > Marked as reviewed by mbaesken (Reviewer). Thank you @MBaesken & @ansteiner - PR Comment: https://git.openjdk.org/jdk/pull/16212#issuecomment-1775382239
Integrated: 8318240: [AIX] Cleaners.java test failure
On Tue, 17 Oct 2023 07:31:07 GMT, Varada M wrote: > When the test sets the sun.security.jgss.native and > sun.security.nativegss.debug as true, the test fails on AIX due to missing > Kerberos GSS API dynamic library` libgssapi_krb5.so` > > JBS Issue : [JDK-8318240](https://bugs.openjdk.org/browse/JDK-8318240) This pull request has now been integrated. Changeset: d4b76124 Author:Varada M Committer: Matthias Baesken URL: https://git.openjdk.org/jdk/commit/d4b761242d91aa1bcadc438cce0a9465c0f8b23d Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod 8318240: [AIX] Cleaners.java test failure Reviewed-by: mbaesken, asteiner - PR: https://git.openjdk.org/jdk/pull/16212
Re: RFR: 8358159: Empty mode/padding in cipher transformations [v2]
> Omitting the mode/padding in a transformation string eg: "AES/ /NoPadding" > throws NoSuchAlgorithmException. > This patch restores the behavior by ensuring that empty mode or padding > strings are interpreted as null. > > Testing done for : tier1 - fastdebug level (AIX) > > JBS: [JDK-8358159](https://bugs.openjdk.org/browse/JDK-8358159) Varada M has updated the pull request incrementally with one additional commit since the last revision: jtreg test case - Changes: - all: https://git.openjdk.org/jdk/pull/25547/files - new: https://git.openjdk.org/jdk/pull/25547/files/e7a35711..ccc650bb Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=25547&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=25547&range=00-01 Stats: 54 lines in 1 file changed: 54 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/25547.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/25547/head:pull/25547 PR: https://git.openjdk.org/jdk/pull/25547
Re: RFR: 8358159: Empty mode/padding in cipher transformations [v3]
> Omitting the mode/padding in a transformation string eg: "AES/ /NoPadding" > throws NoSuchAlgorithmException. > This patch restores the behavior by ensuring that empty mode or padding > strings are interpreted as null. > > Testing done for : tier1 - fastdebug level (AIX) > > JBS: [JDK-8358159](https://bugs.openjdk.org/browse/JDK-8358159) Varada M has updated the pull request incrementally with one additional commit since the last revision: whitespace error fix - Changes: - all: https://git.openjdk.org/jdk/pull/25547/files - new: https://git.openjdk.org/jdk/pull/25547/files/ccc650bb..b5854da0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=25547&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=25547&range=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/25547.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/25547/head:pull/25547 PR: https://git.openjdk.org/jdk/pull/25547
RFR: 8358159: empty mode/padding in cipher transformations.
Omitting the mode/padding in a transformation string eg: "AES/ /NoPadding" throws NoSuchAlgorithmException. This patch restores the behavior by ensuring that empty mode or padding strings are interpreted as null. Testing done for : tier1 - fastdebug level JBS: [JDK-8358159](https://bugs.openjdk.org/browse/JDK-8358159) - Commit messages: - JDK-8358159: empty mode/padding in cipher transformations. Changes: https://git.openjdk.org/jdk/pull/25547/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=25547&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8358159 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/25547.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/25547/head:pull/25547 PR: https://git.openjdk.org/jdk/pull/25547
Re: RFR: 8358159: Empty mode/padding in cipher transformations [v5]
> Omitting the mode/padding in a transformation string eg: "AES/ /NoPadding" > throws NoSuchAlgorithmException. > This patch restores the behavior by ensuring that empty mode or padding > strings are interpreted as null. > > Testing done for : tier1 - fastdebug level (AIX) > > JBS: [JDK-8358159](https://bugs.openjdk.org/browse/JDK-8358159) Varada M has updated the pull request incrementally with one additional commit since the last revision: 8358159: Empty mode/padding in cipher transformations - Changes: - all: https://git.openjdk.org/jdk/pull/25547/files - new: https://git.openjdk.org/jdk/pull/25547/files/4bc7e62d..413dea01 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=25547&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=25547&range=03-04 Stats: 7 lines in 1 file changed: 4 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/25547.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/25547/head:pull/25547 PR: https://git.openjdk.org/jdk/pull/25547
Re: RFR: 8358159: Empty mode/padding in cipher transformations [v4]
On Thu, 5 Jun 2025 21:20:58 GMT, Valerie Peng wrote:
>> Varada M has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8358159: Empty mode/padding in cipher transformations
>
> src/java.base/share/classes/javax/crypto/Cipher.java line 460:
>
>> 458: String pad = (parts[2].length() == 0 ? null : parts[2]);
>> 459:
>> 460: if ((mode == null || mode.isEmpty()) && (pad == null ||
>> pad.isEmpty())) {
>
> The `mode.isEmpty()` and `pad.isEmpty()` should always be `false` since line
> 457 and 458 already converted empty String to `null`, right? Looks redundant
> to me and can be removed.
You are right. I have updated accordingly. Thanks for pointing it out!
-
PR Review Comment: https://git.openjdk.org/jdk/pull/25547#discussion_r2134116721
Re: RFR: 8358159: Empty mode/padding in cipher transformations
On Fri, 30 May 2025 12:40:51 GMT, Sean Mullan wrote: >> Omitting the mode/padding in a transformation string eg: "AES/ /NoPadding" >> throws NoSuchAlgorithmException. >> This patch restores the behavior by ensuring that empty mode or padding >> strings are interpreted as null. >> >> Testing done for : tier1 - fastdebug level (AIX) >> >> JBS: [JDK-8358159](https://bugs.openjdk.org/browse/JDK-8358159) > > I think this should have a regression test for this change. Also, you > indicated this patch restores the behavior - do you know what release this > last worked in and what issue caused it to stop working? Hi @seanjmullan , This [previous commit](https://github.com/openjdk/jdk/pull/11339) in openjdk broke some cipher transformations. - PR Comment: https://git.openjdk.org/jdk/pull/25547#issuecomment-2930371743
Re: RFR: 8358159: Empty mode/padding in cipher transformations [v3]
On Wed, 4 Jun 2025 18:13:59 GMT, Valerie Peng wrote:
>> src/java.base/share/classes/javax/crypto/Cipher.java line 393:
>>
>>> 391: this.suffix = suffix.toUpperCase(Locale.ENGLISH);
>>> 392: this.mode = ((mode == null) || mode.isEmpty()) ? null :
>>> mode;
>>> 393: this.pad = ((pad == null) || pad.isEmpty()) ? null : pad;
>>
>> Thanks for reporting and fixing this issue.
>> Since this is an internal class used solely inside `Cipher` class, instead
>> of changing the empty string to null inside the `Transform` constuctor, we
>> can do that before calling `Transform` constructor. Also if one of `mode` or
>> `pad` is empty, then maybe we don't need all 4 `Transform`s.
>
> For example, line 457, 458, we can do something like:
>
> String mode = (parts[1].length() == 0 ? null : parts[1]);
> String pad = (parts[2].length() == 0 ? null : parts[2]);
>
> When populating the `list `(after line 467), we can skip the Tranform if the
> required component is missing, e.g.
>
> List list = new ArrayList<>(4);
> if ((mode != null) && (pad != null)) {
> list.add(new Transform(alg, "/" + mode + "/" + pad, null,
> null));
> }
Thank you @valeriepeng. I have made the suggested changes. Did the jtreg
testing too, it looks good.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/25547#discussion_r2128613087
Re: RFR: 8358159: Empty mode/padding in cipher transformations [v4]
> Omitting the mode/padding in a transformation string eg: "AES/ /NoPadding" > throws NoSuchAlgorithmException. > This patch restores the behavior by ensuring that empty mode or padding > strings are interpreted as null. > > Testing done for : tier1 - fastdebug level (AIX) > > JBS: [JDK-8358159](https://bugs.openjdk.org/browse/JDK-8358159) Varada M has updated the pull request incrementally with one additional commit since the last revision: 8358159: Empty mode/padding in cipher transformations - Changes: - all: https://git.openjdk.org/jdk/pull/25547/files - new: https://git.openjdk.org/jdk/pull/25547/files/b5854da0..4bc7e62d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=25547&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=25547&range=02-03 Stats: 11 lines in 2 files changed: 4 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/25547.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/25547/head:pull/25547 PR: https://git.openjdk.org/jdk/pull/25547
Integrated: 8358159: Empty mode/padding in cipher transformations
On Fri, 30 May 2025 12:06:21 GMT, Varada M wrote: > Omitting the mode/padding in a transformation string eg: "AES/ /NoPadding" > throws NoSuchAlgorithmException. > This patch restores the behavior by ensuring that empty mode or padding > strings are interpreted as null. > > Testing done for : tier1 - fastdebug level (AIX) > > JBS: [JDK-8358159](https://bugs.openjdk.org/browse/JDK-8358159) This pull request has now been integrated. Changeset: 3ff83ec4 Author:Varada M URL: https://git.openjdk.org/jdk/commit/3ff83ec49e561c44dd99508364b8ba068274b63a Stats: 68 lines in 2 files changed: 62 ins; 0 del; 6 mod 8358159: Empty mode/padding in cipher transformations Reviewed-by: amitkumar, valeriep - PR: https://git.openjdk.org/jdk/pull/25547
Re: RFR: 8358159: Empty mode/padding in cipher transformations [v5]
On Sat, 7 Jun 2025 20:07:45 GMT, Varada M wrote: >> Omitting the mode/padding in a transformation string eg: "AES/ /NoPadding" >> throws NoSuchAlgorithmException. >> This patch restores the behavior by ensuring that empty mode or padding >> strings are interpreted as null. >> >> Testing done for : tier1 - fastdebug level (AIX) >> >> JBS: [JDK-8358159](https://bugs.openjdk.org/browse/JDK-8358159) > > Varada M has updated the pull request incrementally with one additional > commit since the last revision: > > 8358159: Empty mode/padding in cipher transformations Thanks all - PR Comment: https://git.openjdk.org/jdk/pull/25547#issuecomment-2958124525
