Add SpotBugs static analysis target, workflow, and fix reported issues#344
Add SpotBugs static analysis target, workflow, and fix reported issues#344rlm2002 merged 9 commits intowolfSSL:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds SpotBugs static analysis integration (Ant target + GitHub Actions workflow) and applies a broad set of code changes to address SpotBugs-reported issues across the Java/JNI codebase.
Changes:
- Introduces SpotBugs tooling: Ant targets, CI workflow, and an exclusion filter.
- Refactors and hardens multiple JSSE/JNI classes (thread-safety, visibility reductions, encoding fixes, null-safety).
- Renames several internal helper methods/classes and adjusts inner classes to be
static.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/java/com/wolfssl/provider/jsse/adapter/WolfSSLJDK8Helper.java | Removes redundant reflection null-check flagged by static analysis. |
| src/java/com/wolfssl/provider/jsse/WolfSSLX509.java | Converts inner class to static to avoid outer reference capture. |
| src/java/com/wolfssl/provider/jsse/WolfSSLUtil.java | Renames helpers to lowerCamelCase; used by trust/key manager loading paths. |
| src/java/com/wolfssl/provider/jsse/WolfSSLTrustManager.java | Updates renamed helper calls; improves CA listing handling and JAVA_HOME lookup. |
| src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java | Improves concurrency for ALPN selector; renames helper calls; makes inner classes static. |
| src/java/com/wolfssl/provider/jsse/WolfSSLProvider.java | Makes callback class static; changes setDevId to static. |
| src/java/com/wolfssl/provider/jsse/WolfSSLParameters.java | Synchronizes selected getters/setters to address inconsistent synchronization warnings. |
| src/java/com/wolfssl/provider/jsse/WolfSSLKeyManager.java | Updates renamed helper calls to keep keystore loading consistent. |
| src/java/com/wolfssl/provider/jsse/WolfSSLInternalVerifyCb.java | Updates to new server-name getter API. |
| src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java | Marks shared state volatile to address visibility across threads. |
| src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java | Enforces explicit UTF-8 encoding and renames a helper for consistency. |
| src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java | Synchronizes enabled cipher suites getter; adds Javadoc; makes inner callbacks static; tightens debug flag visibility. |
| src/java/com/wolfssl/provider/jsse/WolfSSLCustomUser.java | Reduces field visibility; renames factory method. |
| src/java/com/wolfssl/provider/jsse/WolfSSLContext.java | Updates to renamed WolfSSLCustomUser factory method. |
| src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java | Enforces UTF-8 for byte conversions; makes LRU store inner class static. |
| src/java/com/wolfssl/WolfSSLX509Name.java | Uses StandardCharsets.UTF_8 and avoids repeated getBytes() calls. |
| src/java/com/wolfssl/WolfSSLSession.java | Fixes null dereference; changes synchronization strategy for static settings. |
| src/java/com/wolfssl/WolfSSLDebug.java | Removes redundant null checks and dead locals in logging/formatting. |
| src/java/com/wolfssl/WolfSSLContext.java | Fixes Arrays.asList(int[]) misuse in logging. |
| src/java/com/wolfssl/WolfSSLCertificate.java | Removes unnecessary transient from lock fields. |
| src/java/com/wolfssl/WolfSSLCRL.java | Removes unnecessary transient from lock fields. |
| src/java/com/wolfssl/WolfSSL.java | Makes a constant final to match constant semantics. |
| spotbugs-exclude.xml | Adds comprehensive SpotBugs suppression filter with rationale comments. |
| native/com_wolfssl_WolfSSL.h | Regenerates JNI header constants affected by Java-side constant changes. |
| build.xml | Adds Ant SpotBugs targets and environment/version checks. |
| .github/workflows/spotbugs.yml | Adds CI workflow to run SpotBugs on PRs touching Java/build config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix null dereference in setServerID() before id.length access - Fix Arrays.asList() on primitive int[] in setGroups() debug log - Use StandardCharsets.UTF_8 instead of default encoding in getBytes() - Remove redundant null checks (Thread.getName(), new File(), Class.getMethod(), cast after instanceof) - Fix null dereference of cafiles in LoadAndroidSystemCertsManually() - Add Javadoc to cacheRequestedServerNamesFromNetData()
- Add missing final to WOLFSSL_LEFT_MOST_WILDCARD_ONLY constant - Update generated JNI header for WOLFSSL_LEFT_MOST_WILDCARD_ONLY - Make WolfSSLProvider.setDevId() static - Reduce visibility of WolfSSLCustomUser fields to package-protected - Make WolfSSLEngine debug flags private - Rename internal PascalCase methods to camelCase (LoadKeyAndCertChain, GetJavaHome, LoadKeyStoreFileByType, GetCtxAttributes)
- Fix inconsistent synchronization in WolfSSLParameters for getCipherSuites/setCipherSuites and getApplicationProtocols/setApplicationProtocols - Add volatile to WolfSSLImplementSSLSession.isInTable and alpnSelector fields - Use class lock for static fields in detectSecurityPropertySettings() - Add synchronized to WolfSSLEngine.getEnabledCipherSuites()
- Make 14 inner classes static where outer instance is not needed - Remove unnecessary transient on lock fields in WolfSSLCRL and WolfSSLCertificate - Remove dead local variables in WolfSSLDebug log() and JSONFormatter
There was a problem hiding this comment.
Pull request overview
This PR integrates SpotBugs static analysis into the Ant build + GitHub Actions, and applies a broad set of SpotBugs-driven fixes across the JNI/JSSE Java code (null-safety, encoding correctness, synchronization/visibility, and inner-class/static cleanups).
Changes:
- Added an Ant
spotbugstarget (with an exclusion filter) and a PR workflow to run SpotBugs when Java-related files change. - Addressed numerous SpotBugs findings via thread-safety fixes (synchronization/volatile), null handling, safer encoding (
StandardCharsets.UTF_8), and API/visibility cleanups. - Updated Android CI emulator runner configuration/pinning.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/java/com/wolfssl/provider/jsse/adapter/WolfSSLJDK8Helper.java | Removes redundant reflection null-check in ALPN helper. |
| src/java/com/wolfssl/provider/jsse/WolfSSLX509.java | Makes inner Principal implementation static (no outer ref). |
| src/java/com/wolfssl/provider/jsse/WolfSSLUtil.java | Renames internal helpers to lowerCamelCase. |
| src/java/com/wolfssl/provider/jsse/WolfSSLTrustManager.java | Updates calls to renamed util + refines Android CA listing logic. |
| src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java | Adds volatile to ALPN selector, renames helper call, makes inner classes static, tweaks connect logic. |
| src/java/com/wolfssl/provider/jsse/WolfSSLProvider.java | Makes callback class static and setDevId static. |
| src/java/com/wolfssl/provider/jsse/WolfSSLParameters.java | Synchronizes array-based getters/setters; adds Javadoc to copy(). |
| src/java/com/wolfssl/provider/jsse/WolfSSLKeyManager.java | Updates calls to renamed util method. |
| src/java/com/wolfssl/provider/jsse/WolfSSLInternalVerifyCb.java | Uses wolfSSL-specific SNI accessor for verification. |
| src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java | Marks isInTable as volatile. |
| src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java | Renames key/cert loader method + uses UTF-8 for SNI/serverId bytes. |
| src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java | Marks ALPN selector volatile, tightens debug flag visibility, adds SNI caching Javadoc, makes inner callbacks static, synchronizes getter. |
| src/java/com/wolfssl/provider/jsse/WolfSSLCustomUser.java | Reduces field visibility; renames internal factory method. |
| src/java/com/wolfssl/provider/jsse/WolfSSLContext.java | Updates call site for renamed WolfSSLCustomUser factory. |
| src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java | Uses UTF-8 bytes for IDs/SNI; makes session store inner class static. |
| src/java/com/wolfssl/WolfSSLX509Name.java | Uses StandardCharsets.UTF_8 and avoids repeated getBytes() calls. |
| src/java/com/wolfssl/WolfSSLSession.java | Adjusts locking for static pool settings; fixes setServerID null deref. |
| src/java/com/wolfssl/WolfSSLDebug.java | Removes dead locals in formatter/log path. |
| src/java/com/wolfssl/WolfSSLContext.java | Fixes Arrays.asList(int[]) debug output by using Arrays.toString. |
| src/java/com/wolfssl/WolfSSL.java | Makes WOLFSSL_LEFT_MOST_WILDCARD_ONLY constant final. |
| spotbugs-exclude.xml | Adds SpotBugs suppression filter with rationales. |
| native/com_wolfssl_WolfSSL.h | Regenerates/updates constant macro for JNI header. |
| build.xml | Adds SpotBugs availability/Java-version checks and spotbugs build target emitting HTML/XML reports. |
| .github/workflows/spotbugs.yml | New workflow to run SpotBugs on PRs touching Java/build/filter files. |
| .github/workflows/android_gradle.yml | Pins emulator-runner version and switches emulator target/caching key. |
Comments suppressed due to low confidence (1)
src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java:2316
- In connect(),
address.getAddress()can be null for an unresolvedInetSocketAddress(e.g., created viaInetSocketAddress.createUnresolved(...)), which would cause an NPE when callinggetHostAddress()and thensetPeerAddress(...). Consider handling the unresolved case by usingaddress.getHostString()/address.getHostName()and guardingaddress.getAddress()before dereferencing, or skippingsetPeerAddresswhen unresolved.
if (EngineHelper != null) {
EngineHelper.setHostAndPort(
address.getAddress().getHostAddress(),
address.getPort());
EngineHelper.setPeerAddress(address.getAddress());
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
This PR integrates SpotBugs into the Ant build and GitHub Actions, and applies a broad set of SpotBugs-driven fixes across the wolfJSSE / JNI code to address correctness, concurrency, and API-usage warnings.
Changes:
- Add an Ant
spotbugstarget plusspotbugs-exclude.xml, and a GitHub Actions workflow to run SpotBugs on Java-related PRs. - Fix a set of SpotBugs findings across core JSSE/JNI classes (null-safety, charset correctness, synchronization/visibility, inner-class static-ness, logging correctness).
- Adjust CI Android emulator workflow configuration (pin action version and change emulator target).
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/test/com/wolfssl/test/WolfSSLTestCommon.java | Adds Windows detection helper for tests. |
| src/test/com/wolfssl/test/WolfSSLNameConstraintsTest.java | Skips specific IP/SNI-related tests on Windows due to native parsing issue. |
| src/java/com/wolfssl/provider/jsse/adapter/WolfSSLJDK8Helper.java | Removes redundant null-check after reflection lookup. |
| src/java/com/wolfssl/provider/jsse/WolfSSLX509.java | Makes Principal inner class static. |
| src/java/com/wolfssl/provider/jsse/WolfSSLUtil.java | Renames internal helpers to lower camelCase. |
| src/java/com/wolfssl/provider/jsse/WolfSSLTrustManager.java | Updates renamed util calls and hardens Android CA directory listing logic. |
| src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java | Concurrency visibility tweaks and converts several inner classes to static; renames helper method usage. |
| src/java/com/wolfssl/provider/jsse/WolfSSLProvider.java | Makes callback inner class static; makes setDevId static. |
| src/java/com/wolfssl/provider/jsse/WolfSSLParameters.java | Improves synchronization consistency and adds Javadoc for copy(). |
| src/java/com/wolfssl/provider/jsse/WolfSSLKeyManager.java | Updates renamed util callsites. |
| src/java/com/wolfssl/provider/jsse/WolfSSLInternalVerifyCb.java | Uses wolfSSL-specific SNI server-name accessor. |
| src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java | Marks isInTable volatile for cross-thread visibility. |
| src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java | Uses explicit UTF-8 charset; renames key/cert loader method. |
| src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java | Synchronizes enabled cipher getter; makes callback inner classes static; adds Javadoc. |
| src/java/com/wolfssl/provider/jsse/WolfSSLCustomUser.java | Reduces field visibility; renames internal factory to lower camelCase. |
| src/java/com/wolfssl/provider/jsse/WolfSSLContext.java | Updates call to renamed customization helper. |
| src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java | Uses UTF-8 for bytes; makes session store inner class static. |
| src/java/com/wolfssl/WolfSSLX509Name.java | Uses UTF-8 bytes consistently when passing strings to native code. |
| src/java/com/wolfssl/WolfSSLSession.java | Fixes null dereference in setServerID; adjusts synchronization for static pool settings. |
| src/java/com/wolfssl/WolfSSLDebug.java | Removes redundant null checks / dead locals in formatting and logging. |
| src/java/com/wolfssl/WolfSSLContext.java | Fixes debug logging of int[] by using Arrays.toString. |
| src/java/com/wolfssl/WolfSSL.java | Marks constant as final. |
| spotbugs-exclude.xml | Adds SpotBugs suppression filter with rationales for reviewed findings. |
| native/com_wolfssl_WolfSSL.h | Updates generated constant define for JNI header. |
| build.xml | Adds SpotBugs availability/java-version checks and spotbugs targets to produce HTML+XML reports. |
| .github/workflows/spotbugs.yml | Adds CI workflow to run SpotBugs and gate on warning count. |
| .github/workflows/android_gradle.yml | Pins emulator runner action version and changes emulator target config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Code fixes