Skip to content

Commit 6072615

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
1 parent bf575e6 commit 6072615

File tree

5 files changed

+89
-88
lines changed

5 files changed

+89
-88
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
@@ -53,12 +53,6 @@ public String getProviderId() {
5353
*/
5454
public abstract Builder<?, ?> builder();
5555

56-
@Override
57-
public abstract String getAuthUsername();
58-
59-
@Override
60-
public abstract String getAuthToken();
61-
6256
/** Returns the OCI client for this registry. */
6357
protected abstract OciRegistryClient getOciClient();
6458

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

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@
2727
*/
2828
public class BearerTokenExchange {
2929

30+
private static final String TOKEN_FIELD = "token";
31+
private static final String ACCESS_TOKEN_FIELD = "access_token";
32+
3033
private final CloseableHttpClient httpClient;
3134

3235
/**
@@ -61,14 +64,17 @@ public String getBearerToken(AuthChallenge challenge, String identityToken,
6164
throw new InvalidArgumentException("Bearer challenge missing realm");
6265
}
6366

64-
try {
65-
// Build token request URL using URIBuilder for proper encoding
6667
URI tokenUri = buildTokenUri(realm, challenge, repository, actions);
67-
6868
HttpGet request = new HttpGet(tokenUri);
69-
// Use identity token as Bearer auth for the token endpoint
7069
request.setHeader(HttpHeaders.AUTHORIZATION, "Bearer " + identityToken);
70+
try {
71+
return executeTokenRequest(request);
72+
} catch (IOException e) {
73+
throw new UnknownException("Token exchange request failed", e);
74+
}
75+
}
7176

77+
private String executeTokenRequest(HttpGet request) throws IOException {
7278
try (CloseableHttpResponse response = httpClient.execute(request)) {
7379
int statusCode = response.getStatusLine().getStatusCode();
7480
if (statusCode != HttpStatus.SC_OK) {
@@ -77,7 +83,7 @@ public String getBearerToken(AuthChallenge challenge, String identityToken,
7783
}
7884

7985
String responseBody = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
80-
86+
8187
JsonObject json;
8288
try {
8389
json = JsonParser.parseString(responseBody).getAsJsonObject();
@@ -86,17 +92,14 @@ public String getBearerToken(AuthChallenge challenge, String identityToken,
8692
}
8793

8894
// Token can be in "token" (Docker Hub, AWS ECR) or "access_token" (GCP Artifact Registry) field
89-
if (json.has("token") && !json.get("token").isJsonNull()) {
90-
return json.get("token").getAsString();
91-
} else if (json.has("access_token") && !json.get("access_token").isJsonNull()) {
92-
return json.get("access_token").getAsString();
95+
if (json.has(TOKEN_FIELD) && !json.get(TOKEN_FIELD).isJsonNull()) {
96+
return json.get(TOKEN_FIELD).getAsString();
97+
} else if (json.has(ACCESS_TOKEN_FIELD) && !json.get(ACCESS_TOKEN_FIELD).isJsonNull()) {
98+
return json.get(ACCESS_TOKEN_FIELD).getAsString();
9399
}
94100

95101
throw new UnknownException("Token response missing token field");
96102
}
97-
} catch (IOException e) {
98-
throw new UnknownException("Token exchange request failed", e);
99-
}
100103
}
101104

102105
private URI buildTokenUri(String realm, AuthChallenge challenge, String repository, String[] actions) {

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

Lines changed: 36 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -75,33 +75,42 @@ public InputStream extract() {
7575
return t;
7676
});
7777

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

106115
// Return a wrapper that checks for extraction errors and cleans up executor on close
107116
return new ExtractionInputStream(pipedIn, extractionError, executor);
@@ -121,11 +130,8 @@ private void processLayer(Layer layer, TarArchiveOutputStream tarOut,
121130
while ((entry = tarIn.getNextTarEntry()) != null) {
122131
String name = normalizePath(entry.getName());
123132

124-
if (handleWhiteout(name, deletedPaths, opaqueDirectories)) {
125-
continue;
126-
}
127-
128-
if (shouldSkipEntry(name, seenPaths, deletedPaths, opaqueDirectories)) {
133+
if (handleWhiteout(name, deletedPaths, opaqueDirectories)
134+
|| shouldSkipEntry(name, seenPaths, deletedPaths, opaqueDirectories)) {
129135
continue;
130136
}
131137

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

Lines changed: 37 additions & 39 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

5860
private final String registryEndpoint;
@@ -212,42 +214,42 @@ private String buildBearerAuthHeader(String repository) {
212214
* @throws UnknownException if the request fails or manifest cannot be parsed
213215
*/
214216
public Manifest fetchManifest(String repository, String reference) {
215-
String url = String.format("%s/v2/%s/manifests/%s", registryEndpoint, repository, reference);
217+
HttpGet request = buildFetchManifestRequest(repository, reference);
218+
try {
219+
return executeFetchManifestRequest(request, repository, reference);
220+
} catch (IOException e) {
221+
throw new UnknownException("Failed to fetch manifest", e);
222+
}
223+
}
216224

225+
private HttpGet buildFetchManifestRequest(String repository, String reference) {
226+
String url = String.format("%s/v2/%s/manifests/%s", registryEndpoint, repository, reference);
217227
HttpGet request = new HttpGet(url);
218228
request.setHeader(HttpHeaders.ACCEPT, MANIFEST_ACCEPT_HEADER);
219229
String authHeader = getHttpAuthHeader(repository);
220230
if (authHeader != null) {
221231
request.setHeader(HttpHeaders.AUTHORIZATION, authHeader);
222232
}
233+
return request;
234+
}
223235

224-
try {
236+
private Manifest executeFetchManifestRequest(HttpGet request, String repository, String reference)
237+
throws IOException {
225238
try (CloseableHttpResponse response = httpClient.execute(request)) {
226239
int statusCode = response.getStatusLine().getStatusCode();
227240
if (statusCode != HttpStatus.SC_OK) {
228241
String errorBody = response.getEntity() != null
229242
? EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8)
230243
: StringUtils.EMPTY;
231-
if (statusCode == HttpStatus.SC_NOT_FOUND) {
232-
throw new ResourceNotFoundException(String.format(
233-
"Failed to fetch manifest for %s:%s from %s - HTTP %d: %s",
234-
repository, reference, registryEndpoint, statusCode, errorBody));
235-
}
236-
if (statusCode == HttpStatus.SC_UNAUTHORIZED) {
237-
throw new UnAuthorizedException(String.format(
238-
"Failed to fetch manifest for %s:%s from %s - HTTP %d: %s",
239-
repository, reference, registryEndpoint, statusCode, errorBody));
240-
}
241-
throw new UnknownException(String.format(
242-
"Failed to fetch manifest for %s:%s from %s - HTTP %d: %s",
243-
repository, reference, registryEndpoint, statusCode, errorBody));
244+
String message = String.format(MANIFEST_ERROR_FORMAT,
245+
repository, reference, registryEndpoint, statusCode, errorBody);
246+
throw mapHttpStatusToException(statusCode, message);
244247
}
245248

246249
if (response.getEntity() == null) {
247250
throw new UnknownException("Failed to fetch manifest: empty response body");
248251
}
249252

250-
// Check manifest size limit (100 MB)
251253
long contentLength = response.getEntity().getContentLength();
252254
if (contentLength > MAX_MANIFEST_SIZE_BYTES) {
253255
throw new UnknownException(String.format(
@@ -256,34 +258,30 @@ public Manifest fetchManifest(String repository, String reference) {
256258
}
257259

258260
String responseBody = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
259-
Header header = response.getFirstHeader("Docker-Content-Digest");
260-
String digestHeader = header != null ? header.getValue() : null;
261-
262-
// If Docker-Content-Digest header is missing (e.g., AWS ECR), calculate it from the response body
263-
if (digestHeader == null) {
264-
try {
265-
MessageDigest md = MessageDigest.getInstance(DIGEST_ALGORITHM);
266-
byte[] hash = md.digest(responseBody.getBytes(StandardCharsets.UTF_8));
267-
digestHeader = DIGEST_PREFIX + Hex.encodeHexString(hash);
268-
} catch (NoSuchAlgorithmException e) {
269-
throw new UnknownException("Failed to calculate manifest digest: " + DIGEST_ALGORITHM
270-
+ " SHA-256 algorithm not available", e);
271-
}
272-
}
261+
String digestHeader = computeManifestDigest(response, responseBody);
273262

274-
// Validate digest if fetching by digest
275-
if (reference.startsWith(DIGEST_PREFIX)) {
276-
if (!reference.equals(digestHeader)) {
277-
throw new UnknownException(String.format(
278-
"Manifest digest mismatch: expected %s, got %s for %s:%s",
279-
reference, digestHeader, repository, reference));
280-
}
263+
if (reference.startsWith(DIGEST_PREFIX) && !reference.equals(digestHeader)) {
264+
throw new UnknownException(String.format(
265+
"Manifest digest mismatch: expected %s, got %s for %s:%s",
266+
reference, digestHeader, repository, reference));
281267
}
282268

283269
return parseManifestResponse(responseBody, digestHeader);
284270
}
285-
} catch (IOException e) {
286-
throw new UnknownException("Failed to fetch manifest", e);
271+
}
272+
273+
private String computeManifestDigest(CloseableHttpResponse response, String responseBody) {
274+
Header header = response.getFirstHeader("Docker-Content-Digest");
275+
if (header != null) {
276+
return header.getValue();
277+
}
278+
try {
279+
MessageDigest md = MessageDigest.getInstance(DIGEST_ALGORITHM);
280+
byte[] hash = md.digest(responseBody.getBytes(StandardCharsets.UTF_8));
281+
return DIGEST_PREFIX + Hex.encodeHexString(hash);
282+
} catch (NoSuchAlgorithmException e) {
283+
throw new UnknownException("Failed to calculate manifest digest: " + DIGEST_ALGORITHM
284+
+ " SHA-256 algorithm not available", e);
287285
}
288286
}
289287

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,7 @@ void testDownloadBlob_SetsAuthorizationHeader() throws Exception {
747747
HttpGet request = invocation.getArgument(0);
748748
Header authHeader = request.getFirstHeader("Authorization");
749749
assertNotNull(authHeader, "Authorization header should be set");
750-
assertEquals("Basic dXNlcjp0b2tlbg==", authHeader.getValue()); // Base64("user:token")
750+
assertEquals("Basic " + Base64.getEncoder().encodeToString("user:token".getBytes(StandardCharsets.UTF_8)), authHeader.getValue());
751751
});
752752

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

0 commit comments

Comments
 (0)