It looks fine. One question, line 866 of the test you print the stacktrace on a success, was that intentional?
Tony > On Dec 1, 2016, at 11:02 AM, Sean Mullan <sean.mul...@oracle.com> wrote: > > I enhanced the test case to test more scenarios where MD5 is either disabled > via the jdk.tls.disabledAlgorithms or the jdk.certpath.disabledAlgorithms > property. Please take another look and let me know if you are ok with it: > > http://cr.openjdk.java.net/~mullan/webrevs/8170131/webrev.01/ > > Thanks, > Sean > >> On 11/22/16 11:00 AM, Anthony Scarpino wrote: >>> On 11/22/2016 05:19 AM, Sean Mullan wrote: >>>> On 11/21/16 5:43 PM, Anthony Scarpino wrote: >>>>> On 11/21/2016 01:09 PM, Sean Mullan wrote: >>>>> Please review this fix for a bug where certificates were not being >>>>> blocked if the algorithm is only listed in the >>>>> jdk.tls.disabledAlgorithms property and not the >>>>> jdk.certpath.disabledAlgorithms property. >>>>> >>>>> I have modified an existing regression test to test this functionality >>>>> as there was no previous test for this feature. >>>>> >>>>> webrev: http://cr.openjdk.java.net/~mullan/webrevs/8170131/webrev.00/ >>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8170131 >>>>> >>>>> --Sean >>>> >>>> Is the reason the if() is needed is >>>> constraints.permit(CerttConstraintParameters) is not in the >>>> SSLAlgorithmConstraints class and the method exception is suppressed? >>> >>> SSLAlgorithmConstraints is not an instanceof >>> DisabledAlgorithmConstraints. When AlgorithmChecker.check is called, the >>> previous code (on line 329) would call >>> certPathDefaultConstraints.permits. This would pass, because the test >>> has configured jdk.certpath.disabledAlgorithms property to be empty. The >>> first time through, prevPubKey would also be null, so it would return on >>> line 335. It would never call SSLAlgorithmConstraints.permits. >>> >>> Does that make sense? >>> >>> --Sean >> >> >> Ok.. I see what's going on more.. I'm ok with this.. >> >> Tony >>
