I don't use enum a lot, and we have 2 types of objects there. I'll keep it.
Thanks, Max > On Jan 16, 2019, at 11:15 AM, Xuelei Fan <xuelei....@oracle.com> wrote: > > Looks fine to me. > > I was wondering, if it is more simple to define PSSParamsHolder as an enum? > Anyway, it is just very minor comment. You can leave it as it is. > > Thanks, > Xuelei > >> Hi Xuelei, >> Webrev updated at >> https://cr.openjdk.java.net/~weijun/8215694/webrev.01 >> A new method AlgorithmId::getWithParameterSpec is added. I also cached 6 >> constants in AlgorithmId $PSSParamsHolder, although the static block inside >> it to generate the AlgorithmIds are a little heavy. We can extract the >> encoding lines inside PSSParameters::engineGetEncoded to create something >> like PSSParameterSpec::getEncoded(), but that will be an RFE. >> Thanks, >> Max >>> On Jan 3, 2019, at 11:27 PM, Xue-Lei Fan <xuelei....@oracle.com> wrote: >>> >>> On 1/3/2019 2:10 AM, Weijun Wang wrote: >>>>> On Jan 2, 2019, at 11:56 PM, Xue-Lei Fan <xuelei....@oracle.com> wrote: >>>>> >>>>> sigAlg.equalsIgnoreCase("RSASSA-PSS"): >>>>> Do you really want to ignore the case? I used to think that an algorithm >>>>> name is case sensitive. >>>> getInstance(alg) is always case-insensitive. >>> Hm, I missed it. >>> >>>>> >>>>> Main.java:1445 minor, 4 more indent? >>>> Then it's longer than 80 chars. How about I un-indent lines 1443 and 1444? >>> maybe, use 4 white spaces? See also the following comment. >>> >>>>> >>>>> AlgorithmId.java:1073-1091: >>>>> I may prefer to use cached parameters (for both AlgorithmParameters and >>>>> AlgorithmParameterSpec) for each size, for performance. >>>> OK for AlgorithmParameterSpec. Which AlgorithmParameters do you mean? The >>>> one in SignatureUtil? >>> Yes, replacing SignatureUtil.createAlgorithmParameters(). Then we don't >>> need to worry about the indents above. >>> >>> Thanks, >>> Xuelei >>> >>>> Thanks, >>>> Max >>>>> >>>>> >>>>> Xuelei >>>>> >>>>> >>>>> On 12/21/2018 1:44 AM, Weijun Wang wrote: >>>>>> Please take a review at >>>>>> https://cr.openjdk.java.net/~weijun/8215694/webrev.00/ >>>>>> This bug reveals several issues: >>>>>> 1. Encoding of the RSASSA-PSS signature algorithm in PKCS10 and >>>>>> X509CertImpl. >>>>>> 2. The missing of setParameter() call for PKCS10 and X509CertImpl. >>>>>> 3. All keytool commands of -genkeypair, -certreq, -gencert, -selfcert >>>>>> are affected. >>>>>> 4. Wrong NULL after encoding of RSASSA-PSS key algorithm. >>>>>> Please confirm this is safe to be fixed in JDK 12. >>>>>> Thanks, >>>>>> Max
