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 >> >
