Re: [15] RFR JDK-8242151 Improve OID mapping and reuse among JDK security providers for aliases registration
Hi Max, Webrev has been updated, http://cr.openjdk.java.net/~valeriep/8242151/webrev.02/. Major changes are: - Moved oidTable caching from AlgorithmId class to ObjectIdentifier class. Made ObjectIdentifier constructor private and callers have to use the of(String) method which always check the oidTable cache before creating new instances. Some more files (src files and tests) are added to the webrev due to the private ObjectIdentifier constructor change. - Arrays.asList() calls are now replaced with List.of() calls. - KnownOIDs enum has been enhanced with registerName() method for ensuring that same standard name can have at most one enum mapping. Added the method stdName() instead of relying on toString(). Added aliases support to KnownOIDs enums. Note that external aliases are in SecurityProviderConstants class. The two non-oid BASE ones are removed and keytool/Main.java is updated. - SecurityProviderConstants class will pick up the internal aliases and combine with external aliases and handle multiple oid enums in its store(...) method. - Updated SunRsaSign provider to use the same naming convention (append the letter A) and fixed its KeyFactory and KeyPairGenerator to use the same oid as before. Also update providers outside of the java.base module in a similar fashion. As for the comments below. Please find replies inline. On 5/4/2020 6:24 PM, Weijun Wang wrote: Do you want to add OIDs in CurveDB into KnownOIDs as well? Sure, will do in webrev.03. Thanks, Valerie Thanks, Max
Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)
> On May 6, 2020, at 6:51 AM, Mikael Vidstedt
> wrote:
>
>
> Max,
>
> Thank you so much for the thorough review! I’m working on an incremental
> webrev but could use some help - comments/questions inline..
>
>> On May 4, 2020, at 6:58 AM, Weijun Wang wrote:
>>
>> I've gone through all files. Many of them are PKCS11-related, it will be
>> nice if Valerie can confirm my findings.
>>
>> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/Config.java:
>>
>> Maybe we should not support $ISA at all?
>
> It does seem like it could go, but I’m not comfortable making that change
> myself. How about a follow-up issue?
Sure.
>
>> test/jdk/com/sun/security/auth/login/ConfigFile/InconsistentError.java:
>>
>> Please also remove the whole else block from line 61.
>
> Fixed.
>
>> test/jdk/java/security/KeyPairGenerator/SolarisShortDSA.java:
>>
>> It's not worth keeping this test. Error has never occurred on other
>> platforms.
>
> Fixed.
>
>> test/jdk/java/security/KeyStore/TestKeyStoreBasic.java:
>>
>> 128-135: There is no need to use a try-catch block.
>
> Fixed.
>
>> test/jdk/java/security/KeyStore/TestKeyStoreEntry.java:
>>
>> 54-60: No need
>
> Fixed.
>
>> 81-97: No need. Just a single run() is OK.
>> 101: Remove the ", p” argument
>
> I’m not sure I understand what to do here. Help? :)
Since there is only one provider having "jceks" you can
1 remove run() method
2 rename runTest(p) to run()
3 call 'KeyStore.getInstance("jceks")' without the "p" argument.
>
>> test/jdk/java/security/MessageDigest/TestDigestIOStream.java:
>> test/jdk/java/security/MessageDigest/TestSameLength.java:
>> test/jdk/java/security/MessageDigest/TestSameValue.java:
>>
>> No need for isSHA3Supported(). The SUN provider should always be there.
>
> Fixed.
>
>> Inside the test where NoSuchAlgorithmException is caught, the catch block
>> can be removed and no need to try
>
> Fixed.
>
>> test/jdk/java/security/MessageDigest/UnsupportedProvider.java:
>>
>> Not worth keeping this test.
>
> Fixed.
>
>> test/jdk/sun/security/jca/PreferredProviderTest.java:
>>
>> 53: remove "for other platform”
>
> Fixed.
>
>> test/jdk/sun/security/pkcs/pkcs8/PKCS8Test.java
>>
>> Remove the comments on lines 37-40
>
> Fixed.
>
>> test/jdk/sun/security/pkcs11/Config/ReadConfInUTF16Env.java:
>>
>> Not worth keeping the tests in this directory.
>
> Fixed.
>
>> test/jdk/sun/security/pkcs11/Mac/MacSameTest.java:
>>
>> No need for try
>
> Fixed.
>
>> test/jdk/sun/security/pkcs11/Provider/ConfigShortPath.java:
>>
>> Will this test ever run? There is no out-of-box SunPKCS11 provider. Even if
>> one is configured manually, the name is likely to be SunPKCS11-XYZ,
>
> You tell me? :)
This is why I included Valerie, I'll ping her.
Thanks,
Max
>
>> test/jdk/sun/security/pkcs11/tls/TestKeyMaterial.java:
>>
>> No need for this catch block
>
> Fixed (removed the InvalidAlgorithmParameterException catch).
>
>> test/jdk/sun/security/tools/keytool/fakegen/PSS.java:
>>
>> Please also remove the comment on lines 35-37.
>
> Fixed.
>
>> test/jdk/sun/security/tools/keytool/fakegen/DefaultSignatureAlgorithm.java:
>>
>> Please also remove the comment on lines 36-38.
>
> Fixed.
>
> Cheers,
> Mikael
>
>>
>>> On May 4, 2020, at 1:12 PM, Mikael Vidstedt
>>> wrote:
>>>
>>>
>>> Please review this change which implements part of JEP 381:
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8244224
>>> webrev:
>>> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/security/open/webrev/
>>> JEP: https://bugs.openjdk.java.net/browse/JDK-8241787
>>>
>>>
>>> Note: When reviewing this, please be aware that this exercise was
>>> *extremely* mind-numbing, so I appreciate your help reviewing all the
>>> individual changes carefully. You may want to get that coffee cup filled up
>>> (or whatever keeps you awake)!
>>>
>>>
>>> Background:
>>>
>>> Because of the size of the total patch and wide range of areas touched,
>>> this patch is one out of in total six partial patches which together make
>>> up the necessary changes to remove the Solaris and SPARC ports. The other
>>> patches are being sent out for review to mailing lists appropriate for the
>>> respective areas the touch. An email will be sent to jdk-dev summarizing
>>> all the patches/reviews. To be clear: this patch is *not* in itself
>>> complete and stand-alone - all of the (six) patches are needed to form a
>>> complete patch. Some changes in this patch may look wrong or incomplete
>>> unless also looking at the corresponding changes in other areas.
>>>
>>> For convenience, I’m including a link below[1] to the full webrev, but in
>>> case you have comments on changes in other areas, outside of the files
>>> included in this thread, please provide those comments directly in the
>>> thread on the appropriate mailing list for that area if possible.
>>>
>>> In case it helps, the changes were effectively produced by
Re: [15] RFR JDK-8242151 Improve OID mapping and reuse among JDK security providers for aliases registration
Hi Max,
Thanks for the comments, they are very helpful. I should have webrev.02
ready in a bit. Please find comments below in line.
On 5/1/2020 2:45 AM, Weijun Wang wrote:
ObjectIdentifier.java
-
Have you thought about storing the ObjectIdentifier object somewhere?
ObjectIdentifier.of() creates a new object each time and the conversion of
string to byte[] might be a performance issue. We used to have a lot of
ObjectIdentifier objects in AlgorithmId but now we only have KnownOIDs.
The ObjectIdentifier objects are stored inside the oidTable map in
AlgorithmId class. I removed the static oid constants which aren't
referenced. The oids are retrieved from the oidTable when needed. With
the webrev, we are exchange oid construction with a map look up, at
least this is the intention.
I had a talk with Stuart and he has a suggestion that we can stuff all pre-calculated OID DER
encodings in a long byte array in a resource file, and in KnownOIDs each element has an
offset/length pair that point to its DER encoding. Also, whatever cache mechanism we use in the
future, I suggest making "new ObjectIdentifier(String)" private and keep
"ObjectIdentifier of(String)".
Done in webrev.02.
SecurityProviderConstants.java
--
public static List alias(String ... aliases) {
return Arrays.asList(aliases);
}
Probably not necessary, you can simply call List.of(...) everywhere.
Done in webrev.02.
SunMSCAPI.java
--
Why not call getAliases() inside "new ProviderService" like in the other
providers? Same in UCrypto.
Ok, done in webrev.02.
AlgorithmId.java
algOID(String). You don't check "if (name.indexOf('.') != -1)" at the beginning anymore.
Is there an algorithm name containing "."?
Nope, it's more because the KnownOIDs enum can handle oid string lookup,
so I just need to remove the "OID." prefix if present.
It's a pity we have to collect OIDs from other providers. Maybe it should only
be necessary when we use that provider, for example, when encoding a signature,
we should ask the provider about the OID. I wish there were a
Signature::getAlgorithmId, but if not, maybe we can rename
Algorithm::alfOID(name) to Algorithm::alfOID(name, provider).
The existing OID collection code in AlgorithmId class serves as a safety
net when new OID alias mapping is added to provider but AlgorithmId
class is not also updated. After this KnownOIDs change, this safety net
may not be that necessary and for performance reason, the code would
skip JDK providers. However, for max backward compatibility, we may have
to keep it. I agree with your name->oid mapping can be provider-specific
and handling that would be somewhat sticky with current code. As for
including an extra provider argument, I am not sure if that'd work
across the board as the caller of AlgorithmId class may not have the
provider info. OID usage can be somewhat complicated as demonstrated by
this change.
Do you know a bad case if we don't collect those OIDs? It must be some
algorithm that is not in the Standard Names.
With existing regression tests, nothing breaks if we don't collect the
OID aliases.
Overall I think the change looks great, and we have a single place to store all
OIDs. The mapping among the OID string, KnownOIDs, and ObjectIdentifier could
be enhanced. Do you have a benchmark?
I have not done one. I can try something out once I finish addressing
your other comments.
Thanks,
Valerie
Thanks,
Max
On Apr 30, 2020, at 6:59 AM, Valerie Peng wrote:
Here is the updated webrev
http://cr.openjdk.java.net/~valeriep/8242151/webrev.01/
Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)
John,
Thanks for the review! Comments/questions inline..
> On May 4, 2020, at 4:02 PM, sha.ji...@oracle.com wrote:
>
> Hi,
> Generally, this patch doesn't take care of the solaris/sunos/ucrypto-related
> words in test summaries, code comments, configs and READMEs.
> E.g.
> test/jdk/javax/net/ssl/templates/SSLEngineTemplate.java
> test/jdk/sun/security/provider/MessageDigest/TestSHAClone.java
> test/jdk/sun/security/util/Resources/Format.config
> test/jdk/sun/security/pkcs11/KeyStore/BasicData/README
> Would we need some follow-up issues to double-check this point?
I’ve deliberately avoided changing comments I didn’t feel comfortable
modifying. I’d prefer to file follow-ups as needed, but if you have specific
suggestions (or patches!) I should include in the already huge patch do let me
know.
> test/jdk/sun/security/tools/keytool/KeyToolTest.java
> 39 * Testing Solaris Cryptography Framework PKCS11 keystores:
> 40 * # make sure you've already run pktool and set test12 as pin
> 41 * echo | java -Dsolaris KeyToolTest
> ...
> 1863 if (System.getProperty("solaris") != null) {
> 1864 // For Solaris Cryptography Framework
> 1865 t.srcP11Arg = SUN_SRC_P11_ARG;
> 1866 t.p11Arg = SUN_P11_ARG;
> 1867 t.testPKCS11();
> 1868 t.testPKCS11ImportKeyStore();
> 1869 t.i18nPKCS11Test();
> 1870 }
> It may be necessary to remove the above lines.
I was staring at this one for quite a while. AFAICT there’s nothing Solaris
specific about that block or the methods it’s calling - to me it looks like the
property just happens to be called “solaris”, but it could equally well be
called “pkcs11” or “standard” or “foobar”. That said, I don’t know enough about
PKCS11 & friends to say for sure. Do let me know if it is indeed dead code and
should be removed..
> test/jdk/sun/security/provider/PolicyParser/PrincipalExpansionError.java
> test/jdk/sun/security/provider/PolicyParser/PrincipalExpansionError.policy
> It looks this manual test and the associated policy are solaris-related only.
> Could these files be removed as well?
> In fact, the solaris-related com.sun.security.auth classes, including
> SolarisPrincipal, are already removed.
Ah, that indeed seems to be the case. Made me wonder why the test doesn’t fail
to compile altogether, but then I noticed that it’s ProblemListed on all
platforms..
How about a follow-up since this isn’t strictly related to the Solaris/SPARC
removal itself - perhaps https://bugs.openjdk.java.net/browse/JDK-8039280
covers it?
> test/jdk/sun/security/pkcs11/tls/TestPRF.java
> 114 if (secret == null) {
> 115 // This fails on Solaris, but since we never
> call this
> 116 // API for this case in JSSE, ignore the
> failure.
> 117 // (SunJSSE uses the
> CKM_TLS_KEY_AND_MAC_DERIVE
> 118 // mechanism)
> 119 System.out.print("X");
> 120 continue;
> 121 }
> Could remove this block?
Good question - the comment certainly makes it sound Solaris specific, but
could be a stale comment..
Cheers,
Mikael
>
> On 2020/5/4 13:12, Mikael Vidstedt wrote:
>> Please review this change which implements part of JEP 381:
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8244224
>> webrev:
>> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/security/open/webrev/
>> JEP: https://bugs.openjdk.java.net/browse/JDK-8241787
>>
>>
>> Note: When reviewing this, please be aware that this exercise was
>> *extremely* mind-numbing, so I appreciate your help reviewing all the
>> individual changes carefully. You may want to get that coffee cup filled up
>> (or whatever keeps you awake)!
>>
>>
>> Background:
>>
>> Because of the size of the total patch and wide range of areas touched, this
>> patch is one out of in total six partial patches which together make up the
>> necessary changes to remove the Solaris and SPARC ports. The other patches
>> are being sent out for review to mailing lists appropriate for the
>> respective areas the touch. An email will be sent to jdk-dev summarizing all
>> the patches/reviews. To be clear: this patch is *not* in itself complete and
>> stand-alone - all of the (six) patches are needed to form a complete patch.
>> Some changes in this patch may look wrong or incomplete unless also looking
>> at the corresponding changes in other areas.
>>
>> For convenience, I’m including a link below[1] to the full webrev, but in
>> case you have comments on changes in other areas, outside of the files
>> included in this thread, please provide those comments directly in the
>> thread on the appropriate mailing list for that area if possible.
>>
>> In case it helps, the changes were effectively
Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports (security)
Max, Thank you so much for the thorough review! I’m working on an incremental webrev but could use some help - comments/questions inline.. > On May 4, 2020, at 6:58 AM, Weijun Wang wrote: > > I've gone through all files. Many of them are PKCS11-related, it will be nice > if Valerie can confirm my findings. > > src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/Config.java: > > Maybe we should not support $ISA at all? It does seem like it could go, but I’m not comfortable making that change myself. How about a follow-up issue? > test/jdk/com/sun/security/auth/login/ConfigFile/InconsistentError.java: > > Please also remove the whole else block from line 61. Fixed. > test/jdk/java/security/KeyPairGenerator/SolarisShortDSA.java: > > It's not worth keeping this test. Error has never occurred on other > platforms. Fixed. > test/jdk/java/security/KeyStore/TestKeyStoreBasic.java: > > 128-135: There is no need to use a try-catch block. Fixed. > test/jdk/java/security/KeyStore/TestKeyStoreEntry.java: > > 54-60: No need Fixed. > 81-97: No need. Just a single run() is OK. > 101: Remove the ", p” argument I’m not sure I understand what to do here. Help? :) > test/jdk/java/security/MessageDigest/TestDigestIOStream.java: > test/jdk/java/security/MessageDigest/TestSameLength.java: > test/jdk/java/security/MessageDigest/TestSameValue.java: > > No need for isSHA3Supported(). The SUN provider should always be there. Fixed. > Inside the test where NoSuchAlgorithmException is caught, the catch block > can be removed and no need to try Fixed. > test/jdk/java/security/MessageDigest/UnsupportedProvider.java: > > Not worth keeping this test. Fixed. > test/jdk/sun/security/jca/PreferredProviderTest.java: > > 53: remove "for other platform” Fixed. > test/jdk/sun/security/pkcs/pkcs8/PKCS8Test.java > > Remove the comments on lines 37-40 Fixed. > test/jdk/sun/security/pkcs11/Config/ReadConfInUTF16Env.java: > > Not worth keeping the tests in this directory. Fixed. > test/jdk/sun/security/pkcs11/Mac/MacSameTest.java: > > No need for try Fixed. > test/jdk/sun/security/pkcs11/Provider/ConfigShortPath.java: > > Will this test ever run? There is no out-of-box SunPKCS11 provider. Even if > one is configured manually, the name is likely to be SunPKCS11-XYZ, You tell me? :) > test/jdk/sun/security/pkcs11/tls/TestKeyMaterial.java: > > No need for this catch block Fixed (removed the InvalidAlgorithmParameterException catch). > test/jdk/sun/security/tools/keytool/fakegen/PSS.java: > > Please also remove the comment on lines 35-37. Fixed. > test/jdk/sun/security/tools/keytool/fakegen/DefaultSignatureAlgorithm.java: > > Please also remove the comment on lines 36-38. Fixed. Cheers, Mikael > >> On May 4, 2020, at 1:12 PM, Mikael Vidstedt >> wrote: >> >> >> Please review this change which implements part of JEP 381: >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-8244224 >> webrev: >> http://cr.openjdk.java.net/~mikael/webrevs/8244224/webrev.00/security/open/webrev/ >> JEP: https://bugs.openjdk.java.net/browse/JDK-8241787 >> >> >> Note: When reviewing this, please be aware that this exercise was >> *extremely* mind-numbing, so I appreciate your help reviewing all the >> individual changes carefully. You may want to get that coffee cup filled up >> (or whatever keeps you awake)! >> >> >> Background: >> >> Because of the size of the total patch and wide range of areas touched, this >> patch is one out of in total six partial patches which together make up the >> necessary changes to remove the Solaris and SPARC ports. The other patches >> are being sent out for review to mailing lists appropriate for the >> respective areas the touch. An email will be sent to jdk-dev summarizing all >> the patches/reviews. To be clear: this patch is *not* in itself complete and >> stand-alone - all of the (six) patches are needed to form a complete patch. >> Some changes in this patch may look wrong or incomplete unless also looking >> at the corresponding changes in other areas. >> >> For convenience, I’m including a link below[1] to the full webrev, but in >> case you have comments on changes in other areas, outside of the files >> included in this thread, please provide those comments directly in the >> thread on the appropriate mailing list for that area if possible. >> >> In case it helps, the changes were effectively produced by searching for and >> updating any code mentioning “solaris", “sparc”, “solstudio”, “sunos”, etc. >> More information about the areas impacted can be found in the JEP itself. >> >> >> Testing: >> >> A slightly earlier version of this change successfully passed tier1-8, as >> well as client tier1-2. Additional testing will be done after the first >> round of reviews has been completed. >> >> Cheers, >> Mikael >> >> [1] >>
Re: RFR[15] 8242060: Add revocation checking to jarsigner
> On May 5, 2020, at 6:16 AM, Sean Mullan wrote: > > On 5/2/20 2:19 PM, Hai-May Chao wrote: >>> Looks good otherwise. Please add a release-note and open a follow-on issue >>> to update the man page with the new option. >> Done (Release note: JDK-8244285, and man page: JDK-8244274). > > I added a few more details to the release note. Otherwise, looks good, so you > can mark it Resolved/Delivered. > Thanks for your review. I’ve marked it Resolved/Delivered. Hai-May
RFR JDK-8206925,,Support the certificate_authorities extension
Hi, Could I get the following update reviewed? RFE: https://bugs.openjdk.java.net/browse/JDK-8206925 CSR: https://bugs.openjdk.java.net/browse/JDK-821 Release-note: https://bugs.openjdk.java.net/browse/JDK-8244460 webrev: http://cr.openjdk.java.net/~xuelei/8206925/webrev.00/ The "certificate_authorities" extension is an optional extension introduced in TLS 1.3 and used to indicate the certificate authorities (CAs) which an endpoint supports and which SHOULD be used by the receiving endpoint to guide certificate selection. In TLS 1.2, this function is built in the CertificateRequest handshake massage. This function is supported in TLS 1.2 and prior versions. However, it is not implemented in the TLS 1.3 implementation. Without this function, the authentication certificate selected may be not the one the peer could accepted, when there are multiple certificates available. Thanks, Xuelei
Re: RFR[15] 8242060: Add revocation checking to jarsigner
On 5/2/20 2:19 PM, Hai-May Chao wrote: Looks good otherwise. Please add a release-note and open a follow-on issue to update the man page with the new option. Done (Release note: JDK-8244285, and man page: JDK-8244274). I added a few more details to the release note. Otherwise, looks good, so you can mark it Resolved/Delivered. --Sean
Re: [15] RFR JDK-8242151 Improve OID mapping and reuse among JDK security providers for aliases registration
I am playing with keytool + BouncyCastle and generate a key pair using the sigalg "SHA3-256WITHECDSA", and `keytool -list` cannot show the signature name. So I tried 2 changes in AlgorithmId.java: 1. add both the name->oid and oid->name mapping inside collectOIDAliases() for 3rd party providers 2. fallback to this mapping if KnownOIDs.findMatch() fails in getName(). Then I can see the signature name. You can consider including this or we can make this in a future RFE. Thanks, Max
Re: RFR[15] 8242060: Add revocation checking to jarsigner
> On May 4, 2020, at 10:23 PM, Weijun Wang wrote:
>
>
>
>> On May 5, 2020, at 12:36 PM, Hai-May Chao wrote:
>>
>>
>>
>>> On May 4, 2020, at 6:01 PM, Weijun Wang wrote:
>>>
>>>
>>>
On May 5, 2020, at 3:48 AM, Hai-May Chao wrote:
Hi Max,
> On May 2, 2020, at 5:25 PM, Weijun Wang wrote:
>
> In jarsigner/Main, you can just call
>
> pkixParameters.setRevocationEnabled(revocationCheck);
Ok
>
> Last time, you mentioned that "Event.clearReportListener()" cannot be
> called immediately after validation check because of some race
> conditions. Is that easily reproducible? I still find it strange.
It is easily reproducible. I should have better explained why the
suggested changes did not work but not due to race condition. The code
pkixParameters.setRevocationEnabled(revocationCheck) only sets the
revocation checker enabled. It is the validateCertChain() in Main.java,
which calls into RevocationChecker.check() and ultimately calls
Event.report() prior to making OCSP and CRL connections. If we call
clearReportListener in loadKeyStore() method
>
> I don't intend to clear the listener in loadKeyStore().
>
(i.e finally{ }as suggested), for OCSP, by the time when
OCSP.getOCSPBytes() comes in to report the OCSP event, the reporter has
been cleared. And this would be same problem for CRL. So it cannot be
called immediately.
>
> I was thinking about clearing the listener only after validation. However, I
> can see now that Validator.getInstance(...).validate(...) is called in
> validateCertChain(), then called in multiple places. It's a little
> complicated to set/clear multiple times.
>
> So it's OK to clear it in run(). Do you think we can also set the listener in
> this method? Say, before the "if (verify)" line?
The current change is based on the original flow where it sets PKIX revocation.
Hence, I’d like to keep the current change as is unless there is an issue with
it.
Thanks,
Hai-May
>
> Thanks,
> Max
>
>>>
>>> Then I assume the following is OK.
>>>
>>> try {
>>> setReportListener();
>>> validator.validate();
>>> } finally {
>>> clearReportListener();
>>> }
>>>
>>
>> I’d like to know if there is an issue with the current webrev, which is
>> being tested and it’s working as expected. I’ve also tried with your
>> previous suggested change, and it didn’t work.
>>
>> Thanks,
>> Hai-May
>>
>>
>>> Thanks,
>>> Max
>>>
Thanks,
Hai-May
>
> Thanks,
> Max
>
>> On May 3, 2020, at 2:19 AM, Hai-May Chao wrote:
>>
>> Hi Sean,
>>
>> Thanks for your review. Reply inline.
>>
>>> On May 1, 2020, at 11:50 AM, Sean Mullan wrote:
>>>
>>> * Main.java:
>>>
>>> 2067 Event.setReportListener(new
>>> Event.Reporter() {
>>> 2068 @Override
>>> 2069 public void handle(String t, Object...
>>> o) {
>>> 2070 System.out.println(String.format(rb.getString(t), o));
>>> 2071 }
>>> 2072 });
>>>
>>> I think you could use a lambda expression above.
>>
>> Done.
>>
>>>
>>> * Event.java:
>>>
>>> 35 static Reporter reporter = null;
>>>
>>> Make this private? Also, no need to explicitly initialize to null as
>>> that is the default value.
>>
>> Done (made it private).
>>
>>>
>>> Can you add some comments at the top of the class describing the
>>> purpose of this class?
>>
>> Done.
>>
>>>
>>> * EnableRevocation.java
>>>
>>> - How long does this test take - does it hang for a little while trying
>>> to make a connection or timeout right away? If it takes a while, you
>>> could experiment with overriding the default timeouts for CRLs and OCSP
>>> checks to make this test finish faster. Use the system properties
>>> com.sun.security.ocsp.timeout and com.sun.security.crl.readtimeout.
>>
>> It does not hang for OCSP as the certificate is set with localhost as
>> OCSP responder. It hangs just a little for CRL, thus I’ve changed the
>> certificate to local host instead of remote host. The whole test is
>> finishing very fast now.
>>
>>>
>>> Looks good otherwise. Please add a release-note and open a follow-on
>>> issue to update the man page with the new option.
>>
>> Done (Release note: JDK-8244285, and man page: JDK-8244274).
>>
>> Updated webrev:
>> https://cr.openjdk.java.net/~hchao/8242060/webrev.02/
>>
>> Thanks,
>> Hai-May
>>
>>
>>>
>>> --Sean
>>>
>>> On 5/1/20 12:02 PM, Hai-May Chao wrote:
Hi,
With small change added to ‘Usages.java' test, here is the updated
