On Tue, 6 May 2025 21:19:04 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> 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"?
ok.
This is talking about encryption, not encoding.
>> 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 ...".
ok
>> 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.
ok
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2082195327
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2082211758
PR Review Comment: https://git.openjdk.org/jdk/pull/17543#discussion_r2082370577
