Add withValidityPeriod to MFT and CRL builders #141
Conversation
| public DateTime getThisUpdateTime() { | ||
| return thisUpdateTime; | ||
| } | ||
|
|
||
| public X509CrlBuilder withNextUpdateTime(DateTime instant) { |
There was a problem hiding this comment.
Maybe we can even deprecate the old ones with annotations since they are a footcannon? Not sure if we ever need to set just one.
There was a problem hiding this comment.
It's also curious to think about where we can check that a validityperiod is sane, but that would be hard for any valid object. Maybe in the validation checks in the generic signed object parser?
There was a problem hiding this comment.
Good point,
search says that they are not used separately anywhere
https://github.com/search?q=org%3ARIPE-NCC%20withNextUpdateTime&type=code
So I'll mark them as deprecated
There was a problem hiding this comment.
ValidityPeriod does the check that the second date is >= the first one, so it's relatively sane. Can't come up with a good idea what is the use case of notValidBefore.isEqual(notValidAfter) though.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| public void shouldBeEquivalentToSetDateInDifferentWays() { | ||
| var builder1 = new ManifestCmsBuilder(); | ||
| builder1.withManifestNumber(BigInteger.valueOf(68)); | ||
| builder1.withThisUpdateTime(THIS_UPDATE_TIME); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
| var builder1 = new ManifestCmsBuilder(); | ||
| builder1.withManifestNumber(BigInteger.valueOf(68)); | ||
| builder1.withThisUpdateTime(THIS_UPDATE_TIME); | ||
| builder1.withNextUpdateTime(NEXT_UPDATE_TIME); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
| var builder2 = new ManifestCmsBuilder(); | ||
| builder2.withManifestNumber(BigInteger.valueOf(68)); | ||
| builder2.withValidityPeriod(new ValidityPeriod(THIS_UPDATE_TIME, NEXT_UPDATE_TIME)); | ||
| builder2.withNextUpdateTime(NEXT_UPDATE_TIME); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
| public void shouldBeEquivalentToSetDateInDifferentWays() { | ||
| var builder1 = new X509CrlBuilder(); | ||
| builder1.withIssuerDN(new X500Principal("CN=ROOT")); | ||
| builder1.withThisUpdateTime(THIS_UPDATE_TIME); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
| var builder1 = new X509CrlBuilder(); | ||
| builder1.withIssuerDN(new X500Principal("CN=ROOT")); | ||
| builder1.withThisUpdateTime(THIS_UPDATE_TIME); | ||
| builder1.withNextUpdateTime(NEXT_UPDATE_TIME); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
Yeah, makes sense. |
src/test/java/net/ripe/rpki/commons/crypto/cms/manifest/ManifestCmsBuilderTest.java
Show resolved
Hide resolved
lolepezy
force-pushed
the
add-validity-period-to-mft-crl-builders
branch
from
753822b
to
61db747
Compare
lolepezy
dismissed
bjpbakker’s stale review
Because it's already done and I don't get how GH UI works
This is to avoid mistakes with setting validity dates and getting them backwards (learned the hard way)