On Thu, 16 Oct 2025 12:56:17 GMT, Sean Mullan <[email protected]> wrote:
>> Mark Powers has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> remaining comments
>
> src/java.base/share/classes/sun/security/pkcs12/PBMAC1Parameters.java line
> 121:
>
>> 119: String kdfHmac, String hmac, byte[] digest) throws
>> NoSuchAlgorithmException {
>> 120: if (algName.equals("PBMAC1")) {
>> 121: return new DerOutputStream().write(DerValue.tag_Sequence,
>> new DerOutputStream()
>
> This use of method chaining is compact, but I find it much harder to review.
> Consider breaking it up into something more readable. Also, some of the lines
> are quite long.
Oh, I suggested this format so it maps almost line to line with the actual
encoding:
SEQUENCE
SEQUENCE
SEQUENCE
OID 1.2.840.113549.1.5.14 (PBMAC1)
SEQUENCE
SEQUENCE
OID 1.2.840.113549.1.5.12 (PBKDF2WithHmacSHA1)
SEQUENCE
OCTET STRING (20 bytes)
INTEGER 10000
INTEGER 20
SEQUENCE
OID 1.2.840.113549.2.7 (HmacSHA1)
NULL
SEQUENCE
OID 1.2.840.113549.2.7 (HmacSHA1)
NULL
OCTET STRING (20 bytes)
OCTET STRING (523127819, 8 bytes)
INTEGER 1
But yes, the single large expression does not align well with the multi-level
ASN.1 grammar at the top of this file. I'm fine with breaking it up to better
align with that structure.
BTW, the else block is not about PBMAC1. Maybe we should move it back into
`MacData`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2435926887
