On Wed, 5 Jul 2023 18:04:01 GMT, Roger Riggs <[email protected]> wrote:
>> Pavel Rappo has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains six additional
>> commits since the last revision:
>>
>> - Merge branch 'master' into 8311170
>> - Be consistent with the rest of the change
>> - Fix reported bugs
>> - Add even more cases and tidy up
>> - More cases
>> - Initial commit
>
> src/java.base/share/classes/javax/security/cert/Certificate.java line 107:
>
>> 105: try {
>> 106: return Arrays.hashCode(this.getEncoded());
>> 107: } catch (CertificateException e) {
>
> Ditto, what value at `certData[0]` is now included in the hash?
Those are correct observations. Unspecified hashCode methods changed in this
proposal might return different values than the ones they currently return in
the mainline; hence my disclaimer in this PR description:
> This PR sacrifices compatibility for pragmatism: it changes some hashCode
> implementations to produce different values than before to allow more
> utilization of methods from Objects and Arrays. To my mind, those changes are
> **benign**. If you disagree, I'd be happy to discuss that and/or retract the
> concerning part of the change.
I'm waiting from someone from security-dev to tell me if that's okay to skip
zeroth elements in both hashCode. It might've been an accidental skip in the
first place. FWIW, their `equals` counterparts do not skip zeroth elements. On
the other hand, javax.security.cert.Certificate is deprecated for removal, so I
might as well exclude it.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14738#discussion_r1253622642
