On Mon, 14 Nov 2022 21:47:38 GMT, Valerie Peng <[email protected]> wrote:
>> Weijun Wang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> make class package private
>
> src/java.base/share/classes/sun/security/pkcs/PKCS9Attribute.java line 381:
>
>> 379: index = indexOf(oid, PKCS9_OIDS, 1);
>> 380: Class<?> clazz = index == -1 ? BYTE_ARRAY_CLASS:
>> VALUE_CLASSES[index];
>> 381: if (clazz == null || !clazz.isInstance(value)) {
>
> If my reading of the current impl is correct, if clazz is null, the attribute
> is not supported. The error message seems a bit misleading as it's not really
> due to the value itself, but the attribute is not supported. Is it just for
> avoiding NPE and changing it into IAE?
Yes, just to avoid NPE. I can see the message about type is null looks strange.
I'll change it to something normal.
> test/jdk/sun/security/pkcs/pkcs9/PKCS9AttrTypeTests.java line 176:
>
>> 174: // Encoding is supported
>> 175: DerOutputStream dos = new DerOutputStream();
>> 176: p9Attr.encode(dos);
>
> Should we check the encoding has the expected value? Otherwise, it looks like
> we only require that no exception is thrown?
You mean comparing it with the original value? I tried that but not always the
same. For example, a string might be an IA5String at the beginning but becomes
a UTF8String after re-encoding.
-------------
PR: https://git.openjdk.org/jdk/pull/11070
