Skip to content

[apache5-client] Fail fast when SecurityManager lacks TCP_KEEPIDLE/TCP_KEEPINTERVAL/TCP_KEEPCOUNT permissions.#6992

Open
joviegas wants to merge 6 commits into
masterfrom
joviegas/handle_apache_for_security_manager
Open

[apache5-client] Fail fast when SecurityManager lacks TCP_KEEPIDLE/TCP_KEEPINTERVAL/TCP_KEEPCOUNT permissions.#6992
joviegas wants to merge 6 commits into
masterfrom
joviegas/handle_apache_for_security_manager

Conversation

@joviegas
Copy link
Copy Markdown
Contributor

Motivation and Context

  • Apache HC5 5.6 defaults SocketConfig.Builder to tcpKeepIdle=5, tcpKeepInterval=5, tcpKeepCount=3 with httpcomponents-core#550 (Enable five-second TCP keep-alive by default apache/httpcomponents-core#550)
  • This requires jdk.net.NetworkPermission for setOption.TCP_KEEPIDLE, setOption.TCP_KEEPINTERVAL, and setOption.TCP_KEEPCOUNT. When a SecurityManager is active without these permissions granted, customers get an AccessControlException at request time with no indication of which permissions are missing.
  • This change throws an IllegalStateException at client construction time listing the exact permissions required, so customers know what to fix before sending any requests.

Modifications

  • Added checkTcpKeepAlivePermissions() in Apache5HttpClient.DefaultBuilder.buildWithDefaults() that checks the three required jkdk.net.NetworkPermission entries when a SecurityManager is active.
  • Throws IllegalStateException listing the missing permissions at client construction time instead of AccessControlException at first request.
  • Uses ClassLoaderHelper.loadClass() to reflectively load jdk.net.NetworkPermission since the class is in an optional JDK module.

Testing

  • Added Junit tests

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

License

  • I confirm that this pull request can be released under the Apache 2 license

…ctive and jdk.net.NetworkPermission setOption.TCP_KEEPIDLE, setOption.TCP_KEEPINTERVAL, setOption.TCP_KEEPCOUNT are not granted
@joviegas joviegas requested a review from a team as a code owner May 26, 2026 21:02
@joviegas joviegas changed the title [[apache5-client] Fail fast when SecurityManager lacks TCP_KEEPIDLE/TCP_KEEPINTERVAL/TCP_KEEPCOUNT permissions. [apache5-client] Fail fast when SecurityManager lacks TCP_KEEPIDLE/TCP_KEEPINTERVAL/TCP_KEEPCOUNT permissions. May 26, 2026
}

try {
Class<?> permClass = ClassLoaderHelper.loadClass("jdk.net.NetworkPermission", Apache5HttpClient.class);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that NetworkPermission is marked for forRemoval in Java 25 thus loading it from ClassLoaderHelper to prevent ClassNotFoundException for future Java versions.
https://docs.oracle.com/en/java/javase/26/docs/api/jdk.net/jdk/net/NetworkPermission.html

@joviegas joviegas force-pushed the joviegas/handle_apache_for_security_manager branch from b696460 to 697b152 Compare May 27, 2026 14:41
@dagnir
Copy link
Copy Markdown
Contributor

dagnir commented May 27, 2026

Not a blocker, but can we add a test that uses a security manager with expected permissions enabled + makes an API call so we can catch any instances where more permissions have to be granted, for example in a new Apache 5.x minor version.

@joviegas
Copy link
Copy Markdown
Contributor Author

Not a blocker, but can we add a test that uses a security manager with expected permissions enabled + makes an API call so we can catch any instances where more permissions have to be granted, for example in a new Apache 5.x minor version.

Good call out.
Added SdkHttpClientSecurityManagerTestSuite which does SecurityManager test for both Apache4 and Apache5. The only difference in the permission between these two clients are for TCP_KEEPIDLE/TCP_KEEPINTERVAL/TCP_KEEPCOUNT .

@joviegas joviegas added the api-surface-area-approved-by-team Indicate API surface area introduced by this PR has been approved by team label May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-surface-area-approved-by-team Indicate API surface area introduced by this PR has been approved by team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants