RFR: 8325766: Review seclibs tests for cert expiry [v4]

Sean Mullan mullan at openjdk.org
Tue Jun 3 15:33:56 UTC 2025


On Tue, 3 Jun 2025 13:15:33 GMT, Matthew Donovan <mdonovan at openjdk.org> wrote:

> I think the way it currently is follows the builder pattern better. All of the 'set' methods return `this` which means if I change this method to a constructor, I'd have to duplicate the "set" code from all those methods.

I'm not sure what you mean - can you give an example? I don't see any advantages of using a static method here, but I agree it works either way.

> I like the idea of using `Duration` but the API for it doesn't really lend itself to this use case. When the certificate is finally created, we write `Date` objects to the DerOutputStream so I'd have to choose a start time and then calculate the end time based on the duration. It's not hard, but it doesn't seem worth it when the common use case of this class is to generate short-lived certificates for a test. The default one-hour validity will cover the vast majority of tests.

I view this test utility class as being flexible enough for different cases. This is a useful method in that the other parameters are common fields that all certs usually have but it also means I can't use this method to to create a certificate with a longer, or shorter validity period. Alternatively, how about adding another method that hard-codes it as one hour:

`CertificateBuilder oneHourValidity()`

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23700#discussion_r2124254602


More information about the security-dev mailing list