On Wed, 12 Aug 2026 16:43:56 GMT, Sean Mullan <[email protected]> wrote:
>> The early return handles the initial request that encounters the oversized >> `crl`, but I wonder whether the null check of `crl` in `getMatchingCRLs` is >> still needed for the cached path, as shown below: >> https://github.com/openjdk/jdk/blob/8b81f8fbb8428ccd137240efc465f580ac7eb62b/src/java.base/share/classes/sun/security/provider/certpath/URICertStore.java#L607-L611 >> After the oversized `crl` is set to null. Maybe a subsequent >> `engineGetCRLs(null)` within `CHECK_INTERVAL` takes the cached path and >> calls `getMatchingCRLs(null, null)`. >> >> With the original condition, this returns `Collections.singletonList(null)`, >> while the revised condition returns an empty collection. >> >> It seems that the original logic assumes that `crl` and `selector` cannot >> both be null. Are there any cases where this assumption might not hold? > > Let me take a closer look and get back to you. I feel like this is guarding > against a bug in the code. A null CRL should never really be passed into > `getMatchingCRLs`. I took another look and I see your point. The original condition wasn't logically correct, that is a null CRL should never be wrapped into a Collection. While we could probably restructure the code to not pass a null CRL to `getMatchingCRLs`, it probably isn't worth it if it is not fixing an existing issue. I'd wait for Jamil to run your latest changes thru our testing again. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32164#discussion_r3769439874
