Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,13 @@ public class VaultClient {
private final Predicate<Throwable> shouldRetry = cause -> {
if (WebApplicationException.class.isInstance(cause)) {
final WebApplicationException wae = WebApplicationException.class.cast(cause);
final int status = wae.getResponse().getStatus();
final Response response = wae.getResponse();
if (response == null) {
// no response information: don't make the retry loop NPE, keep retrying as a
// recoverable error (the underlying cause is logged by retryFuture).
return true;
}
Comment thread
undx marked this conversation as resolved.
final int status = response.getStatus();
if (Status.NOT_FOUND.getStatusCode() == status || status >= 500) {
return false;
}
Expand Down Expand Up @@ -474,8 +480,8 @@ private void throwError(final Throwable cause) {
if (WebApplicationException.class.isInstance(cause)) {
final WebApplicationException wae = WebApplicationException.class.cast(cause);
final Response response = wae.getResponse();
status = response.getStatus();
if (response != null) {
status = response.getStatus();
Comment thread
undx marked this conversation as resolved.
if (ErrorPayload.class.isInstance(response.getEntity())) { // internal error
throw wae;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,15 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.HashMap;
import java.util.Map;

import javax.enterprise.inject.se.SeContainer;
import javax.enterprise.inject.se.SeContainerInitializer;
import javax.inject.Inject;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.client.Client;
import javax.ws.rs.client.Entity;
import javax.ws.rs.client.WebTarget;
Expand Down Expand Up @@ -296,6 +299,60 @@
response.readEntity(ErrorPayload.class).getDescription());
}

@Test
void throwErrorWithNullResponseDoesNotNpe() throws Exception {
// A WebApplicationException whose getResponse() returns null must not trigger an NPE
// in throwError(Throwable); the default cantDecipherStatusCode should be used instead.
final int defaultStatus = 422;
// Use a fresh VaultClient instance (not the CDI proxy) so reflective field access works.
final VaultClient vaultClient = new VaultClient();
vaultClient.setCantDecipherStatusCode(defaultStatus);
final WebApplicationException waeWithoutResponse = new WebApplicationException("boom") {

@Override
public Response getResponse() {
return null;
}
};

final Method throwError = VaultClient.class.getDeclaredMethod("throwError", Throwable.class);
throwError.setAccessible(true);
try {
throwError.invoke(vaultClient, waeWithoutResponse);
org.junit.jupiter.api.Assertions.fail("Expected a WebApplicationException to be thrown");
} catch (final InvocationTargetException ite) {
final Throwable target = ite.getTargetException();
assertTrue(target instanceof WebApplicationException,
"Expected WebApplicationException, got " + target);
final WebApplicationException thrown = (WebApplicationException) target;
assertEquals(defaultStatus, thrown.getResponse().getStatus());
assertEquals("boom", thrown.getMessage());
}
}

@Test
@SuppressWarnings("unchecked")
void shouldRetryWithNullResponseDoesNotNpe() throws Exception {
// Regression: shouldRetry must not call wae.getResponse().getStatus() when the response is null;

Check warning on line 336 in vault-client/src/test/java/org/talend/sdk/components/vault/client/VaultClientTest.java

View check run for this annotation

sonar-rnd / SonarQube Code Analysis

vault-client/src/test/java/org/talend/sdk/components/vault/client/VaultClientTest.java#L336

This block of commented-out lines of code should be removed.
// otherwise retryFuture() raises an NPE before throwError(...) can build the fallback payload.
final VaultClient vaultClient = new VaultClient();
final java.lang.reflect.Field f = VaultClient.class.getDeclaredField("shouldRetry");
f.setAccessible(true);
final java.util.function.Predicate<Throwable> shouldRetry =
(java.util.function.Predicate<Throwable>) f.get(vaultClient);

final WebApplicationException waeWithoutResponse = new WebApplicationException("boom") {

@Override
public Response getResponse() {
return null;
}
};

// Must not NPE, and must allow the retry loop to keep running (true = retry).
assertTrue(shouldRetry.test(waeWithoutResponse));
}

/**
* Demonstration purpose: with a "real" vault server instance
*
Expand All @@ -320,9 +377,7 @@
@Test
@Disabled
void executeWithRealVault() {
// setup.setVaultUrl("http://localhost:8200");
final Client realClt = setup.vaultClient(setup.vaultExecutorService());
// vault.setVault(setup.vaultTarget(realClt));
setup.vaultClient(setup.vaultExecutorService());
vault.setAuthEndpoint("/v1/auth/approle/login");
vault.setDecryptEndpoint("/v1/transit/decrypt/{x-talend-tenant-id}");
vault.setRole(() -> "_role-id_");
Expand Down
Loading