On Fri, 7 Jul 2023 19:21:29 GMT, Pavel Rappo <pra...@openjdk.org> 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