On Mon, 14 Nov 2022 21:47:38 GMT, Valerie Peng <valer...@openjdk.org> 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