On Fri, 7 Jul 2023 19:21:29 GMT, Pavel Rappo <[email protected]> wrote:
> > Took another pass at this, looks good, but I would like to take another
> > last look and make sure that changing the hash code for some of the classes
> > like X509CRL is a benign change.
>
> Thanks, Sean. Take your time, you're an expert in this area. Meanwhile, I'll
> reflow other similar `{@return ... }` constructs that I missed before, for
> readability.
This comment is NOT to rush the review.
I once again note, that those classes in this PR that skip the first array
element in `hashCode`, do not seem to skip that same element in `equals`.
Although that does not breach equals-hashCode contract, it puzzles the reader
and theoretically makes `hashCode` more blunt (i.e. more instances may share a
hash code value).
This observation is an investigation opportunity. But equally, I'm okay with
skipping the investigation and reverting those particular changes, if
security-dev wants to be on the safe side. We might soon have a more flexible
way to compute hashCode [^*]. If we have it, we could then both refactor those
hashCode implementations and preserve their behaviour.
[^*]: https://github.com/openjdk/jdk/pull/14831
-------------
PR Comment: https://git.openjdk.org/jdk/pull/14738#issuecomment-1631404126
