Skip to content

Commit 2bda44c

Browse files
committed
Resolve review feedback & upgrade to OkHttp3 client
This commit addresses and resolves all outstanding review comments, primarily encompassing a shift to OkHttp3, security configuration cleanup, and general code improvements. * **Review:** Addressed and resolved all pending review comments. * **Feature:** Switched the underlying HTTP client implementation to [OkHttp3](https://square.github.io/okhttp/). * **Configuration:** Consolidated TLS Certificates-related configuration into the dedicated configuration area. * **Cleanup:** Removed unused components (`HttpUtils` and `HttpModule`) and performed general code cleanup. * **Quality:** Improved exception handling logic for better robustness.
1 parent a1bf10d commit 2bda44c

File tree

17 files changed

+755
-697
lines changed

17 files changed

+755
-697
lines changed

core/src/main/java/google/registry/config/RegistryConfig.java

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
import google.registry.bsa.UploadBsaUnavailableDomainsAction;
3737
import google.registry.dns.ReadDnsRefreshRequestsAction;
3838
import google.registry.model.common.DnsRefreshRequest;
39-
import google.registry.mosapi.client.MosApiClient;
39+
import google.registry.mosapi.MosApiClient;
4040
import google.registry.persistence.transaction.JpaTransactionManager;
4141
import google.registry.request.Action.Service;
4242
import google.registry.util.RegistryEnvironment;
@@ -50,7 +50,6 @@
5050
import java.lang.annotation.Retention;
5151
import java.net.URI;
5252
import java.net.URL;
53-
import java.util.List;
5453
import java.util.Map.Entry;
5554
import java.util.Optional;
5655
import java.util.function.Supplier;
@@ -1423,9 +1422,9 @@ public static String provideBsaUploadUnavailableDomainsUrl(RegistryConfigSetting
14231422
* @see MosApiClient
14241423
*/
14251424
@Provides
1426-
@Config("mosapiUrl")
1427-
public static String provideMosapiUrl(RegistryConfigSettings config) {
1428-
return config.mosapi.mosapiUrl;
1425+
@Config("mosapiServiceUrl")
1426+
public static String provideMosapiServiceUrl(RegistryConfigSettings config) {
1427+
return config.mosapi.serviceUrl;
14291428
}
14301429

14311430
/**
@@ -1434,21 +1433,33 @@ public static String provideMosapiUrl(RegistryConfigSettings config) {
14341433
* @see MosApiClient
14351434
*/
14361435
@Provides
1437-
@Config("entityType")
1436+
@Config("mosapiEntityType")
14381437
public static String provideMosapiEntityType(RegistryConfigSettings config) {
14391438
return config.mosapi.entityType;
14401439
}
14411440

1441+
@Provides
1442+
@Config("mosapiTlsCertSecretName")
1443+
public static String provideMosapiTlsCertSecretName(RegistryConfigSettings config) {
1444+
return config.mosapi.tlsCertSecretName;
1445+
}
1446+
1447+
@Provides
1448+
@Config("mosapiTlsCertKeyName")
1449+
public static String provideMosapiTlsKeySecretName(RegistryConfigSettings config) {
1450+
return config.mosapi.tlsKeySecretName;
1451+
}
1452+
14421453
@Provides
14431454
@Config("mosapiTlds")
1444-
public static List<String> provideMosapiTlds(RegistryConfigSettings config) {
1445-
return ImmutableList.copyOf(config.mosapi.tlds);
1455+
public static ImmutableSet<String> provideMosapiTlds(RegistryConfigSettings config) {
1456+
return ImmutableSet.copyOf(config.mosapi.tlds);
14461457
}
14471458

14481459
@Provides
14491460
@Config("mosapiServices")
1450-
public static List<String> provideMosapiServices(RegistryConfigSettings config) {
1451-
return ImmutableList.copyOf(config.mosapi.services);
1461+
public static ImmutableSet<String> provideMosapiServices(RegistryConfigSettings config) {
1462+
return ImmutableSet.copyOf(config.mosapi.services);
14521463
}
14531464

14541465
private static String formatComments(String text) {

core/src/main/java/google/registry/config/RegistryConfigSettings.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,9 @@ public static class Bsa {
266266

267267
/** Configuration for Mosapi. */
268268
public static class MosApi {
269-
public String mosapiUrl;
269+
public String serviceUrl;
270+
public String tlsCertSecretName;
271+
public String tlsKeySecretName;
270272
public String entityType;
271273
public List<String> tlds;
272274
public List<String> services;

core/src/main/java/google/registry/config/files/default-config.yaml

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -619,11 +619,21 @@ bsa:
619619

620620
mosapi:
621621
# URL for the MosAPI
622-
mosapiUrl: https://mosapi.icann.org
622+
serviceUrl: https://mosapi.icann.org
623+
# The type of entity being monitored.
624+
# For registries, this is 'ry'
625+
# For registrars, this is 'rr'
623626
entityType: ry
624-
# List of TLDs to be monitored.
627+
# Add your List of TLDs to be monitored
625628
tlds:
626-
- "test"
629+
- YOUR_TLD1
630+
- YOUR_TLD2
631+
# Add tls cert secret name
632+
# you configured in secret manager
633+
tlsCertSecretName: YOUR_TLS_CERT_SECRET_NAME
634+
# Add tls key secret name
635+
# you configured in secret manager
636+
tlsKeySecretName: YOUR_TLS_KEY_SECRET_NAME
627637
# List of services to check for each TLD.
628638
services:
629639
- "dns"

core/src/main/java/google/registry/module/RegistryComponent.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050
import google.registry.request.RequestHandler;
5151
import google.registry.request.auth.AuthModule;
5252
import google.registry.request.auth.RequestAuthenticator;
53-
import google.registry.util.HttpModule;
5453
import google.registry.util.UtilsModule;
5554
import jakarta.inject.Provider;
5655
import jakarta.inject.Singleton;
@@ -73,7 +72,6 @@
7372
GroupsModule.class,
7473
GroupssettingsModule.class,
7574
GsonModule.class,
76-
HttpModule.class,
7775
MosApiModule.class,
7876
JSchModule.class,
7977
KeyModule.class,

core/src/main/java/google/registry/mosapi/client/MosApiClient.java renamed to core/src/main/java/google/registry/mosapi/MosApiClient.java

Lines changed: 55 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -4,44 +4,50 @@
44
// you may not use this file except in compliance with the License.
55
// You may obtain a copy of the License at
66
//
7-
// http://www.apache.org/licenses/LICENSE-2.0
7+
// http://www.apache.org/licenses/LICENSE-2.0
88
//
99
// Unless required by applicable law or agreed to in writing, software
1010
// distributed under the License is distributed on an "AS IS" BASIS,
1111
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
package google.registry.mosapi.client;
15+
package google.registry.mosapi;
1616

1717
import google.registry.config.RegistryConfig.Config;
1818
import google.registry.mosapi.exception.MosApiException;
1919
import google.registry.mosapi.exception.MosApiException.MosApiAuthorizationException;
20-
import google.registry.util.HttpUtils;
2120
import jakarta.inject.Inject;
2221
import jakarta.inject.Named;
2322
import jakarta.inject.Singleton;
23+
import java.io.IOException;
2424
import java.net.HttpURLConnection;
25-
import java.net.URLEncoder;
26-
import java.net.http.HttpClient;
27-
import java.net.http.HttpResponse;
28-
import java.nio.charset.StandardCharsets;
2925
import java.util.Map;
30-
import java.util.stream.Collectors;
26+
import okhttp3.HttpUrl;
27+
import okhttp3.MediaType;
28+
import okhttp3.OkHttpClient;
29+
import okhttp3.Request;
30+
import okhttp3.RequestBody;
31+
import okhttp3.Response;
3132

3233
@Singleton
3334
public class MosApiClient {
3435

35-
private final HttpClient httpClient;
36+
private final OkHttpClient httpClient;
3637
private final String baseUrl;
3738

3839
@Inject
3940
public MosApiClient(
40-
@Named("mosapiHttpClient") HttpClient httpClient,
41-
@Config("mosapiUrl") String mosapiUrl,
42-
@Config("entityType") String entityType) {
41+
@Named("mosapiHttpClient") OkHttpClient httpClient,
42+
@Config("mosapiServiceUrl") String mosapiUrl,
43+
@Config("mosapiEntityType") String entityType) {
4344
this.httpClient = httpClient;
44-
this.baseUrl = String.format("%s/%s", mosapiUrl, entityType);
45+
// Pre-calculate base URL and validate it to fail fast on bad config
46+
String fullUrl = String.format("%s/%s", mosapiUrl, entityType);
47+
if (HttpUrl.parse(fullUrl) == null) {
48+
throw new IllegalArgumentException("Invalid MoSAPI Service URL configuration: " + fullUrl);
49+
}
50+
this.baseUrl = fullUrl;
4551
}
4652

4753
/**
@@ -51,45 +57,27 @@ public MosApiClient(
5157
* @param endpoint The specific API endpoint path (e.g., "v2/monitoring/state").
5258
* @param params A map of query parameters to be URL-encoded and appended to the request.
5359
* @param headers A map of HTTP headers to be included in the request.
54-
* @return The {@link HttpResponse} from the server if the request is successful.
60+
* @return The {@link Response} from the server if the request is successful. <b>The caller is
61+
* responsible for closing this response.</b>
5562
* @throws MosApiException if the request fails due to a network error or an unhandled HTTP
5663
* status.
5764
* @throws MosApiAuthorizationException if the server returns a 401 Unauthorized status.
5865
*/
59-
public HttpResponse<String> sendGetRequest(
60-
String entityId, String endpoint, Map<String, String> params, Map<String, String> headers)
61-
throws MosApiException {
62-
String url = buildUrl(entityId, endpoint, params);
63-
try {
64-
HttpResponse<String> response = HttpUtils.sendGetRequest(httpClient, url, headers);
65-
return checkResponseForAuthError(response);
66-
} catch (RuntimeException e) {
67-
throw new MosApiException("Error during GET request to " + url, e);
68-
}
69-
}
70-
71-
/**
72-
* Sends a GET request and decompresses the GZIP-encoded response body.
73-
*
74-
* @param entityId The TLD or registrar ID the request is for.
75-
* @param endpoint The specific API endpoint path.
76-
* @param params A map of query parameters to be URL-encoded.
77-
* @param headers A map of HTTP headers to be included in the request.
78-
* @return The decompressed {@link HttpResponse} from the server.
79-
* @throws MosApiException if the request fails.
80-
* @throws MosApiAuthorizationException if the server returns a 401 Unauthorized status.
81-
*/
82-
public HttpResponse<String> sendGetRequestWithDecompression(
66+
public Response sendGetRequest(
8367
String entityId, String endpoint, Map<String, String> params, Map<String, String> headers)
8468
throws MosApiException {
85-
String url = buildUrl(entityId, endpoint, params);
69+
HttpUrl url = buildUri(entityId, endpoint, params);
70+
Request.Builder requestBuilder = new Request.Builder().url(url).get();
71+
headers.forEach(requestBuilder::addHeader);
8672
try {
87-
HttpResponse<String> response =
88-
HttpUtils.sendGetRequestWithDecompression(httpClient, url, headers);
73+
Response response = httpClient.newCall(requestBuilder.build()).execute();
8974
return checkResponseForAuthError(response);
90-
75+
} catch (MosApiAuthorizationException e) {
76+
throw e;
9177
} catch (RuntimeException e) {
9278
throw new MosApiException("Error during GET request to " + url, e);
79+
} catch (IOException e) {
80+
throw new MosApiException("IOException during GET request to " + url, e);
9381
}
9482
}
9583

@@ -104,29 +92,39 @@ public HttpResponse<String> sendGetRequestWithDecompression(
10492
* @param params A map of query parameters to be URL-encoded.
10593
* @param headers A map of HTTP headers to be included in the request.
10694
* @param body The request body to be sent with the POST request.
107-
* @return The {@link HttpResponse} from the server.
95+
* @return The {@link Response} from the server. <b>The caller is responsible for closing this
96+
* response.</b>
10897
* @throws MosApiException if the request fails.
10998
* @throws MosApiAuthorizationException if the server returns a 401 Unauthorized status.
11099
*/
111-
public HttpResponse<String> sendPostRequest(
100+
public Response sendPostRequest(
112101
String entityId,
113102
String endpoint,
114103
Map<String, String> params,
115104
Map<String, String> headers,
116105
String body)
117106
throws MosApiException {
118-
String url = buildUrl(entityId, endpoint, params);
107+
HttpUrl url = buildUri(entityId, endpoint, params);
108+
RequestBody requestBody = RequestBody.create(body, MediaType.parse("application/json"));
109+
110+
Request.Builder requestBuilder = new Request.Builder().url(url).post(requestBody);
111+
headers.forEach(requestBuilder::addHeader);
119112
try {
120-
HttpResponse<String> response = HttpUtils.sendPostRequest(httpClient, url, headers, body);
113+
Response response = httpClient.newCall(requestBuilder.build()).execute();
121114
return checkResponseForAuthError(response);
115+
} catch (MosApiAuthorizationException e) {
116+
throw e;
122117
} catch (RuntimeException e) {
123118
throw new MosApiException("Error during POST request to " + url, e);
119+
} catch (IOException e) {
120+
throw new MosApiException("IOException during POST request to " + url, e);
124121
}
125122
}
126123

127-
private HttpResponse<String> checkResponseForAuthError(HttpResponse<String> response)
124+
private Response checkResponseForAuthError(Response response)
128125
throws MosApiAuthorizationException {
129-
if (response.statusCode() == HttpURLConnection.HTTP_UNAUTHORIZED) {
126+
if (response.code() == HttpURLConnection.HTTP_UNAUTHORIZED) {
127+
response.close();
130128
throw new MosApiAuthorizationException(
131129
"Authorization failed for the requested resource. The client certificate may not be"
132130
+ " authorized for the specified TLD or Registrar.");
@@ -137,21 +135,16 @@ private HttpResponse<String> checkResponseForAuthError(HttpResponse<String> resp
137135
/**
138136
* Builds the full URL for a request, including the base URL, entityId, path, and query params.
139137
*/
140-
private String buildUrl(String entityId, String path, Map<String, String> queryParams) {
141-
String sanitizedPath = path.startsWith("/") ? path : "/" + path;
142-
String fullPath = "/" + entityId + sanitizedPath;
138+
private HttpUrl buildUri(String entityId, String path, Map<String, String> queryParams) {
139+
String sanitizedPath = path.startsWith("/") ? path.substring(1) : path;
140+
141+
// We can safely use get() here because we validated baseUrl in the constructor
142+
HttpUrl.Builder urlBuilder =
143+
HttpUrl.get(baseUrl).newBuilder().addPathSegment(entityId).addPathSegments(sanitizedPath);
143144

144-
if (queryParams == null || queryParams.isEmpty()) {
145-
return baseUrl + fullPath;
145+
if (queryParams != null) {
146+
queryParams.forEach(urlBuilder::addQueryParameter);
146147
}
147-
String queryString =
148-
queryParams.entrySet().stream()
149-
.map(
150-
entry ->
151-
entry.getKey()
152-
+ "="
153-
+ URLEncoder.encode(entry.getValue(), StandardCharsets.UTF_8))
154-
.collect(Collectors.joining("&"));
155-
return baseUrl + fullPath + "?" + queryString;
148+
return urlBuilder.build();
156149
}
157150
}
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// Copyright 2024 The Nomulus Authors. All Rights Reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package google.registry.mosapi;
16+
17+
import java.util.Arrays;
18+
import java.util.Map;
19+
import java.util.Optional;
20+
import java.util.function.Function;
21+
import java.util.stream.Collectors;
22+
23+
/**
24+
* Represents known MoSAPI API result codes and their default messages.
25+
*
26+
* <p>The definitions for these codes can be found in the official ICANN MoSAPI Specification,
27+
* specifically in the 'Result Codes' section.
28+
*
29+
* @see <a href="https://www.icann.org/mosapi-specification.pdf">ICANN MoSAPI Specification</a>
30+
*/
31+
public enum MosApiResponse {
32+
DATE_DURATION_INVALID(
33+
"2011", "The difference between endDate and startDate is" + "more than 31 days"),
34+
DATE_ORDER_INVALID("2012", "The EndDate is before startDate"),
35+
START_DATE_SYNTAX_INVALID("2013", "StartDate syntax is invalid"),
36+
END_DATE_SYNTAX_INVALID("2014", "EndDate syntax is invalid");
37+
38+
private final String code;
39+
private final String defaultMessage;
40+
41+
private static final Map<String, MosApiResponse> CODE_MAP =
42+
Arrays.stream(values()).collect(Collectors.toMap(e -> e.code, Function.identity()));
43+
44+
MosApiResponse(String code, String defaultMessage) {
45+
this.code = code;
46+
this.defaultMessage = defaultMessage;
47+
}
48+
49+
public String getCode() {
50+
return code;
51+
}
52+
53+
public String getDefaultMessage() {
54+
return defaultMessage;
55+
}
56+
57+
// Returns the enum constant associated with the given result code string
58+
public static Optional<MosApiResponse> fromCode(String code) {
59+
60+
return Optional.ofNullable(CODE_MAP.get(code));
61+
}
62+
}

0 commit comments

Comments
 (0)