Will the update for getEncodedParams() has other compatibility impact? As it is a public method, I'm not sure if the update is necessary and will impact ECDSA encoding in other places. Please double check it.

Otherwise, looks good to me.

Xuelei

On 12/21/2019 5:33 AM, Weijun Wang wrote:
I uploaded a less disruptive webrev at 
https://cr.openjdk.java.net/~weijun/8236470/webrev.01.

--Max

On Dec 21, 2019, at 7:47 PM, Weijun Wang <weijun.w...@oracle.com> wrote:

Oh, I take back the "mutates the internal" words, it didn't. algName is only a 
local variable.

So the major problem of the current code is that while getName() is modified to return 
the "full" signature algorithm name, getEncodedParams() still returns it.

I realized my fix does have the problem that AlgorithmId.parse(input).encode() 
will be different from input, but making change at the root is still better 
than recognizing specifiedWithECDSA_oid and ignoring parameters everywhere.

Thanks,
Max


On Dec 21, 2019, at 7:23 PM, Weijun Wang <weijun.w...@oracle.com> wrote:

Please take a review at

  http://cr.openjdk.java.net/~weijun/8236470/webrev.00/

The current implementation is not good at dealing with ECDSA specified by ecdsa-with-SHA2 
plus a hash algorithm. While the AlgorithmId::getName is able to return the 
"full" signature algorithm name, it mutates the internal, cannot be guaranteed 
to be called, and leaves the parameters unchanged. This fix move the logic to 
AlgorithmId::parse and the class becomes practically immutable.

For Oracle internal reviewers: An update is made to the test also. The closed 
path has been wrong since the repo consolidation. We do have such certs in the 
closed area.

Thanks,
Max



Reply via email to