server: do not NPE when a VPN provider returns no per-user result - #14047
server: do not NPE when a VPN provider returns no per-user result#14047nagaboinaramgopal wants to merge 1 commit into
Conversation
applyVpnUsers sizes a Boolean[] finals to the user list but only populates it inside the if (results != null) block. A RemoteAccessVPNServiceProvider that returns null (for example when the VPN network has no router yet) leaves entries null, and the later consumption loop unboxed them with if (finals[i]), throwing NullPointerException and aborting the entire apply/add/remove-user operation. Treat a null entry as not-applied with Boolean.TRUE.equals.
DaanHoogland
left a comment
DaanHoogland
left a comment
There was a problem hiding this comment.
clgtm, good practice but I doubt this would be a possible issue ever, the loop filling finals above seems airtight.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #14047 +/- ##
=========================================
Coverage 16.34% 16.35%
- Complexity 13574 13581 +7
=========================================
Files 5669 5669
Lines 501368 501368
Branches 60903 60903
=========================================
+ Hits 81964 82000 +36
+ Misses 410219 410175 -44
- Partials 9185 9193 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@blueorangutan package |
|
@abh1sar a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19124 |
|
@abh1sar , do you think smoke tests will make us the wiser? I think this is a hardening we can merge like this. |
4 participants
applyVpnUsers sizes a Boolean[] finals to the user list but only populates it inside the if (results != null) block. A RemoteAccessVPNServiceProvider that returns null (for example when the VPN network has no router yet) leaves entries null, and the consumption loop unboxed them with if (finals[i]), throwing NullPointerException and aborting the entire apply. Treat a null entry as not-applied with Boolean.TRUE.equals.
Tested: new unit test applyVpnUsersHandlesNullProviderResultWithoutNpe (fails before, passes after); RemoteAccessVpnManagerImplTest green.