Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

NullPointerException when retrieving Vault contents with null values #327

Open
joeyy-watts opened this issue Sep 27, 2024 · 2 comments · May be fixed by #329
Open

NullPointerException when retrieving Vault contents with null values #327

joeyy-watts opened this issue Sep 27, 2024 · 2 comments · May be fixed by #329

Comments

@joeyy-watts
Copy link

joeyy-watts commented Sep 27, 2024 •
edited
Loading

This issue is found in version 4.0.1, although I still see the problematic code in 4.1.0.

Issue

In my use case, I have a JSON secret in Vault which can contain null values, e.g:

{
     "password": "foo",
     "someotherfield": null
}

When I try to read this secret with quarkus-vault, the following exception is thrown:

2024-09-26 12:45:19,744 WARN  [io.agr.pool] (agroal-21) Datasource '<default>': Failed to create connection due to NullPointerException
2024-09-26 12:45:19,744 ERROR [io.sma.health] (executor-thread-1) SRHCK01000: Error processing Health Checks: java.lang.NullPointerException
    at java.base/java.util.Objects.requireNonNull(Unknown Source)
    at java.base/java.util.stream.Collectors.lambda$uniqKeysMapAccumulator$1(Unknown Source)
    at java.base/java.util.stream.ReduceOps$3ReducingSink.accept(Unknown Source)
    at java.base/java.util.Iterator.forEachRemaining(Unknown Source)
    at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Unknown Source)
    at java.base/java.util.stream.AbstractPipeline.copyInto(Unknown Source)
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(Unknown Source)
    at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(Unknown Source)
    at java.base/java.util.stream.AbstractPipeline.evaluate(Unknown Source)
    at java.base/java.util.stream.ReferencePipeline.collect(Unknown Source)
    at io.quarkus.vault.runtime.VaultKvManager.convert(VaultKvManager.java:38)

On this line in VaultKvManager.java I've found the cause to be the way the library is casting the JSON value to String.

(String) entry.getValue()

In the case that the entry's value is null this will also return null, causing the exception above.

I suspect it will also fail if the value is of a non-String type, with the following exception (although I haven't tested with the library; just ran some scratch code):

Exception in thread "main" java.lang.ClassCastException: class java.lang.Integer cannot be cast to class java.lang.String (java.lang.Integer and java.lang.String are in module java.base of loader 'bootstrap')

How to Reproduce the Issue

Attempt to read a secret with at least one null value.

Proposed Fix

To fix this issue, I have applied this patch to use String.valueOf() instead. I've used this in my own fork of the library, and confirmed it to be working:

private Map<String, String> convert(Map<String, Object> map) {
        return map.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, entry -> String.valueOf(entry.getValue())));
}
@kdubb
Copy link
Contributor

kdubb commented Sep 27, 2024

Are you able to provide a PR? If possible with a test covering the use case.

@joeyy-watts joeyy-watts linked a pull request Sep 28, 2024 that will close this issue
Open
@joeyy-watts
Copy link
Author

joeyy-watts commented Sep 28, 2024 •
edited
Loading

After going through the code more thoroughly and finding this comment I've decided to change the fix.

(I'd rather keep VaultKvManager as-is to avoid unexpected side effects elsewhere)

I've opted to update VaultCredentialsProvider instead to use .readSecretJson (which doesn't do the problematic conversion), and using String.valueOf() to cast it where it's actually used.

Although this does mean it no longer (implicitly) guarantees the password to never be null. I'm open to your input here @kdubb

I'll test it with my use case and let you know how it goes.
Tested on my own fork, and confirmed to be working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

use .readSecretJson in VaultCredentialsProvider joeyy-watts/quarkus-vault
2 participants
@kdubb @joeyy-watts