On Tue, 8 Aug 2023 16:17:44 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
On Tue, 8 Aug 2023 16:17:44 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
On Tue, 8 Aug 2023 16:17:44 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
> Please review this PR to use modern APIs and language features to simplify
> `equals` and `hashCode` in security area.
>
> I understand that security area is sensitive and a non-expert, such as
> myself, should tread carefully; so below are my notes to assist the review.
>
> * Unlike `hashCod
On Tue, 8 Aug 2023 11:54:42 GMT, Pavel Rappo wrote:
>> src/java.security.jgss/share/classes/sun/security/krb5/internal/HostAddress.java
>> line 105:
>>
>>> 103: return false;
>>> 104: if (address != null && h.address != null) {
>>> 105: return Arrays.equals(addre
> Please review this PR to use modern APIs and language features to simplify
> `equals` and `hashCode` in security area.
>
> I understand that security area is sensitive and a non-expert, such as
> myself, should tread carefully; so below are my notes to assist the review.
>
> * Unlike `hashCod
On Fri, 4 Aug 2023 21:30:00 GMT, Valerie Peng wrote:
>> Pavel Rappo has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Feedback
>
> src/java.security.jgss/share/classes/sun/security/krb5/internal/HostAddress.java
> line 105:
>
>> 103:
On Fri, 4 Aug 2023 22:00:41 GMT, Pavel Rappo wrote:
>> src/java.security.jgss/share/classes/sun/security/krb5/internal/KRBError.java
>> line 523:
>>
>>> 521: if (crealm != null) result = 37 * result + crealm.hashCode();
>>> 522: if (cname != null) result = 37 * result + cname.ha
On Fri, 4 Aug 2023 15:11:47 GMT, Pavel Rappo wrote:
>> src/java.base/share/classes/sun/security/provider/certpath/CertId.java line
>> 182:
>>
>>> 180: myhash += Arrays.hashCode(issuerNameHash);
>>> 181: myhash += Arrays.hashCode(issuerKeyHash);
>>> 182: myhas
On Fri, 4 Aug 2023 21:40:37 GMT, Valerie Peng wrote:
> Why changing the ordering?
The order was changed to be parallel with that of `equals`.
> I was wondering if you'd change this to Objects.hash(...) with all the fields
> as arguments.
Sure, we could use a convenience method, but because `e
On Tue, 1 Aug 2023 08:57:56 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
On Tue, 1 Aug 2023 08:57:56 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
On Fri, 4 Aug 2023 19:54:58 GMT, Sean Mullan wrote:
> I would add to that list that I think it is really important any change to
> the `hashCode` impl doesn't break the equals/hashCode contract, so it's
> important to look at the `equals` implementation in conjunction with any
> changes to `ha
On Fri, 4 Aug 2023 19:45:34 GMT, Sean Mullan wrote:
>> Pavel Rappo has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Feedback
>
> src/java.base/share/classes/com/sun/crypto/provider/DESKey.java line 113:
>
>> 111: public int hashCode(
On Fri, 4 Aug 2023 15:01:51 GMT, Pavel Rappo wrote:
>> src/java.base/share/classes/sun/security/x509/PolicyInformation.java line
>> 140:
>>
>>> 138: @Override
>>> 139: public int hashCode() {
>>> 140: return Objects.hash(policyIdentifier, policyQualifiers);
>>
>> What is the cr
On Tue, 1 Aug 2023 08:57:56 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
On Fri, 4 Aug 2023 00:04:03 GMT, Valerie Peng wrote:
>> Pavel Rappo has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Feedback
>
> src/java.base/share/classes/sun/security/provider/certpath/CertId.java line
> 182:
>
>> 180: m
On Thu, 3 Aug 2023 21:43:44 GMT, Valerie Peng wrote:
>> Pavel Rappo has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Feedback
>
> src/java.base/share/classes/sun/security/x509/PolicyInformation.java line 140:
>
>> 138: @Override
>> 1
On Fri, 4 Aug 2023 09:35:54 GMT, Pavel Rappo wrote:
>> src/java.base/share/classes/com/sun/crypto/provider/PBKDF2KeyImpl.java line
>> 272:
>>
>>> 270: }
>>> 271:
>>> 272: if
>>> (!(that.getAlgorithm().equalsIgnoreCase(getAlgorithm({
>>
>> why removing the space be
On Thu, 3 Aug 2023 22:49:46 GMT, Valerie Peng wrote:
>> Pavel Rappo has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Feedback
>
> src/java.base/share/classes/com/sun/crypto/provider/PBKDF2KeyImpl.java line
> 272:
>
>> 270: }
On Tue, 1 Aug 2023 08:57:56 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
On Tue, 1 Aug 2023 08:57:56 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
On Tue, 1 Aug 2023 08:57:56 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
> Please review this PR to use modern APIs and language features to simplify
> `equals` and `hashCode` in security area.
>
> I understand that security area is sensitive and a non-expert, such as
> myself, should tread carefully; so below are my notes to assist the review.
>
> * Unlike `hashCod
On Fri, 28 Jul 2023 22:14:09 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so bel
> Please review this PR to use modern APIs and language features to simplify
> `equals` and `hashCode` in security area.
>
> I understand that security area is sensitive and a non-expert, such as
> myself, should tread carefully; so below are my notes to assist the review.
>
> * Unlike `hashCod
On Mon, 17 Jul 2023 23:03:45 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so bel
On Mon, 17 Jul 2023 18:39:37 GMT, Daniel Jeliński wrote:
> I think `oid.hashCode() + Arrays.hashCode(nameValue)` would have been good
> enough, but deepHashCode is also acceptable.
Fixed as you suggested, in 3139043370f.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14738#
> Please review this PR to use modern APIs and language features to simplify
> `equals` and `hashCode` in security area.
>
> I understand that security area is sensitive and a non-expert, such as
> myself, should tread carefully; so below are my notes to assist the review.
>
> * Unlike `hashCod
On Mon, 17 Jul 2023 18:13:38 GMT, Pavel Rappo wrote:
>> Thanks for noticing this difference!
>>
>> Yes, the initial value would've been exponentiated (^ or ** in your
>> notation). However, I note that the original code computed this:
>>
>> if nameValue.length > 0
>>
>> (37^nameValue.leng
On Mon, 17 Jul 2023 12:23:17 GMT, Pavel Rappo wrote:
>> The original code calculated `(37 **
>> nameValue.length)*(37+oid.hashCode)+Arrays.hashCode`; since you already
>> dropped the multiplication, you can also drop the addition.
>
> Thanks for noticing this difference!
>
> Yes, the initial v
On Mon, 17 Jul 2023 11:02:46 GMT, Daniel Jeliński wrote:
>> You think it was a typo (+ instead of *) in the original code?
>
> The original code calculated `(37 **
> nameValue.length)*(37+oid.hashCode)+Arrays.hashCode`; since you already
> dropped the multiplication, you can also drop the addit
On Mon, 17 Jul 2023 10:59:42 GMT, Pavel Rappo wrote:
>> src/java.base/share/classes/sun/security/x509/OtherName.java line 212:
>>
>>> 210: public int hashCode() {
>>> 211: if (myhash == -1) {
>>> 212: myhash = 37 + oid.hashCode() + Arrays.hashCode(nameValue);
>>
>> I thi
On Thu, 13 Jul 2023 22:57:49 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so bel
On Mon, 17 Jul 2023 10:15:18 GMT, Daniel Jeliński wrote:
>> Pavel Rappo has updated the pull request incrementally with five additional
>> commits since the last revision:
>>
>> - Feedback: avoid intermediate assignments
>> - More previously missed cases
>> - Fix: log hashCode as an unsigned
On Thu, 13 Jul 2023 22:57:49 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so bel
On Thu, 13 Jul 2023 08:51:23 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so bel
> Please review this PR to use modern APIs and language features to simplify
> `equals` and `hashCode` in security area.
>
> I understand that security area is sensitive and a non-expert, such as
> myself, should tread carefully; so below are my notes to assist the review.
>
> * Unlike `hashCod
On Thu, 13 Jul 2023 08:51:23 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so bel
On Fri, 7 Jul 2023 23:19:27 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
> Please review this PR to use modern APIs and language features to simplify
> `equals` and `hashCode` in security area.
>
> I understand that security area is sensitive and a non-expert, such as
> myself, should tread carefully; so below are my notes to assist the review.
>
> * Unlike `hashCod
On Fri, 7 Jul 2023 23:19:27 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
On Fri, 7 Jul 2023 19:21:29 GMT, Pavel Rappo 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 exper
> Please review this PR to use modern APIs and language features to simplify
> `equals` and `hashCode` in security area.
>
> I understand that security area is sensitive and a non-expert, such as
> myself, should tread carefully; so below are my notes to assist the review.
>
> * Unlike `hashCod
On Fri, 7 Jul 2023 19:19:03 GMT, Sean Mullan 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
On Thu, 6 Jul 2023 19:10:14 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
On Thu, 6 Jul 2023 19:10:14 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
> Please review this PR to use modern APIs and language features to simplify
> `equals` and `hashCode` in security area.
>
> I understand that security area is sensitive and a non-expert, such as
> myself, should tread carefully; so below are my notes to assist the review.
>
> * Unlike `hashCod
> Please review this PR to use modern APIs and language features to simplify
> `equals` and `hashCode` in security area.
>
> I understand that security area is sensitive and a non-expert, such as
> myself, should tread carefully; so below are my notes to assist the review.
>
> * Unlike `hashCod
On Thu, 6 Jul 2023 17:06:31 GMT, Sean Mullan wrote:
>> src/java.base/share/classes/sun/security/x509/X500Name.java line 422:
>>
>>> 420: // quick check that number of RDNs and AVAs match before
>>> canonicalizing
>>> 421: if (!Arrays.equals(this.names, other.names,
>>> 422:
On Thu, 6 Jul 2023 16:55:12 GMT, Jonathan Gibbons wrote:
>> src/java.base/share/classes/java/security/cert/CertPath.java line 191:
>>
>>> 189:
>>> 190: /**
>>> 191: * {@return the hashcode value for this certification path} The
>>> hash code
>>
>> This looks a bit odd w/o a period at
On Wed, 5 Jul 2023 17:53:55 GMT, Roger Riggs 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 sin
On Wed, 5 Jul 2023 18:18:02 GMT, Roger Riggs 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 sin
On Thu, 6 Jul 2023 16:41:37 GMT, Sean Mullan 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 sin
On Wed, 5 Jul 2023 14:52:22 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
On Thu, 6 Jul 2023 16:25:05 GMT, Sean Mullan 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 sin
On Wed, 5 Jul 2023 14:52:22 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
On Wed, 5 Jul 2023 20:52:56 GMT, Pavel Rappo wrote:
>> Are you sure? I just checked lines 91-92 and I'd say the change looks
>> correct.
>
>> The original `<=` was correct, the number of bits in the input array must be
>> less than the requested length of the BitArray. The constructors also
>>
On Wed, 5 Jul 2023 19:35:23 GMT, Daniel Jeliński wrote:
>> src/java.base/share/classes/sun/security/util/BitArray.java line 72:
>>
>>> 70: * specified byte array. The most significant bit of {@code a[0]}
>>> gets
>>> 71: * index zero in the BitArray. The array must be large enough to
On Wed, 5 Jul 2023 18:04:01 GMT, Roger Riggs 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 sin
On Wed, 5 Jul 2023 18:11:41 GMT, Roger Riggs 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 sin
On Wed, 5 Jul 2023 14:52:22 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
On Wed, 5 Jul 2023 14:52:22 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
On Wed, 5 Jul 2023 12:16:32 GMT, Daniel Jeliński wrote:
>> Pavel Rappo has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add even more cases and tidy up
>
> src/java.base/share/classes/sun/security/provider/PolicyParser.java line 1133:
>
> Please review this PR to use modern APIs and language features to simplify
> `equals` and `hashCode` in security area.
>
> I understand that security area is sensitive and a non-expert, such as
> myself, should tread carefully; so below are my notes to assist the review.
>
> * Unlike `hashCod
On Wed, 5 Jul 2023 14:39:44 GMT, Pavel Rappo wrote:
>> src/java.base/share/classes/java/security/spec/ECFieldF2m.java line 235:
>>
>>> 233: public int hashCode() {
>>> 234: int value = m << 5;
>>> 235: // consider simplifying using Objects.hashCode(rp) after
>>> JDK-8015417
On Wed, 5 Jul 2023 12:31:09 GMT, Daniel Jeliński wrote:
>> Pavel Rappo has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add even more cases and tidy up
>
> src/java.base/share/classes/java/security/spec/ECFieldF2m.java line 235:
>
>> 233
On Tue, 4 Jul 2023 16:27:05 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
On Tue, 4 Jul 2023 16:27:05 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
On Tue, 4 Jul 2023 16:27:05 GMT, Pavel Rappo wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in security area.
>>
>> I understand that security area is sensitive and a non-expert, such as
>> myself, should tread carefully; so belo
> Please review this PR to use modern APIs and language features to simplify
> `equals` and `hashCode` in security area.
>
> I understand that security area is sensitive and a non-expert, such as
> myself, should tread carefully; so below are my notes to assist the review.
>
> * Unlike `hashCod
> Please review this PR to use modern APIs and language features to simplify
> `equals` and `hashCode` in security area.
>
> I understand that security area is sensitive and a non-expert, such as
> myself, should tread carefully; so below are my notes to assist the review.
>
> * Unlike `hashCod
Please review this PR to use modern APIs and language features to simplify
`equals` and `hashCode` in security area.
I understand that security area is sensitive and a non-expert, such as myself,
should tread carefully; so below are my notes to assist the review.
* Unlike `hashCode`, non-secure
73 matches
Mail list logo
