-
Notifications
You must be signed in to change notification settings - Fork 916
Adds DNS resolver configuration option for ApacheHttpClient #2834
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
{ | ||
"category": "Apache HTTP Client", | ||
"contributor": "", | ||
"type": "feature", | ||
"description": "Add DNS resolver override support for Apache HTTP Client" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ | |
package software.amazon.awssdk.http.apache; | ||
|
||
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; | ||
import static com.github.tomakehurst.wiremock.client.WireMock.any; | ||
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; | ||
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; | ||
import static org.mockito.Mockito.verify; | ||
|
@@ -25,21 +26,30 @@ | |
import com.github.tomakehurst.wiremock.client.WireMock; | ||
import com.github.tomakehurst.wiremock.junit.WireMockRule; | ||
import com.github.tomakehurst.wiremock.matching.RequestPatternBuilder; | ||
import java.io.IOException; | ||
import java.net.HttpURLConnection; | ||
import java.net.InetAddress; | ||
import java.net.URI; | ||
import java.net.UnknownHostException; | ||
import org.apache.http.HttpHost; | ||
import org.apache.http.auth.AuthScope; | ||
import org.apache.http.auth.Credentials; | ||
import org.apache.http.auth.UsernamePasswordCredentials; | ||
import org.apache.http.client.CredentialsProvider; | ||
import org.apache.http.conn.DnsResolver; | ||
import org.apache.http.conn.HttpClientConnectionManager; | ||
import org.apache.http.conn.routing.HttpRoute; | ||
import org.apache.http.impl.conn.SystemDefaultDnsResolver; | ||
import org.junit.Rule; | ||
import org.junit.Test; | ||
import org.junit.runner.RunWith; | ||
import org.mockito.Mock; | ||
import org.mockito.runners.MockitoJUnitRunner; | ||
import software.amazon.awssdk.http.HttpExecuteRequest; | ||
import software.amazon.awssdk.http.SdkHttpClient; | ||
import software.amazon.awssdk.http.SdkHttpClientTestSuite; | ||
import software.amazon.awssdk.http.SdkHttpFullRequest; | ||
import software.amazon.awssdk.http.SdkHttpMethod; | ||
import software.amazon.awssdk.http.apache.internal.ApacheHttpRequestConfig; | ||
import software.amazon.awssdk.http.apache.internal.impl.ConnectionManagerAwareHttpClient; | ||
import software.amazon.awssdk.utils.AttributeMap; | ||
|
@@ -148,4 +158,69 @@ public void clear() { | |
|
||
mockProxyServer.verify(2, RequestPatternBuilder.allRequests()); | ||
} | ||
|
||
@Test | ||
public void overrideDnsResolver_WithDnsMatchingResolver_successful() throws Exception { | ||
overrideDnsResolver("magic.local.host"); | ||
} | ||
|
||
@Test(expected = UnknownHostException.class) | ||
public void overrideDnsResolver_WithUnknownHost_throwsException() throws Exception { | ||
overrideDnsResolver("sad.local.host"); | ||
} | ||
|
||
@Test | ||
public void overrideDnsResolver_WithLocalhost_successful() throws Exception { | ||
overrideDnsResolver("localhost"); | ||
} | ||
|
||
@Test | ||
public void explicitNullDnsResolver_WithLocalhost_successful() throws Exception { | ||
overrideDnsResolver("localhost", true); | ||
} | ||
|
||
private void overrideDnsResolver(String hostName) throws IOException { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will take a look There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's used for the URI a bit further down |
||
overrideDnsResolver(hostName, false); | ||
} | ||
|
||
private void overrideDnsResolver(String hostName, boolean nullifyResolver) throws IOException { | ||
|
||
DnsResolver dnsResolver = new SystemDefaultDnsResolver() { | ||
@Override | ||
public InetAddress[] resolve(final String host) throws UnknownHostException { | ||
if (host.equalsIgnoreCase("magic.local.host")) { | ||
return new InetAddress[] { InetAddress.getByName("127.0.0.1") }; | ||
} else { | ||
return super.resolve(host); | ||
} | ||
} | ||
}; | ||
if (nullifyResolver) { | ||
dnsResolver = null; | ||
} | ||
|
||
SdkHttpClient client = ApacheHttpClient.builder() | ||
.dnsResolver(dnsResolver) | ||
.buildWithDefaults(AttributeMap.builder() | ||
.put(TRUST_ALL_CERTIFICATES, Boolean.TRUE) | ||
.build()); | ||
|
||
mockProxyServer.resetToDefaultMappings(); | ||
mockProxyServer.stubFor(any(urlPathEqualTo("/")).willReturn(aResponse().withStatus(HttpURLConnection.HTTP_OK))); | ||
|
||
URI uri = URI.create("https://" + hostName + ":" + mockProxyServer.httpsPort()); | ||
SdkHttpFullRequest req = SdkHttpFullRequest.builder() | ||
.uri(uri) | ||
.method(SdkHttpMethod.POST) | ||
.putHeader("Host", uri.getHost()) | ||
.build(); | ||
|
||
client.prepareRequest(HttpExecuteRequest.builder() | ||
.request(req) | ||
.contentStreamProvider(req.contentStreamProvider().orElse(null)) | ||
.build()) | ||
.call(); | ||
|
||
mockProxyServer.verify(1, RequestPatternBuilder.allRequests()); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add a test that if the caller explicitly sets to null we use the default resolver?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes