On Mon, 23 Aug 2021 15:48:10 GMT, Jamil Nimeh <[email protected]> wrote:
>> This code change adds new methods to `DerInputStream` to easily and safely
>> read optional fields in a ASN.1 DER-encoded value. It also adds several
>> wrapping methods to `DerValue` to avoid unnecessary memory copying when
>> calling by an internal method.
>>
>> The new methods are applied to `OAEPParameters` to show out how they work. A
>> new regression test is added to show that not only the new methods are
>> simper but they are safer and more correct.
>
> src/java.base/share/classes/sun/security/util/DerInputStream.java line 353:
>
>> 351: */
>> 352: public boolean seeOptionalContextSpecific(int n) throws IOException
>> {
>> 353: return checkNextTag(t -> (t & 0x0c0) == 0x080 && (t & 0x01f) ==
>> n);
>
> Minor nit for clarity, the 0x080 could use the static DerValue.TAG_CONTEXT.
>
> On an unrelated note, it's a bummer that we don't have names for the typical
> masking constants 0x0c0 and 0x01f for getting at the tag class and number
> values (or static methods maybe). But those numeric literals are only in
> wide use in DerValue.java and maybe not worth changing since you don't touch
> that file. It might make that code a bit clearer, but it is just a nit.
In fact, I tried using DerValue.TAG_CONTEXT at the beginning but it's defined
as a byte so it's actually -128, and will never equal to any `t & 0x0c0`. Even
the `DerValue::isContextSpecific` uses 0x80 directly. (Unfortunately this is
not a static method otherwise I'll use it).
-------------
PR: https://git.openjdk.java.net/jdk/pull/5221
