RFR: 8313226: Redundant condition test in X509CRLImpl

John Jiang jjiang at openjdk.org
Tue Aug 1 15:52:49 UTC 2023


On Tue, 1 Aug 2023 15:22:48 GMT, Jamil Nimeh <jnimeh at openjdk.org> wrote:

>> if ((nextByte == DerValue.tag_SequenceOf)
>>         && (! ((nextByte & 0x0c0) == 0x080))) {
>>     ...
>>     ...
>> }
>> 
>> If `nextByte` is `DerValue.tag_SequenceOf`, exactly `0x30`, then the test after `&&` should always be true.
>
> The change itself looks fine to me since bits 8 and 7 will always be zero when `nextByte` is 0x30.  It looks like the second check was to see if the tag was a context-specific tag, but I don't know why since RFC 5280 doesn't indicate that it can be context specific.  I wonder if it is a remnant from an earlier version of the code.
> Regardless, the second clause isn't doing anything so the removal looks good to me.

@jnimeh 
Thanks very much for your review!

This code snippet has a looooong [history]. It is in the initial commit, and at least 15+ years old.

[history]:
<https://github.com/openjdk/jdk/blob/319a3b994703aac84df7bcde272adfcb3cdbbbf0/jdk/src/share/classes/sun/security/x509/X509CRLImpl.java#L1086>

-------------

PR Comment: https://git.openjdk.org/jdk/pull/15051#issuecomment-1660600286



More information about the security-dev mailing list