On Wed, 16 Nov 2022 03:41:11 GMT, Weijun Wang <wei...@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.
>
> Weijun Wang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> comment and exception message
Did you consider changing `EncryptedPrivateKeyInfo(...,AlgorithmParameters)` to
throw `IllegalStateException` if the parameters were not initialized? I know
you said you were worried about changing the API, but it would be a cleaner
option. I wonder if there is really any code that is initializing the
parameters after creating the EPKI.
src/java.base/share/classes/sun/security/x509/AlgorithmId.java line 103:
> 101: * @param oid the identifier for the algorithm.
> 102: * @param algparams the associated algorithm parameters, can be null.
> 103: * @exception IllegalStateException if algparams is not initialized
add "or cannot be decoded"
src/java.base/share/classes/sun/security/x509/AlgorithmId.java line 493:
> 491: * @param algparams the associated algorithm parameters.
> 492: * @exception NoSuchAlgorithmException on error.
> 493: * @exception IllegalStateException if algparams is not initialized
Add "or cannot be decoded"
test/jdk/javax/crypto/EncryptedPrivateKeyInfo/GetAlgName.java line 64:
> 62: }
> 63:
> 64: AlgorithmParameters ap2 =
> AlgorithmParameters.getInstance(ap.getAlgorithm());
It would be useful to add a comment here that you are testing that an EPKI can
be created with an uninitialized AP.
test/jdk/sun/security/x509/AlgorithmId/Uninitialized.java line 35:
> 33: import java.security.AlgorithmParameters;
> 34:
> 35: public class Uninitialized {
Is this test necessary? It seems to be duplicating the additional test you
added to GetAlgName.java
-------------
PR: https://git.openjdk.org/jdk/pull/11067
