-
Notifications
You must be signed in to change notification settings - Fork 695
GEODE-10531: Remove SecurityManager Usage from OSProcess.java - Java 21 Blocker #7971
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.
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
Open
sboorlagadda
wants to merge
4
commits into
develop
Choose a base branch
from
feature/GEODE-10531
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
5e57ebe
GEODE-10479: Added spec and plan
sboorlagadda 9f2f47b
GEODE-10531: Remove deprecated SecurityManager APIs for Java 21 compa…
sboorlagadda f9be0fa
Update todo.md: Mark GEODE-10531 as complete
sboorlagadda 3951f25
GEODE-10531: Remove outdated @exception SecurityException JavaDoc
sboorlagadda File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| h2. Summary | ||
|
|
||
| Remove deprecated SecurityManager APIs from {{OSProcess.java}} that were marked for removal in Java 17 and already removed in Java 21. This issue blocks Java 21 migration. | ||
| h2. Description | ||
|
|
||
| The {{geode-logging}} module contains usage of {{java.lang.SecurityManager}} and {{System.getSecurityManager()}} APIs that have been deprecated for removal in Java 17 (JEP 411) and subsequently removed in Java 21. | ||
|
|
||
| *Affected File:* | ||
| {code:java} | ||
| geode-logging/src/main/java/org/apache/geode/logging/internal/OSProcess.java:200 | ||
| {code} | ||
| *Current Warnings:* | ||
| {code:java} | ||
| warning: [removal] SecurityManager in java.lang has been deprecated and marked for removal | ||
| warning: [removal] getSecurityManager() in System has been deprecated and marked for removal | ||
| {code} | ||
| h2. Technical Details | ||
|
|
||
| *Deprecated APIs:* | ||
| * {{java.lang.SecurityManager}} (class) | ||
| * {{java.lang.System.getSecurityManager()}} (method) | ||
|
|
||
| *Deprecation Timeline:* | ||
| * Deprecated: Java 17 (JEP 411 - September 2021) | ||
| * Status: REMOVED in Java 21 | ||
|
|
||
| *Package:* {{org.apache.geode.logging.internal}} | ||
| *Module:* {{geode-logging}} | ||
| *Warning Type:* {{[removal]}} | ||
| *Priority:* CRITICAL - Blocks Java 21 migration | ||
| h2. Required Actions | ||
| # Remove all {{SecurityManager}} references from {{OSProcess.java}} | ||
| # Replace {{System.getSecurityManager()}} calls with modern security patterns | ||
| # Refactor security checks to use alternative approaches (context-specific permissions, Java Security API, or application-level security) | ||
| # Verify the code compiles with Java 17 without warnings | ||
| # Ensure functionality is preserved after removal | ||
|
|
||
| h2. How to Verify Warnings | ||
|
|
||
| *Step 1:* Enable warnings in {{{}build-tools/scripts/src/main/groovy/warnings.gradle{}}}: | ||
| {code:groovy} | ||
| tasks.withType(JavaCompile) { | ||
| options.compilerArgs << '-Xlint:unchecked' << '-Xlint:deprecation' << '-Xlint:removal' | ||
| options.deprecation = true | ||
| } | ||
| {code} | ||
| *Step 2:* Build and check warnings: | ||
| {code:bash} | ||
| ./gradlew clean :geode-logging:compileJava --no-build-cache 2>&1 | grep "warning:" | ||
| {code} | ||
| *Step 3:* After fixing, restore original configuration: | ||
| {code:bash} | ||
| git checkout build-tools/scripts/src/main/groovy/warnings.gradle | ||
| {code} | ||
| h2. References | ||
| * [JEP 411: Deprecate the Security Manager for Removal|https://openjdk.org/jeps/411] | ||
| * [Java 17 Release Notes - Security Manager Deprecation|https://www.oracle.com/java/technologies/javase/17-relnote-issues.html#JDK-8199704] | ||
| * [Migration Guide: Removing SecurityManager Usage|https://docs.oracle.com/en/java/javase/17/migrate/migrating-jdk-8-later-jdk-releases.html#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82F] | ||
|
|
||
| h2. Acceptance Criteria | ||
| * All {{SecurityManager}} and {{System.getSecurityManager()}} references removed from {{OSProcess.java}} | ||
| * Code compiles without {{[removal]}} warnings in Java 17 | ||
| * Code compiles successfully in Java 21 | ||
| * All existing tests pass | ||
| * Security functionality is preserved or replaced with modern alternatives | ||
| * No new security vulnerabilities introduced | ||
|
|
||
| h2. Estimated Effort | ||
|
|
||
| *Story Points:* 3 | ||
| *Time Estimate:* 2-4 hours | ||
| h2. Impact | ||
|
|
||
| *Risk:* CRITICAL - This is a Java 21 blocker. The code will not compile in Java 21 without this fix. | ||
| *Scope:* 1 file, 1 location, 2 API references | ||
| *Related Work:* Part of Java 17 warning cleanup effort (36 total warnings across 6 modules) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| h2. Summary | ||
|
|
||
| Replace deprecated {{ClientHttpResponse.getRawStatusCode()}} method with {{getStatusCode().value()}} in {{{}HttpRequester.java{}}}. This API is marked for removal in Spring Framework 7.0. | ||
| h2. Description | ||
|
|
||
| The {{geode-gfsh}} module contains 3 usages of {{ClientHttpResponse.getRawStatusCode()}} that has been deprecated for removal in Spring Framework 6.0 and will be removed in Spring Framework 7.0 (estimated 2026). | ||
|
|
||
| *Affected File:* | ||
| {code:java} | ||
| geode-gfsh/src/main/java/org/apache/geode/management/internal/web/http/support/HttpRequester.java | ||
| {code} | ||
| *Affected Lines:* 113, 115, 117 | ||
|
|
||
| *Current Warnings:* | ||
| {code:java} | ||
| Line 113: warning: [removal] getRawStatusCode() in ClientHttpResponse has been deprecated and marked for removal | ||
| Line 115: warning: [removal] getRawStatusCode() in ClientHttpResponse has been deprecated and marked for removal | ||
| Line 117: warning: [removal] getRawStatusCode() in ClientHttpResponse has been deprecated and marked for removal | ||
| {code} | ||
| h2. Technical Details | ||
|
|
||
| *Deprecated API:* | ||
| * {{org.springframework.http.client.ClientHttpResponse.getRawStatusCode()}} | ||
|
|
||
| *Method Signature:* | ||
| {code:java} | ||
| int getRawStatusCode() throws IOException | ||
| {code} | ||
| *Deprecation Timeline:* | ||
| * Deprecated: Spring Framework 6.0 | ||
| * Removal Target: Spring Framework 7.0 (estimated 2026) | ||
| * Status: Marked for removal | ||
|
|
||
| *Replacement API:* | ||
| * {{org.springframework.http.HttpStatusCode.value()}} | ||
|
|
||
| *Package:* {{org.apache.geode.management.internal.web.http.support}} | ||
| *Module:* {{geode-gfsh}} | ||
| *Warning Type:* {{[removal]}} | ||
| *Priority:* HIGH - Blocks Spring Framework 7.0 migration | ||
| h2. Migration Pattern | ||
|
|
||
| *Before:* | ||
| {code:java} | ||
| int statusCode = response.getRawStatusCode(); | ||
| {code} | ||
| *After:* | ||
| {code:java} | ||
| int statusCode = response.getStatusCode().value(); | ||
| {code} | ||
| h2. Required Actions | ||
| # Locate all 3 occurrences of {{getRawStatusCode()}} in {{HttpRequester.java}} (lines 113, 115, 117) | ||
| # Replace {{response.getRawStatusCode()}} with {{response.getStatusCode().value()}} | ||
| # Verify the code compiles with Java 17 without warnings | ||
| # Run all GFSH-related tests to ensure HTTP status code handling works correctly | ||
| # Verify error handling and exception paths still function properly | ||
|
|
||
| h2. How to Verify Warnings | ||
|
|
||
| *Step 1:* Enable warnings in {{{}build-tools/scripts/src/main/groovy/warnings.gradle{}}}: | ||
| {code:groovy} | ||
| tasks.withType(JavaCompile) { | ||
| options.compilerArgs << '-Xlint:unchecked' << '-Xlint:deprecation' << '-Xlint:removal' | ||
| options.deprecation = true | ||
| } | ||
| {code} | ||
| *Step 2:* Build and check warnings: | ||
| {code:bash} | ||
| ./gradlew clean :geode-gfsh:compileJava --no-build-cache 2>&1 | grep "warning:" | ||
| {code} | ||
| *Step 3:* After fixing, restore original configuration: | ||
| {code:bash} | ||
| git checkout build-tools/scripts/src/main/groovy/warnings.gradle | ||
| {code} | ||
| h2. References | ||
| * [Spring Framework 6.0 Release Notes|https://github.com/spring-projects/spring-framework/wiki/What%27s-New-in-Spring-Framework-6.x] | ||
| * [ClientHttpResponse API Documentation|https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/http/client/ClientHttpResponse.html] | ||
| * [HttpStatusCode API Documentation|https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/http/HttpStatusCode.html] | ||
| * [Spring Framework Migration Guide|https://github.com/spring-projects/spring-framework/wiki/Upgrading-to-Spring-Framework-6.x] | ||
|
|
||
| h2. Acceptance Criteria | ||
| * All 3 occurrences of {{getRawStatusCode()}} replaced with {{getStatusCode().value()}} | ||
| * Code compiles without {{[removal]}} warnings in Java 17 | ||
| * All existing GFSH tests pass | ||
| * HTTP status code handling verified (success, error, and edge cases) | ||
| * No changes to public API behavior | ||
| * Exception handling remains consistent | ||
|
|
||
| h2. Test Coverage | ||
|
|
||
| Ensure the following scenarios are tested: | ||
| * Successful HTTP responses (200, 201, etc.) | ||
| * Client errors (400, 404, etc.) | ||
| * Server errors (500, 503, etc.) | ||
| * Edge cases (redirects, custom status codes) | ||
|
|
||
| h2. Impact | ||
|
|
||
| *Risk:* HIGH - This API will be removed in Spring Framework 7.0. The code will not compile without this fix when upgrading to Spring 7.x. | ||
| *Scope:* 1 file, 3 locations (lines 113, 115, 117) | ||
| *Related Work:* Part of Java 17 warning cleanup effort (36 total warnings across 6 modules) | ||
| *Reusability:* Same fix pattern applies to all 3 occurrences - straightforward replacement |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.