On Thu, 10 Nov 2022 05:49:12 GMT, Xue-Lei Andrew Fan <xue...@openjdk.org> wrote:
>> An `EncryptedPrivateKeyInfo` object can be created with an uninitialized
>> `AlgorithmParameters`, but before you call `getEncoded` on it you need to
>> remember to initialize the params. This is unfortunate but since this is a
>> public API, I hesitate to make a change.
>>
>> Instead, this code change fixes the much more widely used internal class
>> `AlgorithmId` so that it cannot be created with an uninitialized
>> `AlgorithmParameters`. `EncryptedPrivateKeyInfo` now works with both
>> initialized and uninitialized params, and it's immutable.
>>
>> No intention to make `AlgorithmId` immutable this time. It has a child class
>> named `AlgIdDSA` which makes things complicated.
>
> src/java.base/share/classes/javax/crypto/EncryptedPrivateKeyInfo.java line
> 192:
>
>> 190: // create an AlgorithmId from it. This should usually be
>> true
>> 191: // since we already require its getEncoded() returning the
>> 192: // encoded bytes of it.
>
> It looks like the comment implies there is no need of the catch block. Did
> you want to add something like "But it may be not true if ..."? Or, is the
> comment belong to AlgorithmId?
This comment is for this class. The spec mentions `{@code
algParams.getEncoded()} should return` so one can say that it already implied
that the params has already been initialized. This is a little subtle and given
that real world developer might first create this subject with an uninitialized
params and then initialize it before calling `getEncoded`, I decide to support
this case as well. I'll think about how to describe it better.
> src/java.base/share/classes/javax/crypto/EncryptedPrivateKeyInfo.java line
> 199:
>
>> 197: if (tmp != null) {
>> 198: this.algid = tmp;
>> 199: this.params = null;
>
> There is a getAlgParameters() method in the class. Does it make sense to
> cache the params?
OK. Thanks.
> src/java.base/share/classes/javax/crypto/EncryptedPrivateKeyInfo.java line
> 437:
>
>> 435:
>> 436: @SuppressWarnings("fallthrough")
>> 437: private static PKCS8EncodedKeySpec pKCS8EncodingToSpec(byte[]
>> encodedKey)
>
> Does "pkcs8..." (will lowercase) look like a better name for the method?
OK.
-------------
PR: https://git.openjdk.org/jdk/pull/11067
