Skip to content

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

Open
undx wants to merge 8 commits into
masterfrom
ouf/QTDI-2954-guard-npe
Open

fix(QTDI-2954): fix possible NPE in VaultClient#1230
undx wants to merge 8 commits into
masterfrom
ouf/QTDI-2954-guard-npe

Conversation

@undx

@undx undx commented May 28, 2026

Copy link
Copy Markdown
Member

https://qlik-dev.atlassian.net/browse/QTDI-2954

Requirements

  • Any code change adding any logic MUST be tested through a unit test executed with the default build
  • Any API addition MUST be done with a documentation update if relevant

Why this PR is needed?

What does this PR adds (design/code thoughts)?

AI generated code

https://internal.qlik.dev/general/ways-of-working/code-reviews/#guidelines-for-ai-generated-code

  • this PR has been written with the help of GitHub Copilot or another generative AI tool

@undx undx requested a review from Copilot May 28, 2026 09:44
Copilot AI reviewed May 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a possible null response handling issue in VaultClient.throwError(Throwable) by preserving the configured fallback status when a WebApplicationException has no Response.

Changes:

  • Moves response.getStatus() inside the existing null check in throwError(Throwable).
  • Adds a unit test that invokes throwError(Throwable) with a WebApplicationException whose response is null.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
vault-client/src/main/java/org/talend/sdk/components/vault/client/VaultClient.java Makes throwError(Throwable) avoid dereferencing a null JAX-RS response.
vault-client/src/test/java/org/talend/sdk/components/vault/client/VaultClientTest.java Adds regression coverage for null response handling in throwError(Throwable).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread vault-client/src/main/java/org/talend/sdk/components/vault/client/VaultClient.java Outdated
@undx undx requested a review from Copilot May 28, 2026 12:26
Copilot AI reviewed May 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread vault-client/src/main/java/org/talend/sdk/components/vault/client/VaultClient.java
@undx undx requested a review from Copilot May 28, 2026 14:01
Copilot AI reviewed May 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread vault-client/src/main/java/org/talend/sdk/components/vault/client/VaultClient.java Outdated
@undx
Potential fix for pull request finding
ddef9b8 
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@undx undx requested a review from Copilot May 28, 2026 14:11
Copilot AI reviewed May 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@undx undx requested a review from ozhelezniak-talend May 28, 2026 14:37
undx added 3 commits May 29, 2026 11:38
@undx
Merge branch 'master' into ouf/QTDI-2954-guard-npe
9a3bf85
@undx
Merge branch 'master' into ouf/QTDI-2954-guard-npe
a987761 
# Conflicts:
#	vault-client/src/main/java/org/talend/sdk/components/vault/client/VaultClient.java
#	vault-client/src/test/java/org/talend/sdk/components/vault/client/VaultClientTest.java
@undx
fix(QTDI-2954): fix master merge
6a0f1d5
ozhelezniak-talend
ozhelezniak-talend approved these changes Jul 9, 2026
View reviewed changes

@ozhelezniak-talend ozhelezniak-talend left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay

Could we rewrite the tests to not call private methods as units?
Maybe we can do component-test instead?

Comment thread vault-client/src/test/java/org/talend/sdk/components/vault/client/VaultClientTest.java
@Test
@Disabled
void executeWithRealVault() {
// setup.setVaultUrl("http://localhost:8200");

@ozhelezniak-talend ozhelezniak-talend Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fishy...
Was it AI that modified that test (even so it's Disabled)?

Comment thread vault-client/src/test/java/org/talend/sdk/components/vault/client/VaultClientTest.java
throwError.setAccessible(true);
try {
throwError.invoke(vaultClient, waeWithoutResponse);
org.junit.jupiter.api.Assertions.fail("Expected a WebApplicationException to be thrown");

@ozhelezniak-talend ozhelezniak-talend Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please import this 'org.junit.jupiter.api.Assertions'

Comment thread vault-client/src/test/java/org/talend/sdk/components/vault/client/VaultClientTest.java
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 =

@ozhelezniak-talend ozhelezniak-talend Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need this predicate...

@ozhelezniak-talend ozhelezniak-talend Jul 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just call this function directly inside

@sonar-rnd

sonar-rnd Bot commented Jul 9, 2026

Copy link
Copy Markdown

Failed Quality Gate failed

  • 0.00% Coverage on New Code (is less than 80.00%)
  • 2 New Issues (is greater than 0)

Project ID: org.talend.sdk.component:component-runtime

View in SonarQube

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@ozhelezniak-talend ozhelezniak-talend ozhelezniak-talend approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@undx @ozhelezniak-talend