fix(QTDI-2954): fix possible NPE in VaultClient

vault-client/src/main/java/org/talend/sdk/components/vault/client/VaultClient.java

@@ -155,7 +155,13 @@
     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;
+            }
+            final int status = response.getStatus();
             if (Status.NOT_FOUND.getStatusCode() == status || status >= 500) {
                 return false;
             }
@@ -327,7 +333,7 @@
                             })
                             // oops, smtg went wrong
                             .exceptionally(e -> {
-                                final Throwable cause = e.getCause();
+                                final Throwable cause = unwrap(e);
                                 String message = "";
                                 int status = cantDecipherStatusCode;
                                 if (WebApplicationException.class.isInstance(cause)) {
@@ -408,29 +414,34 @@
                     return authentication;
                 })
                 //
-                .exceptionally(e -> {
-                    final Throwable cause = e.getCause();
-                    if (WebApplicationException.class.isInstance(cause)) {
-                        final WebApplicationException wae = WebApplicationException.class.cast(cause);
-                        final Response response = wae.getResponse();
-                        String message = "";
-                        if (ErrorPayload.class.isInstance(wae.getResponse().getEntity())) {
-                            throw wae; // already logged and setup broken so just rethrow
-                        } else {
-                            try {
-                                message = response.readEntity(String.class);
-                            } catch (final Exception ignored) {
-                                // no-op
-                            }
-                            if (message.isEmpty()) {
-                                message = cause.getMessage();
-                            }
-                            throwError(response.getStatus(), message);
-                        }
-                    }
-                    throwError(cause);
-                    return null;
-                });
+                .exceptionally(this::handleAuthException);
+    }
+
+    private Authentication handleAuthException(final Throwable e) {
+        final Throwable cause = unwrap(e);
+        if (WebApplicationException.class.isInstance(cause)) {
+            final WebApplicationException wae = WebApplicationException.class.cast(cause);
+            final Response response = wae.getResponse();
+            // if the response is null we cannot extract status/entity, fall back to the
+            // null-safe throwError(Throwable) below (avoids an NPE that would mask the cause).
+            if (response != null) {
+                String message = "";
+                if (ErrorPayload.class.isInstance(response.getEntity())) {
+                    throw wae; // already logged and setup broken so just rethrow
+                }
+                try {
+                    message = response.readEntity(String.class);
+                } catch (final Exception ignored) {
+                    // no-op
+                }
+                if (message == null || message.isEmpty()) {
+                    message = cause.getMessage();
+                }
+                throwError(response.getStatus(), message);
+            }
+        }
+        throwError(cause);
+        return null;
     }
 
     private <T> CompletableFuture<T> withRetries(final Supplier<CompletableFuture<T>> attempt,
@@ -449,9 +460,9 @@
         log
                 .info("[retryFuture] Retry failed operation ({}/{}). Reason: {}.", attemptsSoFar,
                         numberOfRetryOnFailure, throwable.getMessage());
-        if (nextAttempt > numberOfRetryOnFailure || !shouldRetry.test(throwable.getCause())) {
+        if (nextAttempt > numberOfRetryOnFailure || !shouldRetry.test(unwrap(throwable))) {
             log.info("[retryFuture] Stop retry failed operation (condition triggered).");
-            throwError(throwable.getCause());
+            throwError(unwrap(throwable));
         }
         return flatten(flatten(CompletableFuture.supplyAsync(attempter, scheduler))
                 .thenApply(CompletableFuture::completedFuture)
@@ -469,13 +480,19 @@
     }
 
     private void throwError(final Throwable cause) {
+        // CompletableFuture#exceptionally hands the exception "as is". If the previous stage threw
+        // the exception directly (not wrapped in a CompletionException), Throwable#getCause() can
+        // return null. Be defensive so we never trigger an NPE that would mask the original error.
+        final Throwable safeCause = cause != null
+                ? cause
+                : new IllegalStateException("Unknown error (null cause)");
         String message = "";
         int status = cantDecipherStatusCode;
-        if (WebApplicationException.class.isInstance(cause)) {
-            final WebApplicationException wae = WebApplicationException.class.cast(cause);
+        if (WebApplicationException.class.isInstance(safeCause)) {
+            final WebApplicationException wae = WebApplicationException.class.cast(safeCause);
             final Response response = wae.getResponse();
-            status = response.getStatus();
             if (response != null) {
+                status = response.getStatus();
                 if (ErrorPayload.class.isInstance(response.getEntity())) { // internal error
                     throw wae;
                 } else {
@@ -487,13 +504,27 @@
                 }
             }
         }
-        if (message.isEmpty()) {
-            message = cause.getMessage();
+        if (message == null || message.isEmpty()) {
+            message = safeCause.getMessage();
         }
         throw new WebApplicationException(message,
                 Response.status(status).entity(new ErrorPayload(ErrorDictionary.UNEXPECTED, message)).build());
     }
 
+    /**
+     * Returns a non-null root cause for a {@link Throwable} received by a
+     * {@link java.util.concurrent.CompletableFuture#exceptionally(java.util.function.Function)} lambda.
+     * Falls back to the exception itself when {@link Throwable#getCause()} is {@code null} (the
+     * upstream stage threw the exception directly instead of wrapping it in a {@code CompletionException}).
+     */
+    private static Throwable unwrap(final Throwable e) {
+        if (e == null) {
+            return new IllegalStateException("Unknown error (null exception)");
+        }
+        final Throwable cause = e.getCause();
+        return cause != null ? cause : e;
+    }
+
     // workaround while geronimo-config does not support generics of generics
     // (1.2.1 in org.apache.geronimo.config.cdi.ConfigInjectionBean.create)
     private boolean isReloadableConfigSet(final String value) {

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

@@ -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;
@@ -296,6 +299,124 @@
                 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;
+        // 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));
+    }
+
+    @Test
+    void handleAuthExceptionWithNullResponseDoesNotNpe() throws Exception {
+        // Regression: doAuth().exceptionally() used to dereference wae.getResponse().getEntity()
+        // and response.getStatus() without a null check; with a null response that NPE masked the
+        // original cause. The handler now falls through to throwError(Throwable) which is null-safe.
+        final int defaultStatus = 422;
+        final VaultClient client = new VaultClient();
+        client.setCantDecipherStatusCode(defaultStatus);
+
+        final WebApplicationException waeWithoutResponse = new WebApplicationException("boom") {
+
+            @Override
+            public Response getResponse() {
+                return null;
+            }
+        };
+        // Mimics what CompletableFuture#exceptionally receives: a CompletionException wrapping cause.
+        final java.util.concurrent.CompletionException wrapped =
+                new java.util.concurrent.CompletionException(waeWithoutResponse);
+
+        final java.lang.reflect.Method handler =
+                VaultClient.class.getDeclaredMethod("handleAuthException", Throwable.class);
+        handler.setAccessible(true);
+        try {
+            handler.invoke(client, wrapped);
+            org.junit.jupiter.api.Assertions.fail("Expected a WebApplicationException to be thrown");
+        } catch (final java.lang.reflect.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
+    void handleAuthExceptionWithNullCauseDoesNotNpe() throws Exception {
+        // Regression: CompletableFuture#exceptionally can receive an exception thrown directly
+        // (not wrapped in CompletionException), so e.getCause() may be null. The handler must
+        // not NPE on cause.getMessage() / shouldRetry / throwError.
+        final int defaultStatus = 422;
+        final VaultClient client = new VaultClient();
+        client.setCantDecipherStatusCode(defaultStatus);
+
+        // RuntimeException with no cause => e.getCause() returns null.
+        final RuntimeException directThrow = new RuntimeException("direct");
+
+        final java.lang.reflect.Method handler =
+                VaultClient.class.getDeclaredMethod("handleAuthException", Throwable.class);
+        handler.setAccessible(true);
+        try {
+            handler.invoke(client, directThrow);
+            org.junit.jupiter.api.Assertions.fail("Expected a WebApplicationException to be thrown");
+        } catch (final java.lang.reflect.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("direct", thrown.getMessage());
+        }
+    }
+
     /**
      * Demonstration purpose: with a "real" vault server instance
      * 
@@ -320,9 +441,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_");