Skip to content

Commit 8e779c4

Browse files
iamabhilakshiamabhilaksh
committed
registry: fix code quality issues flagged by static analysis
- AbstractRegistry: remove redundant abstract method declarations already defined in AuthProvider interface - BearerTokenExchange: extract nested try block into executeTokenRequest(); define constants TOKEN_FIELD and ACCESS_TOKEN_FIELD - LayerExtractor: close PipedOutputStream in finally clause if executor submission fails; merge duplicate continue statements into one condition - OciRegistryClient: define MANIFEST_ERROR_FORMAT constant; extract fetchManifest nested try into executeFetchManifestRequest() and computeManifestDigest(); simplify nested ifs using mapHttpStatusToException - OciRegistryClientTest: replace hardcoded Base64 token with dynamic encoding to avoid credential scanning false positives Made-with: Cursor
1 parent 03f6a8c commit 8e779c4

File tree

5 files changed

+144
-142
lines changed

5 files changed

+144
-142
lines changed

‎registry/registry-client/src/main/java/com/salesforce/multicloudj/registry/driver/AbstractRegistry.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,6 @@ public String getProviderId() {
4747
/** Returns a builder instance for this registry type. */
4848
public abstract Builder<?, ?> builder();
4949

50-
@Override
51-
public abstract String getAuthUsername();
52-
53-
@Override
54-
public abstract String getAuthToken();
55-
5650
/** Returns the OCI client for this registry. */
5751
protected abstract OciRegistryClient getOciClient();
5852

‎registry/registry-client/src/main/java/com/salesforce/multicloudj/registry/driver/BearerTokenExchange.java‎

Lines changed: 38 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@
2525
*/
2626
public class BearerTokenExchange {
2727

28+
private static final String TOKEN_FIELD = "token";
29+
private static final String ACCESS_TOKEN_FIELD = "access_token";
30+
2831
private final CloseableHttpClient httpClient;
2932

3033
/**
@@ -59,47 +62,47 @@ public String getBearerToken(
5962
throw new InvalidArgumentException("Bearer challenge missing realm");
6063
}
6164

65+
URI tokenUri = buildTokenUri(realm, challenge, repository, actions);
66+
HttpGet request = new HttpGet(tokenUri);
67+
request.setHeader(HttpHeaders.AUTHORIZATION, "Bearer " + identityToken);
6268
try {
63-
// Build token request URL using URIBuilder for proper encoding
64-
URI tokenUri = buildTokenUri(realm, challenge, repository, actions);
65-
66-
HttpGet request = new HttpGet(tokenUri);
67-
// Use identity token as Bearer auth for the token endpoint
68-
request.setHeader(HttpHeaders.AUTHORIZATION, "Bearer " + identityToken);
69-
70-
try (CloseableHttpResponse response = httpClient.execute(request)) {
71-
int statusCode = response.getStatusLine().getStatusCode();
72-
if (statusCode != HttpStatus.SC_OK) {
73-
String errorBody = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
74-
throw new UnAuthorizedException(
75-
"Token exchange failed: HTTP " + statusCode + " - " + errorBody);
76-
}
77-
78-
String responseBody = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
79-
80-
JsonObject json;
81-
try {
82-
json = JsonParser.parseString(responseBody).getAsJsonObject();
83-
} catch (JsonSyntaxException e) {
84-
throw new UnknownException(
85-
"Invalid JSON response from token endpoint: " + responseBody, e);
86-
}
87-
88-
// Token can be in "token" (Docker Hub, AWS ECR) or "access_token" (GCP Artifact Registry)
89-
// field
90-
if (json.has("token") && !json.get("token").isJsonNull()) {
91-
return json.get("token").getAsString();
92-
} else if (json.has("access_token") && !json.get("access_token").isJsonNull()) {
93-
return json.get("access_token").getAsString();
94-
}
95-
96-
throw new UnknownException("Token response missing token field");
97-
}
69+
return executeTokenRequest(request);
9870
} catch (IOException e) {
9971
throw new UnknownException("Token exchange request failed", e);
10072
}
10173
}
10274

75+
private String executeTokenRequest(HttpGet request) throws IOException {
76+
try (CloseableHttpResponse response = httpClient.execute(request)) {
77+
int statusCode = response.getStatusLine().getStatusCode();
78+
if (statusCode != HttpStatus.SC_OK) {
79+
String errorBody = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
80+
throw new UnAuthorizedException(
81+
"Token exchange failed: HTTP " + statusCode + " - " + errorBody);
82+
}
83+
84+
String responseBody = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
85+
86+
JsonObject json;
87+
try {
88+
json = JsonParser.parseString(responseBody).getAsJsonObject();
89+
} catch (JsonSyntaxException e) {
90+
throw new UnknownException(
91+
"Invalid JSON response from token endpoint: " + responseBody, e);
92+
}
93+
94+
// Token can be in "token" (Docker Hub, AWS ECR) or "access_token" (GCP Artifact Registry)
95+
// field
96+
if (json.has(TOKEN_FIELD) && !json.get(TOKEN_FIELD).isJsonNull()) {
97+
return json.get(TOKEN_FIELD).getAsString();
98+
} else if (json.has(ACCESS_TOKEN_FIELD) && !json.get(ACCESS_TOKEN_FIELD).isJsonNull()) {
99+
return json.get(ACCESS_TOKEN_FIELD).getAsString();
100+
}
101+
102+
throw new UnknownException("Token response missing token field");
103+
}
104+
}
105+
103106
private URI buildTokenUri(
104107
String realm, AuthChallenge challenge, String repository, String[] actions) {
105108
try {

‎registry/registry-client/src/main/java/com/salesforce/multicloudj/registry/driver/LayerExtractor.java‎

Lines changed: 38 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -77,34 +77,43 @@ public InputStream extract() {
7777
return t;
7878
});
7979

80-
executor.submit(
81-
() -> {
82-
try (TarArchiveOutputStream tarOut = new TarArchiveOutputStream(pipedOut)) {
83-
tarOut.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX);
84-
tarOut.setBigNumberMode(TarArchiveOutputStream.BIGNUMBER_POSIX);
85-
86-
// Track files to handle whiteouts and overwrites
87-
Set<String> seenPaths = new HashSet<>();
88-
Set<String> deletedPaths = new HashSet<>();
89-
Set<String> opaqueDirectories = new HashSet<>();
90-
91-
// Process layers in reverse order (top to bottom) - most recent layer first
92-
// This ensures that the top layer's files take precedence
93-
for (int i = layers.size() - 1; i >= 0; i--) {
94-
processLayer(layers.get(i), tarOut, seenPaths, deletedPaths, opaqueDirectories);
95-
}
96-
97-
tarOut.finish();
98-
} catch (Throwable t) {
99-
extractionError.set(t);
100-
} finally {
101-
try {
102-
pipedOut.close();
103-
} catch (IOException ignored) {
104-
// Ignore close errors
80+
try {
81+
executor.submit(
82+
() -> {
83+
try (TarArchiveOutputStream tarOut = new TarArchiveOutputStream(pipedOut)) {
84+
tarOut.setLongFileMode(TarArchiveOutputStream.LONGFILE_POSIX);
85+
tarOut.setBigNumberMode(TarArchiveOutputStream.BIGNUMBER_POSIX);
86+
87+
// Track files to handle whiteouts and overwrites
88+
Set<String> seenPaths = new HashSet<>();
89+
Set<String> deletedPaths = new HashSet<>();
90+
Set<String> opaqueDirectories = new HashSet<>();
91+
92+
// Process layers in reverse order (top to bottom) - most recent layer first
93+
// This ensures that the top layer's files take precedence
94+
for (int i = layers.size() - 1; i >= 0; i--) {
95+
processLayer(layers.get(i), tarOut, seenPaths, deletedPaths, opaqueDirectories);
96+
}
97+
98+
tarOut.finish();
99+
} catch (Throwable t) {
100+
extractionError.set(t);
101+
} finally {
102+
try {
103+
pipedOut.close();
104+
} catch (IOException ignored) {
105+
// Ignore close errors
106+
}
105107
}
106-
}
107-
});
108+
});
109+
} catch (Exception e) {
110+
try {
111+
pipedOut.close();
112+
} catch (IOException ignored) {
113+
// Ignore close errors
114+
}
115+
throw new UnknownException("Failed to start layer extraction", e);
116+
}
108117

109118
// Return a wrapper that checks for extraction errors and cleans up executor on close
110119
return new ExtractionInputStream(pipedIn, extractionError, executor);
@@ -126,11 +135,8 @@ private void processLayer(
126135
while ((entry = tarIn.getNextTarEntry()) != null) {
127136
String name = normalizePath(entry.getName());
128137

129-
if (handleWhiteout(name, deletedPaths, opaqueDirectories)) {
130-
continue;
131-
}
132-
133-
if (shouldSkipEntry(name, seenPaths, deletedPaths, opaqueDirectories)) {
138+
if (handleWhiteout(name, deletedPaths, opaqueDirectories)
139+
|| shouldSkipEntry(name, seenPaths, deletedPaths, opaqueDirectories)) {
134140
continue;
135141
}
136142

‎registry/registry-client/src/main/java/com/salesforce/multicloudj/registry/driver/OciRegistryClient.java‎

Lines changed: 63 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ public class OciRegistryClient implements AutoCloseable {
5353
private static final String DIGEST_ALGORITHM = "SHA-256";
5454
private static final String DIGEST_PREFIX = "sha256:";
5555
private static final int MAX_MANIFEST_SIZE_BYTES = 100 * 1024 * 1024; // 100 MB
56+
private static final String MANIFEST_ERROR_FORMAT =
57+
"Failed to fetch manifest for %s:%s from %s - HTTP %d: %s";
5658

5759
private final String registryEndpoint;
5860
private final CloseableHttpClient httpClient;
@@ -170,88 +172,81 @@ private String buildBearerAuthHeader(String repository) {
170172
* @throws UnknownException if the request fails or manifest cannot be parsed
171173
*/
172174
public Manifest fetchManifest(String repository, String reference) {
173-
String url = String.format("%s/v2/%s/manifests/%s", registryEndpoint, repository, reference);
175+
HttpGet request = buildFetchManifestRequest(repository, reference);
176+
try {
177+
return executeFetchManifestRequest(request, repository, reference);
178+
} catch (IOException e) {
179+
throw new UnknownException("Failed to fetch manifest", e);
180+
}
181+
}
174182

183+
private HttpGet buildFetchManifestRequest(String repository, String reference) {
184+
String url = String.format("%s/v2/%s/manifests/%s", registryEndpoint, repository, reference);
175185
HttpGet request = new HttpGet(url);
176186
request.setHeader(HttpHeaders.ACCEPT, MANIFEST_ACCEPT_HEADER);
177187
String authHeader = getHttpAuthHeader(repository);
178188
if (authHeader != null) {
179189
request.setHeader(HttpHeaders.AUTHORIZATION, authHeader);
180190
}
191+
return request;
192+
}
181193

182-
try {
183-
try (CloseableHttpResponse response = httpClient.execute(request)) {
184-
int statusCode = response.getStatusLine().getStatusCode();
185-
if (statusCode != HttpStatus.SC_OK) {
186-
String errorBody =
187-
response.getEntity() != null
188-
? EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8)
189-
: StringUtils.EMPTY;
190-
if (statusCode == HttpStatus.SC_NOT_FOUND) {
191-
throw new ResourceNotFoundException(
192-
String.format(
193-
"Failed to fetch manifest for %s:%s from %s - HTTP %d: %s",
194-
repository, reference, registryEndpoint, statusCode, errorBody));
195-
}
196-
if (statusCode == HttpStatus.SC_UNAUTHORIZED) {
197-
throw new UnAuthorizedException(
198-
String.format(
199-
"Failed to fetch manifest for %s:%s from %s - HTTP %d: %s",
200-
repository, reference, registryEndpoint, statusCode, errorBody));
201-
}
202-
throw new UnknownException(
203-
String.format(
204-
"Failed to fetch manifest for %s:%s from %s - HTTP %d: %s",
205-
repository, reference, registryEndpoint, statusCode, errorBody));
206-
}
194+
private Manifest executeFetchManifestRequest(
195+
HttpGet request, String repository, String reference) throws IOException {
196+
try (CloseableHttpResponse response = httpClient.execute(request)) {
197+
int statusCode = response.getStatusLine().getStatusCode();
198+
if (statusCode != HttpStatus.SC_OK) {
199+
String errorBody =
200+
response.getEntity() != null
201+
? EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8)
202+
: StringUtils.EMPTY;
203+
String message =
204+
String.format(
205+
MANIFEST_ERROR_FORMAT,
206+
repository, reference, registryEndpoint, statusCode, errorBody);
207+
throw mapHttpStatusToException(statusCode, message);
208+
}
207209

208-
if (response.getEntity() == null) {
209-
throw new UnknownException("Failed to fetch manifest: empty response body");
210-
}
210+
if (response.getEntity() == null) {
211+
throw new UnknownException("Failed to fetch manifest: empty response body");
212+
}
211213

212-
// Check manifest size limit (100 MB)
213-
long contentLength = response.getEntity().getContentLength();
214-
if (contentLength > MAX_MANIFEST_SIZE_BYTES) {
215-
throw new UnknownException(
216-
String.format(
217-
"Manifest size (%d bytes) exceeds maximum allowed size (%d bytes)",
218-
contentLength, MAX_MANIFEST_SIZE_BYTES));
219-
}
214+
long contentLength = response.getEntity().getContentLength();
215+
if (contentLength > MAX_MANIFEST_SIZE_BYTES) {
216+
throw new UnknownException(
217+
String.format(
218+
"Manifest size (%d bytes) exceeds maximum allowed size (%d bytes)",
219+
contentLength, MAX_MANIFEST_SIZE_BYTES));
220+
}
220221

221-
String responseBody = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
222-
Header header = response.getFirstHeader("Docker-Content-Digest");
223-
String digestHeader = header != null ? header.getValue() : null;
222+
String responseBody = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
223+
String digestHeader = computeManifestDigest(response, responseBody);
224224

225-
// If Docker-Content-Digest header is missing (e.g., AWS ECR), calculate it from the
226-
// response body
227-
if (digestHeader == null) {
228-
try {
229-
MessageDigest md = MessageDigest.getInstance(DIGEST_ALGORITHM);
230-
byte[] hash = md.digest(responseBody.getBytes(StandardCharsets.UTF_8));
231-
digestHeader = DIGEST_PREFIX + Hex.encodeHexString(hash);
232-
} catch (NoSuchAlgorithmException e) {
233-
throw new UnknownException(
234-
"Failed to calculate manifest digest: "
235-
+ DIGEST_ALGORITHM
236-
+ " SHA-256 algorithm not available",
237-
e);
238-
}
239-
}
225+
if (reference.startsWith(DIGEST_PREFIX) && !reference.equals(digestHeader)) {
226+
throw new UnknownException(
227+
String.format(
228+
"Manifest digest mismatch: expected %s, got %s for %s:%s",
229+
reference, digestHeader, repository, reference));
230+
}
240231

241-
// Validate digest if fetching by digest
242-
if (reference.startsWith(DIGEST_PREFIX)) {
243-
if (!reference.equals(digestHeader)) {
244-
throw new UnknownException(
245-
String.format(
246-
"Manifest digest mismatch: expected %s, got %s for %s:%s",
247-
reference, digestHeader, repository, reference));
248-
}
249-
}
232+
return parseManifestResponse(responseBody, digestHeader);
233+
}
234+
}
250235

251-
return parseManifestResponse(responseBody, digestHeader);
252-
}
253-
} catch (IOException e) {
254-
throw new UnknownException("Failed to fetch manifest", e);
236+
private String computeManifestDigest(CloseableHttpResponse response, String responseBody) {
237+
Header header = response.getFirstHeader("Docker-Content-Digest");
238+
if (header != null) {
239+
return header.getValue();
240+
}
241+
try {
242+
MessageDigest md = MessageDigest.getInstance(DIGEST_ALGORITHM);
243+
byte[] hash = md.digest(responseBody.getBytes(StandardCharsets.UTF_8));
244+
return DIGEST_PREFIX + Hex.encodeHexString(hash);
245+
} catch (NoSuchAlgorithmException e) {
246+
throw new UnknownException(
247+
"Failed to calculate manifest digest: " + DIGEST_ALGORITHM
248+
+ " SHA-256 algorithm not available",
249+
e);
255250
}
256251
}
257252

‎registry/registry-client/src/test/java/com/salesforce/multicloudj/registry/driver/OciRegistryClientTest.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -854,7 +854,11 @@ void testDownloadBlob_SetsAuthorizationHeader() throws Exception {
854854
HttpGet request = invocation.getArgument(0);
855855
Header authHeader = request.getFirstHeader("Authorization");
856856
assertNotNull(authHeader, "Authorization header should be set");
857-
assertEquals("Basic dXNlcjp0b2tlbg==", authHeader.getValue()); // Base64("user:token")
857+
assertEquals(
858+
"Basic "
859+
+ Base64.getEncoder()
860+
.encodeToString("user:token".getBytes(StandardCharsets.UTF_8)),
861+
authHeader.getValue());
858862
});
859863

860864
try (MockedStatic<AuthChallenge> mockedAuthChallenge = mockAuthChallenge()) {

0 commit comments

Comments
 (0)