RFR: 8261502: ECDHKeyAgreement: Allows alternate ECPrivateKey impl and revised exception handling

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8261502: ECDHKeyAgreement: Allows alternate ECPrivateKey impl and revised exception handling

Anthony Scarpino-2
Hi,

I need a code review of this change to ECDH.  It is a combination of fixing the implementation to not only accept ECPrivateKeyImpl along with a fix to the exception handling.  They started as two fixes, but with the exception handling the underlying code changed significantly that made the ECPrivateKey change in a different place.  The new exception handling is a result of no longer having the native library.  Many of the checks waited until generateSecret() to send the keys to the native library. Now that native is gone, checks can happen when keys are provided to the methods and proper exceptions can be thrown instead of wrapping everything as a ProviderException

thanks,

Tony

-------------

Commit messages:
 - merge with 8261502
 - initial change

Changes: https://git.openjdk.java.net/jdk/pull/2659/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2659&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8261502
  Stats: 205 lines in 2 files changed: 138 ins; 32 del; 35 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2659.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2659/head:pull/2659

PR: https://git.openjdk.java.net/jdk/pull/2659
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261502: ECDHKeyAgreement: Allows alternate ECPrivateKey impl and revised exception handling [v2]

Anthony Scarpino-2
> Hi,
>
> I need a code review of this change to ECDH.  It is a combination of fixing the implementation to not only accept ECPrivateKeyImpl along with a fix to the exception handling.  They started as two fixes, but with the exception handling the underlying code changed significantly that made the ECPrivateKey change in a different place.  The new exception handling is a result of no longer having the native library.  Many of the checks waited until generateSecret() to send the keys to the native library. Now that native is gone, checks can happen when keys are provided to the methods and proper exceptions can be thrown instead of wrapping everything as a ProviderException
>
> thanks,
>
> Tony

Anthony Scarpino has updated the pull request incrementally with one additional commit since the last revision:

  Simpler fix for ECPrivateKey

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2659/files
  - new: https://git.openjdk.java.net/jdk/pull/2659/files/3451055a..aec24cc1

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2659&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2659&range=00-01

  Stats: 5 lines in 1 file changed: 0 ins; 1 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2659.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2659/head:pull/2659

PR: https://git.openjdk.java.net/jdk/pull/2659