Skip to content

Clean up unused APIs and add test to make sure it can be handled with alternatives #6211

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

Merged
merged 2 commits into from
Jun 26, 2025

Conversation

joviegas
Copy link
Contributor

@joviegas joviegas commented Jun 25, 2025

Remove local address API from Apache5HttpClient and provide HttpRoutePlanner alternative

Motivation and Context

  • Removes the direct localAddress API from Apache5HttpClient to simplify the client interface while ensuring equivalent functionality remains available through alternative mechanisms.
  • Add NTLM credentials to keep backward compatibilty with Apach4.x since Apache5.x will be made default client down the line.

Also refer https://stackoverflow.com/questions/78113769/apache-httpclient5-how-to-set-localaddress-per-request

Modifications

  • Removed APIs:
    • Apache5HttpClient.Builder.localAddress(InetAddress) method and related configuration
    • InputShutdownCheckingSslSocket wrapper class for SSL socket handling
  • Added alternatives:
    • HttpRoutePlanner-based approach for local address configuration in Apache5 client
    • Comprehensive test suite SdkHttpClientLocalAddressFunctionalTestSuite for local address functionality
  • Updated socket handling: Simplified SSL socket creation without the input shutdown checking wrapper
  • New TLS half-close tests: Added dedicated tests to verify behavior in TLS half-close scenarios

Testing

  • Added Apache5HttpClientLocalAddressFunctionalTest demonstrating HttpRoutePlanner-based local address configuration
  • Added ApacheHttpClientLocalAddressFunctionalTest for Apache client local address testing
  • Comprehensive test coverage in SdkHttpClientLocalAddressFunctionalTestSuite with valid/invalid address scenarios
  • TLS half-close behavior testing in Apache5ClientTlsHalfCloseTest
  • Removed obsolete tests for deleted InputShutdownCheckingSslSocket class

The removed APIs have confirmed alternatives with full test coverage ensuring no functionality loss.

localAddress support for Apache5x

import java.net.InetAddress;
import org.apache.hc.client5.http.impl.DefaultSchemePortResolver;
import org.apache.hc.client5.http.impl.routing.DefaultRoutePlanner;
import org.apache.hc.client5.http.routing.HttpRoutePlanner;
import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.protocol.HttpContext;
import software.amazon.awssdk.http.apache5.Apache5HttpClient;

// Example: Setting local address to localhost
InetAddress localAddress = InetAddress.getByName("127.0.0.1");

// Create custom HttpRoutePlanner that uses the specified local address
HttpRoutePlanner routePlanner = new DefaultRoutePlanner(DefaultSchemePortResolver.INSTANCE) {
    @Override
    protected InetAddress determineLocalAddress(HttpHost firstHop, HttpContext context) throws HttpException {
        return localAddress; // Return your desired local address
    }
};

// Build the Apache5HttpClient with the custom route planner
Apache5HttpClient httpClient = Apache5HttpClient.builder()
                                                .httpRoutePlanner(routePlanner)
                                                .build();

License

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

@joviegas joviegas requested a review from a team as a code owner June 25, 2025 21:40
@@ -62,9 +60,6 @@ public Socket connectSocket(TimeValue connectTimeout,
log.trace(() -> String.format("Connecting to %s:%s", remoteAddress.getAddress(), remoteAddress.getPort()));

Socket connectSocket = super.connectSocket(connectTimeout, socket, host, remoteAddress, localAddress, context);
if (connectSocket instanceof SSLSocket) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we test this with Apache5ClientTlsHalfCloseTest so that functionality is same

@joviegas joviegas force-pushed the joviegas/apache_cleanup_unused branch from 7e2a5a1 to aa489a5 Compare June 25, 2025 22:18
@joviegas joviegas force-pushed the joviegas/apache_cleanup_unused branch from 05de44b to 4ea443c Compare June 25, 2025 23:35
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
66.0% Coverage on New Code (required ≥ 80%)
18.7% Duplication on New Code (required ≤ 3%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@joviegas joviegas merged commit 3cc94bb into feature/master/apache5x Jun 26, 2025
25 of 26 checks passed
Copy link

This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants