Skip to content

Commit

Permalink
Remove java.util.Date
Browse files Browse the repository at this point in the history
There is one remaining instance in JpaTransactionManagerImpl that cannot
be removed because DetachingTypedQuery is implementing TypedQuery, which has
a method that expectred java.util.Date.
  • Loading branch information
jianglai committed Mar 14, 2024
1 parent b9cfa65 commit 8aa48af
Show file tree
Hide file tree
Showing 12 changed files with 98 additions and 142 deletions.
11 changes: 9 additions & 2 deletions config/presubmits.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,17 +173,24 @@ def fails(self, file):
):
"JavaScript files should not include console logging.",
PresubmitCheck(
r"org\.testcontainers\.shaded\.",
r".*org\.testcontainers\.shaded\..*",
"java",
{"/node_modules/"},
):
"Do not use shaded dependencies from testcontainers.",
PresubmitCheck(
r"com\.google\.common\.truth\.Truth8",
r".*com\.google\.common\.truth\.Truth8.*",
"java",
{"/node_modules/"},
):
"Truth8 is deprecated. Use Truth instead.",
PresubmitCheck(
r".*java\.util\.Date.*",
"java",
{"/node_modules/", "JpaTransactionManagerImpl.java"},
):
"Do not use java.util.Date. Use classes in Java.date package instead.",

}

# Note that this regex only works for one kind of Flyway file. If we want to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import java.math.BigDecimal;
import java.time.Duration;
import java.util.Collection;
import java.util.Date;
import java.util.Map;
import java.util.ServiceLoader;
import org.apache.commons.codec.binary.Base64;
Expand Down Expand Up @@ -195,10 +194,7 @@ public AccessToken refreshAccessToken() throws IOException {
GenericData responseData = response.parseAs(GenericData.class);
String accessToken = validateString(responseData, "access_token", PARSE_ERROR_PREFIX);
int expiresInSeconds = validateInt32(responseData, "expires_in", PARSE_ERROR_PREFIX);
long expiresAtMilliseconds = clock.nowUtc().getMillis() + expiresInSeconds * 1000L;
@SuppressWarnings("JavaUtilDate")
AccessToken token = new AccessToken(accessToken, new Date(expiresAtMilliseconds));
return token;
return new AccessToken(accessToken, clock.nowUtc().plusSeconds(expiresInSeconds).toDate());
}

String createAssertion(JsonFactory jsonFactory, long currentTime) throws IOException {
Expand Down Expand Up @@ -257,8 +253,7 @@ static int validateInt32(Map<String, Object> map, String key, String errorPrefix
if (value == null) {
throw new IOException(String.format(VALUE_NOT_FOUND_MESSAGE, errorPrefix, key));
}
if (value instanceof BigDecimal) {
BigDecimal bigDecimalValue = (BigDecimal) value;
if (value instanceof BigDecimal bigDecimalValue) {
return bigDecimalValue.intValueExact();
}
if (!(value instanceof Integer)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import java.security.cert.X509Certificate;
import java.security.interfaces.ECPublicKey;
import java.security.interfaces.RSAPublicKey;
import java.util.Date;
import java.util.stream.Collectors;
import javax.inject.Inject;
import org.bouncycastle.jcajce.provider.asymmetric.util.EC5Util;
Expand Down Expand Up @@ -106,12 +105,9 @@ private static boolean checkCurveName(PublicKey key, ImmutableSet<String> allowe
// These 2 different instances of PublicKey need to be handled separately since their OIDs are
// encoded differently. More details on this can be found at
// https://stackoverflow.com/questions/49895713/how-to-find-the-matching-curve-name-from-an-ecpublickey.
if (key instanceof ECPublicKey) {
ECPublicKey ecKey = (ECPublicKey) key;
if (key instanceof ECPublicKey ecKey) {
params = EC5Util.convertSpec(ecKey.getParams());
} else if (key instanceof org.bouncycastle.jce.interfaces.ECPublicKey) {
org.bouncycastle.jce.interfaces.ECPublicKey ecKey =
(org.bouncycastle.jce.interfaces.ECPublicKey) key;
} else if (key instanceof org.bouncycastle.jce.interfaces.ECPublicKey ecKey) {
params = ecKey.getParameters();
} else {
throw new IllegalArgumentException("Unrecognized instance of PublicKey.");
Expand Down Expand Up @@ -148,7 +144,7 @@ private void handleCertViolations(ImmutableSet<CertificateViolation> violations)
if (!violations.isEmpty()) {
String displayMessages =
violations.stream()
.map(violation -> getViolationDisplayMessage(violation))
.map(this::getViolationDisplayMessage)
.collect(Collectors.joining("\n"));
throw new InsecureCertificateException(violations, displayMessages);
}
Expand All @@ -162,7 +158,7 @@ public ImmutableSet<CertificateViolation> checkCertificate(X509Certificate certi
ImmutableSet.Builder<CertificateViolation> violations = new ImmutableSet.Builder<>();

// Check if currently in validity period
Date now = clock.nowUtc().toDate();
DateTime now = clock.nowUtc();
if (DateTimeComparator.getInstance().compare(certificate.getNotAfter(), now) < 0) {
violations.add(CertificateViolation.EXPIRED);
} else if (DateTimeComparator.getInstance().compare(certificate.getNotBefore(), now) > 0) {
Expand Down Expand Up @@ -231,13 +227,13 @@ public boolean shouldReceiveExpiringNotification(
DateTime lastExpiringNotificationSentDate, String certificateStr) {
X509Certificate certificate = getCertificate(certificateStr);
DateTime now = clock.nowUtc();
// expiration date is one day after lastValidDate
// the expiration date is one day after lastValidDate
DateTime lastValidDate = new DateTime(certificate.getNotAfter());
if (lastValidDate.isBefore(now)) {
return false;
}
/*
* Client should receive a notification if :
* Client should receive a notification if:
* 1) client has never received notification (lastExpiringNotificationSentDate is initially
* set to START_OF_TIME) and the certificate has entered the expiring period, OR
* 2) client has received notification but the interval between now and
Expand All @@ -254,29 +250,21 @@ private String getViolationDisplayMessage(CertificateViolation certificateViolat
// Yes, we'd rather do this as an instance method on the CertificateViolation enum itself, but
// we can't because we need access to configuration (injected as instance variables) which you
// can't get in a static enum context.
switch (certificateViolation) {
case EXPIRED:
return "Certificate is expired.";
case NOT_YET_VALID:
return "Certificate start date is in the future.";
case ALGORITHM_CONSTRAINED:
return "Certificate key algorithm must be RSA or ECDSA.";
case RSA_KEY_LENGTH_TOO_SHORT:
return String.format(
"RSA key length is too short; the minimum allowed length is %d bits.",
this.minimumRsaKeyLength);
case VALIDITY_LENGTH_TOO_LONG:
return String.format(
"Certificate validity period is too long; it must be less than or equal to %d days.",
this.maxValidityLengthSchedule.lastEntry().getValue());
case INVALID_ECDSA_CURVE:
return String.format(
"The ECDSA key must use one of these algorithms: %s", allowedEcdsaCurves);
default:
throw new IllegalArgumentException(
String.format(
"Unknown CertificateViolation enum value: %s", certificateViolation.name()));
}
return switch (certificateViolation) {
case EXPIRED -> "Certificate is expired.";
case NOT_YET_VALID -> "Certificate start date is in the future.";
case ALGORITHM_CONSTRAINED -> "Certificate key algorithm must be RSA or ECDSA.";
case RSA_KEY_LENGTH_TOO_SHORT ->
String.format(
"RSA key length is too short; the minimum allowed length is %d bits.",
this.minimumRsaKeyLength);
case VALIDITY_LENGTH_TOO_LONG ->
String.format(
"Certificate validity period is too long; it must be less than or equal to %d days.",
this.maxValidityLengthSchedule.lastEntry().getValue());
case INVALID_ECDSA_CURVE ->
String.format("The ECDSA key must use one of these algorithms: %s", allowedEcdsaCurves);
};
}

/**
Expand All @@ -295,7 +283,7 @@ public enum CertificateViolation {
* Gets a suitable end-user-facing display message for this particular certificate violation.
*
* <p>Note that the {@link CertificateChecker} instance must be passed in because it contains
* configuration values (e.g. minimum RSA key length) that go into the error message text.
* configuration values (e.g., minimum RSA key length) that go into the error message text.
*/
public String getDisplayMessage(CertificateChecker certificateChecker) {
return certificateChecker.getViolationDisplayMessage(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ private static ImmutableMap<TmchCaMode, X509Certificate> loadRootCertificates()
*/
public void verify(X509Certificate cert) throws GeneralSecurityException {
synchronized (TmchCertificateAuthority.class) {
X509Utils.verifyCertificate(getAndValidateRoot(), getCrl(), cert, clock.nowUtc().toDate());
X509Utils.verifyCertificate(getAndValidateRoot(), getCrl(), cert, clock.nowUtc());
}
}

Expand All @@ -151,7 +151,7 @@ public void updateCrl(String asciiCrl, String url) throws GeneralSecurityExcepti
} catch (Exception e) {
logger.atWarning().withCause(e).log("Old CRL is invalid, ignored during CRL update.");
}
X509Utils.verifyCrl(getAndValidateRoot(), oldCrl, newCrl, clock.nowUtc().toDate());
X509Utils.verifyCrl(getAndValidateRoot(), oldCrl, newCrl, clock.nowUtc());
TmchCrl.set(asciiCrl, url);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import google.registry.model.domain.token.AllocationToken.TokenType;
import google.registry.model.domain.token.BulkPricingPackage;
import google.registry.persistence.VKey;
import java.util.Date;
import google.registry.tools.params.DateTimeParameter;
import java.util.List;
import java.util.Optional;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -56,9 +56,11 @@ abstract class CreateOrUpdateBulkPricingPackageCommand extends MutatingCommand {
@Nullable
@Parameter(
names = "--next_billing_date",
converter = DateTimeParameter.class,
validateWith = DateTimeParameter.class,
description =
"The next date that the bulk pricing package should be billed for its annual fee")
Date nextBillingDate;
DateTime nextBillingDate;

/** Returns the existing BulkPricingPackage or null if it does not exist. */
@Nullable
Expand Down Expand Up @@ -105,9 +107,7 @@ protected final void init() throws Exception {
Optional.ofNullable(maxCreates).ifPresent(builder::setMaxCreates);
Optional.ofNullable(price).ifPresent(builder::setBulkPrice);
Optional.ofNullable(nextBillingDate)
.ifPresent(
nextBillingDate ->
builder.setNextBillingDate(new DateTime(nextBillingDate)));
.ifPresent(nextBillingDate -> builder.setNextBillingDate(nextBillingDate));
if (clearLastNotificationSent()) {
builder.setLastNotificationSent(null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import google.registry.module.backend.BackendRequestComponent;
import google.registry.module.bsa.BsaRequestComponent;
import google.registry.module.frontend.FrontendRequestComponent;
Expand All @@ -28,7 +29,6 @@
import java.util.ArrayList;
import java.util.List;
import org.junit.jupiter.api.Test;
import org.testcontainers.shaded.com.google.common.collect.ImmutableMap;

/** Unit tests for {@link RequestComponent}. */
public class RequestComponentTest {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import com.google.common.collect.Multimaps;
import com.google.common.net.HttpHeaders;
import com.google.common.net.MediaType;
import com.google.common.truth.Truth8;
import com.google.protobuf.Timestamp;
import com.google.protobuf.util.Timestamps;
import dagger.Module;
Expand Down Expand Up @@ -133,7 +132,7 @@ public List<Task> getTestTasksFor(String queue) {
public void assertTasksEnqueuedWithProperty(
String queueName, Function<Task, String> propertyGetter, String... expectedTaskProperties) {
// Ordering is irrelevant but duplicates should be considered independently.
Truth8.assertThat(getTestTasksFor(queueName).stream().map(propertyGetter))
assertThat(getTestTasksFor(queueName).stream().map(propertyGetter))
.containsExactly((Object[]) expectedTaskProperties);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,10 @@
import java.security.cert.CertificateExpiredException;
import java.security.cert.CertificateNotYetValidException;
import java.security.cert.X509Certificate;
import java.time.Duration;
import java.time.Instant;
import java.util.Date;
import java.util.stream.Stream;
import javax.net.ssl.SSLException;
import javax.net.ssl.SSLSession;
import org.joda.time.DateTime;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
Expand All @@ -62,7 +60,7 @@
* the overhead of routing traffic through the network layer, even if it were to go through
* loopback. It also alleviates the need to pick a free port to use.
*
* <p>The local addresses used in each test method must to be different, otherwise tests run in
* <p>The local addresses used in each test method must be different, otherwise tests run in
* parallel may interfere with each other.
*/
class SslClientInitializerTest {
Expand Down Expand Up @@ -204,15 +202,15 @@ void testSuccess_customTrustManager_acceptCertSignedByTrustedCa(SslProvider sslP
// Generate a new key pair.
KeyPair keyPair = getKeyPair();

// Generate a self signed certificate, and use it to sign the key pair.
// Generate a self-signed certificate, and use it to sign the key pair.
SelfSignedCaCertificate ssc = SelfSignedCaCertificate.create();
X509Certificate cert = signKeyPair(ssc, keyPair, SSL_HOST);

// Set up the server to use the signed cert and private key to perform handshake;
PrivateKey privateKey = keyPair.getPrivate();
nettyExtension.setUpServer(localAddress, getServerHandler(false, privateKey, cert));

// Set up the client to trust the self signed cert used to sign the cert that server provides.
// Set up the client to trust the self-signed cert used to sign the cert that server provides.
SslClientInitializer<LocalChannel> sslClientInitializer =
new SslClientInitializer<>(
sslProvider,
Expand All @@ -239,21 +237,17 @@ void testFailure_customTrustManager_serverCertExpired(SslProvider sslProvider) t
// Generate a new key pair.
KeyPair keyPair = getKeyPair();

// Generate a self signed certificate, and use it to sign the key pair.
// Generate a self-signed certificate, and use it to sign the key pair.
SelfSignedCaCertificate ssc = SelfSignedCaCertificate.create();
X509Certificate cert =
signKeyPair(
ssc,
keyPair,
SSL_HOST,
Date.from(Instant.now().minus(Duration.ofDays(2))),
Date.from(Instant.now().minus(Duration.ofDays(1))));
ssc, keyPair, SSL_HOST, DateTime.now().minusDays(2), DateTime.now().minusDays(1));

// Set up the server to use the signed cert and private key to perform handshake;
PrivateKey privateKey = keyPair.getPrivate();
nettyExtension.setUpServer(localAddress, getServerHandler(false, privateKey, cert));

// Set up the client to trust the self signed cert used to sign the cert that server provides.
// Set up the client to trust the self-signed cert used to sign the cert that server provides.
SslClientInitializer<LocalChannel> sslClientInitializer =
new SslClientInitializer<>(
sslProvider,
Expand All @@ -280,21 +274,16 @@ void testFailure_customTrustManager_serverCertNotYetValid(SslProvider sslProvide
// Generate a new key pair.
KeyPair keyPair = getKeyPair();

// Generate a self signed certificate, and use it to sign the key pair.
// Generate a self-signed certificate, and use it to sign the key pair.
SelfSignedCaCertificate ssc = SelfSignedCaCertificate.create();
X509Certificate cert =
signKeyPair(
ssc,
keyPair,
SSL_HOST,
Date.from(Instant.now().plus(Duration.ofDays(1))),
Date.from(Instant.now().plus(Duration.ofDays(2))));
signKeyPair(ssc, keyPair, SSL_HOST, DateTime.now().plusDays(1), DateTime.now().plusDays(2));

// Set up the server to use the signed cert and private key to perform handshake;
PrivateKey privateKey = keyPair.getPrivate();
nettyExtension.setUpServer(localAddress, getServerHandler(false, privateKey, cert));

// Set up the client to trust the self signed cert used to sign the cert that server provides.
// Set up the client to trust the self-signed cert used to sign the cert that server provides.
SslClientInitializer<LocalChannel> sslClientInitializer =
new SslClientInitializer<>(
sslProvider,
Expand Down Expand Up @@ -333,7 +322,7 @@ void testSuccess_customTrustManager_acceptSelfSignedCert_clientCertRequired(
SslClientInitializerTest::hostProvider,
SslClientInitializerTest::portProvider,
ImmutableList.of(serverSsc.cert()),
() -> clientSsc.key(),
clientSsc::key,
() -> ImmutableList.of(clientSsc.cert()));
nettyExtension.setUpClient(localAddress, sslClientInitializer);

Expand All @@ -360,15 +349,15 @@ void testFailure_customTrustManager_wrongHostnameInCertificate(SslProvider sslPr
// Generate a new key pair.
KeyPair keyPair = getKeyPair();

// Generate a self signed certificate, and use it to sign the key pair.
// Generate a self-signed certificate, and use it to sign the key pair.
SelfSignedCaCertificate ssc = SelfSignedCaCertificate.create();
X509Certificate cert = signKeyPair(ssc, keyPair, "wrong.com");

// Set up the server to use the signed cert and private key to perform handshake;
PrivateKey privateKey = keyPair.getPrivate();
nettyExtension.setUpServer(localAddress, getServerHandler(false, privateKey, cert));

// Set up the client to trust the self signed cert used to sign the cert that server provides.
// Set up the client to trust the self-signed cert used to sign the cert that server provides.
SslClientInitializer<LocalChannel> sslClientInitializer =
new SslClientInitializer<>(
sslProvider,
Expand All @@ -379,7 +368,7 @@ void testFailure_customTrustManager_wrongHostnameInCertificate(SslProvider sslPr
null);
nettyExtension.setUpClient(localAddress, sslClientInitializer);

// When the client rejects the server cert due to wrong hostname, both the client and server
// When the client rejects the server cert due to the wrong hostname, both the client and server
// should throw exceptions.
nettyExtension.assertThatClientRootCause().isInstanceOf(CertificateException.class);
nettyExtension.assertThatClientRootCause().hasMessageThat().contains(SSL_HOST);
Expand Down
Loading

0 comments on commit 8aa48af

Please sign in to comment.