You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "Akanksha-kedia (via GitHub)" <gi...@apache.org> on 2026/05/11 10:12:31 UTC
[PR] Fix NPE in StaticTokenAuthProvider and UrlAuthProvider when required config property is absent [pinot]
Akanksha-kedia opened a new pull request, #18456:
URL: https://github.com/apache/pinot/pull/18456
## Summary
Both `StaticTokenAuthProvider(AuthConfig)` and `UrlAuthProvider(AuthConfig)` call `.toString()` directly on `authConfig.getProperties().get(...)` without a null check. When the required property (`token` or `url`) is absent from the config the result is an opaque `NullPointerException` with no indication of which property is missing.
This PR replaces the implicit null deref with an explicit check that throws `IllegalArgumentException` naming the missing property, giving operators an actionable error message.
```java
// Before
String userToken = authConfig.getProperties().get(TOKEN).toString(); // NPE if TOKEN absent
// After
Object tokenValue = authConfig.getProperties().get(TOKEN);
if (tokenValue == null) {
throw new IllegalArgumentException("Missing required auth config property: " + TOKEN);
}
String userToken = tokenValue.toString();
```
The same pattern is applied to `UrlAuthProvider`.
## Test plan
- [ ] Existing auth provider unit tests pass
- [ ] Constructing either provider with a config missing the required property throws `IllegalArgumentException` with a clear message instead of `NullPointerException`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix NPE in StaticTokenAuthProvider and UrlAuthProvider when required config property is absent [pinot]
Posted by "yashmayya (via GitHub)" <gi...@apache.org>.
yashmayya merged PR #18456:
URL: https://github.com/apache/pinot/pull/18456
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix NPE in StaticTokenAuthProvider and UrlAuthProvider when required config property is absent [pinot]
Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on PR #18456:
URL: https://github.com/apache/pinot/pull/18456#issuecomment-4430175110
@yashmayya please merge
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix NPE in StaticTokenAuthProvider and UrlAuthProvider when required config property is absent [pinot]
Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on code in PR #18456:
URL: https://github.com/apache/pinot/pull/18456#discussion_r3226080339
##########
pinot-common/src/main/java/org/apache/pinot/common/auth/UrlAuthProvider.java:
##########
@@ -57,7 +57,11 @@ public UrlAuthProvider(AuthConfig authConfig) {
try {
_header = AuthProviderUtils.getOrDefault(authConfig, HEADER, HttpHeaders.AUTHORIZATION);
_prefix = AuthProviderUtils.getOrDefault(authConfig, PREFIX, "Bearer");
- _url = new URL(authConfig.getProperties().get(URL).toString());
+ Object urlValue = authConfig.getProperties().get(URL);
+ if (urlValue == null) {
+ throw new IllegalArgumentException("Missing required auth config property: " + URL);
+ }
Review Comment:
Done, moved both the null check and urlValue outside the try block in the latest commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix NPE in StaticTokenAuthProvider and UrlAuthProvider when required config property is absent [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #18456:
URL: https://github.com/apache/pinot/pull/18456#issuecomment-4420242958
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/18456?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `8 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 55.72%. Comparing base ([`1881884`](https://app.codecov.io/gh/apache/pinot/commit/18818842e5cfcb97ca9cc7e21f01eee5a94ef353?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`2e644ce`](https://app.codecov.io/gh/apache/pinot/commit/2e644ce2ece66d14ceb4fe73b6dcfad0b2d1b297?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 76 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/18456?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...che/pinot/common/auth/StaticTokenAuthProvider.java](https://app.codecov.io/gh/apache/pinot/pull/18456?src=pr&el=tree&filepath=pinot-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcommon%2Fauth%2FStaticTokenAuthProvider.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXV0aC9TdGF0aWNUb2tlbkF1dGhQcm92aWRlci5qYXZh) | 0.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18456?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../org/apache/pinot/common/auth/UrlAuthProvider.java](https://app.codecov.io/gh/apache/pinot/pull/18456?src=pr&el=tree&filepath=pinot-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcommon%2Fauth%2FUrlAuthProvider.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXV0aC9VcmxBdXRoUHJvdmlkZXIuamF2YQ==) | 0.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18456?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (1881884) and HEAD (2e644ce). Click for more details.
>
> <details><summary>HEAD has 12 uploads less than BASE</summary>
>
>| Flag | BASE (1881884) | HEAD (2e644ce) |
>|------|------|------|
>|java-21|4|1|
>|unittests|2|1|
>|temurin|4|1|
>|unittests2|1|0|
>|integration|2|0|
>|integration1|1|0|
>|custom-integration1|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #18456 +/- ##
============================================
- Coverage 63.40% 55.72% -7.69%
+ Complexity 1679 815 -864
============================================
Files 3253 2545 -708
Lines 198767 148072 -50695
Branches 30791 23930 -6861
============================================
- Hits 126034 82509 -43525
+ Misses 62659 58497 -4162
+ Partials 10074 7066 -3008
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/18456/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/18456/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/18456/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/18456/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/18456/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.72% <0.00%> (-7.69%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/18456/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.72% <0.00%> (-7.69%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/18456/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.72% <0.00%> (-7.69%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/18456/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.72% <0.00%> (+0.35%)` | :arrow_up: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/18456/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/18456?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix NPE in StaticTokenAuthProvider and UrlAuthProvider when required config property is absent [pinot]
Posted by "Akanksha-kedia (via GitHub)" <gi...@apache.org>.
Akanksha-kedia commented on PR #18456:
URL: https://github.com/apache/pinot/pull/18456#issuecomment-4419620580
@xiangfu0 please review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Fix NPE in StaticTokenAuthProvider and UrlAuthProvider when required config property is absent [pinot]
Posted by "yashmayya (via GitHub)" <gi...@apache.org>.
yashmayya commented on code in PR #18456:
URL: https://github.com/apache/pinot/pull/18456#discussion_r3222798213
##########
pinot-common/src/main/java/org/apache/pinot/common/auth/UrlAuthProvider.java:
##########
@@ -57,7 +57,11 @@ public UrlAuthProvider(AuthConfig authConfig) {
try {
_header = AuthProviderUtils.getOrDefault(authConfig, HEADER, HttpHeaders.AUTHORIZATION);
_prefix = AuthProviderUtils.getOrDefault(authConfig, PREFIX, "Bearer");
- _url = new URL(authConfig.getProperties().get(URL).toString());
+ Object urlValue = authConfig.getProperties().get(URL);
+ if (urlValue == null) {
+ throw new IllegalArgumentException("Missing required auth config property: " + URL);
+ }
Review Comment:
nit: shouldn't this be outside the try block?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org