On Tue, 21 May 2024 20:11:03 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> Matthew Donovan has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains three additional >> commits since the last revision: >> >> - renamed CertificateBuilder methods, updated tests, and removed duplication >> - Merge branch 'master' into certbuilding >> - 8325766: Review seclibs tests for cert expiry > > test/jdk/java/security/testlibrary/CertificateBuilder.java line 417: > >> 415: * @throws Exception >> 416: */ >> 417: public static CertificateBuilder >> createClientCertificateBuilder(String subjectName, PublicKey publicKey, > > Suggest renaming to `newEndEntity()`. I think it would be more useful to pass > in the type of end-entity certificate you want to create as a parameter, like > TLS Server, TLS client, code signer, Time Stamp Authority, etc, and then the > method creates the recommended key usage and extended key usages for each > type, and maybe some of the extensions as well, although you may need more > parameters and overloaded methods in those cases. You can use an Enum for the > type. See `sun.security.validator` classes for similar ideas. I renamed the method. I don't want to over-generalize the code when I don't know what we'll need/want in the future. The tests in this PR just create CA and end-entity certs and with a couple exceptions, the tests in this PR are all of the tests that fail when the system clock is set to 2050. The exceptions are also client/server tests that don't need code signer and TSA certificates. When considering your comment, I went through the tests in this PR and found and removed some additional redundancy. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18958#discussion_r1610165349
