RFR: 8298420: PEM API: Implementation (Preview) [v15]

[Weijun Wang]

On Mon, 5 May 2025 14:41:01 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> Anthony Scarpino has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 66 commits:
>> 
>>  - major code review comments update
>>  - Merge branch 'master' into pem
>>  - Merge branch 'master' into pem
>>  - javadoc updates
>>  - code review comments
>>  - merge with master
>>  - better comment and remove commented out code
>>  - Merge branch 'master' into pem
>>  - Merge branch 'pem-merge' into pem
>>  - merge
>>  - ... and 56 more: https://git.openjdk.org/jdk/compare/e2ae50d8...0c540327
>
> src/java.base/share/classes/java/security/PEMEncoder.java line 218:
> 
>> 216:      * encryption algorithm and a given password.
>> 217:      *
>> 218:      * <p> Only {@link PrivateKey} will be encrypted with this newly configured
> 
> s/will/objects will/
> 
> Also suggest saying "can be" instead of "will be".

Shall we say "be encrypted" or "be encoded"?

> src/java.base/share/classes/java/security/PEMEncoder.java line 220:
> 
>> 218:      * <p> Only {@link PrivateKey} will be encrypted with this newly configured
>> 219:      * instance.  Other {@link DEREncodable} classes that do not support
>> 220:      * encrypted PEM will cause encode() to throw an IllegalArgumentException.
> 
> This sentence sounds like it is possible for classes other than PrivateKey to support encrypted PEM. Is that true? If not, I would be more specific and just say objects other than PrivateKey.
> 
> Put code font around IllegalArgumentException.

Since this covers `encodeToString` as well I suggest rewriting to "Encoding other ... will throw ...".

> src/java.base/share/classes/java/security/PEMEncoder.java line 222:
> 
>> 220:      * encrypted PEM will cause encode() to throw an IllegalArgumentException.
>> 221:      *
>> 222:      * @implNote Default algorithm defined by Security Property {@code
> 
> First sentence is incomplete.

You might want to be more clear that this implementation uses the PBE cipher algorithm using default parameters from the most preferred provider. Otherwise, users might be clueless about what encryption options they can configure.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2076308529
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2076310733
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2076314921