On Wed, 24 May 2023 13:00:40 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> Sibabrata Sahoo has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8308711: Comment addressed > > test/jdk/javax/crypto/KEM/GenLargeNumberOfKeys.java line 1: > >> 1: /* > > 1. `testXDH` and `testEC` are mostly identical. Maybe you can write a single > method with 2 extra arguments. > 2. According to the spec, multiple keys generated from a *single* > `Encapsulator` are different. The `test` method is creating a new > encapsulators each time. > 3. There is no guarantee that a `SecretKey` follows the `hashCode/equals` > convention and can be put inside a `Set` to detect duplication. It happens > that in this implementation the key is a `SecretKeySpec` object so it works. Updated accordingly. > test/jdk/javax/crypto/KEM/KemInterop.java line 77: > >> 75: KEM.Encapsulated enc2 = encT1.encapsulate(); >> 76: >> 77: Asserts.assertEQ(enc.key(), enc.key()); > > Again, we cannot guarantee `equals` between 2 `SecretKey` objects. However, > since it's a positive test here, it's OK to do this. If we really modify the > implementation later and return a different kind of `SecretKey`, we can > update the test later. Removed Key equals() comparison. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14113#discussion_r1211141295 PR Review Comment: https://git.openjdk.org/jdk/pull/14113#discussion_r1211141000
