This is an automated email from the ASF dual-hosted git repository.
gwenshap pushed a commit to branch 2.0
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/2.0 by this push:
new 436bd77 KAFKA-7799; Use httpcomponents-client in RestServerTest.
436bd77 is described below
commit 436bd779a4039d0b33834a091b4d9de9ee702947
Author: Alex Diachenko <sansanic...@gmail.com>
AuthorDate: Tue Feb 19 11:52:07 2019 -0800
KAFKA-7799; Use httpcomponents-client in RestServerTest.
The test
`org.apache.kafka.connect.runtime.rest.RestServerTest#testCORSEnabled` assumes
Jersey client can send restricted HTTP headers(`Origin`).
Jersey client uses `sun.net.www.protocol.http.HttpURLConnection`.
`sun.net.www.protocol.http.HttpURLConnection` drops restricted
headers(`Host`, `Keep-Alive`, `Origin`, etc) based on static property
`allowRestrictedHeaders`.
This property is initialized in a static block by reading Java system
property `sun.net.http.allowRestrictedHeaders`.
So, if classloader loads `HttpURLConnection` before we set
`sun.net.http.allowRestrictedHeaders=true`, then all subsequent changes of this
system property won't take any effect(which happens if
`org.apache.kafka.connect.integration.ExampleConnectIntegrationTest` is
executed before `RestServerTest`).
To prevent this, we have to either make sure we set
`sun.net.http.allowRestrictedHeaders=true` as early as possible or do not rely
on this system property at all.
This PR adds test dependency on `httpcomponents-client` which doesn't
depend on `sun.net.http.allowRestrictedHeaders` system property. Thus none of
existing tests should interfere with `RestServerTest`.
Author: Alex Diachenko <sansanic...@gmail.com>
Reviewers: Gwen Shapira
Closes #6276 from avocader/KAFKA-7799_2.0
---
build.gradle | 1 +
checkstyle/import-control.xml | 1 +
.../kafka/connect/runtime/rest/RestServerTest.java | 138 +++++++++------------
gradle/dependencies.gradle | 4 +-
4 files changed, 63 insertions(+), 81 deletions(-)
diff --git a/build.gradle b/build.gradle
index 95d2df1..42aabea 100644
--- a/build.gradle
+++ b/build.gradle
@@ -1378,6 +1378,7 @@ project(':connect:runtime') {
testCompile libs.junit
testCompile libs.powermockJunit4
testCompile libs.powermockEasymock
+ testCompile libs.httpclient
testCompile project(':clients').sourceSets.test.output
testCompile project(':core')
diff --git a/checkstyle/import-control.xml b/checkstyle/import-control.xml
index 933dcbc..526f17f 100644
--- a/checkstyle/import-control.xml
+++ b/checkstyle/import-control.xml
@@ -330,6 +330,7 @@
<allow pkg="javax.servlet" />
<allow pkg="org.glassfish.jersey" />
<allow pkg="com.fasterxml.jackson" />
+ <allow pkg="org.apache.http"/>
</subpackage>
<subpackage name="isolation">
diff --git
a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/RestServerTest.java
b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/RestServerTest.java
index d084e4c..8824d77 100644
---
a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/RestServerTest.java
+++
b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/RestServerTest.java
@@ -16,6 +16,13 @@
*/
package org.apache.kafka.connect.runtime.rest;
+import org.apache.http.HttpHost;
+import org.apache.http.HttpRequest;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpOptions;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClients;
import org.apache.kafka.clients.CommonClientConfigs;
import org.apache.kafka.connect.rest.ConnectRestExtension;
import org.apache.kafka.connect.runtime.Herder;
@@ -28,30 +35,25 @@ import org.easymock.Capture;
import org.easymock.EasyMock;
import org.junit.After;
import org.junit.Assert;
-import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.powermock.api.easymock.PowerMock;
import org.powermock.api.easymock.annotation.MockStrict;
+import org.powermock.core.classloader.annotations.PowerMockIgnore;
import org.powermock.modules.junit4.PowerMockRunner;
-import java.net.URI;
-import java.net.URISyntaxException;
+import javax.ws.rs.core.MediaType;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
-import javax.ws.rs.client.Client;
-import javax.ws.rs.client.ClientBuilder;
-import javax.ws.rs.client.Invocation;
-import javax.ws.rs.client.WebTarget;
-import javax.ws.rs.core.MediaType;
-import javax.ws.rs.core.Response;
-
-import static org.junit.Assert.assertEquals;
@RunWith(PowerMockRunner.class)
+@PowerMockIgnore({"javax.net.ssl.*", "javax.security.*"})
public class RestServerTest {
@MockStrict
@@ -60,12 +62,6 @@ public class RestServerTest {
private Plugins plugins;
private RestServer server;
- @Before
- public void setUp() {
- // To be able to set the Origin, we need to toggle this flag
- System.setProperty("sun.net.http.allowRestrictedHeaders", "true");
- }
-
@After
public void tearDown() {
server.stop();
@@ -87,12 +83,12 @@ public class RestServerTest {
}
@Test
- public void testCORSEnabled() {
+ public void testCORSEnabled() throws IOException {
checkCORSRequest("*", "http://bar.com";, "http://bar.com";, "PUT");
}
@Test
- public void testCORSDisabled() {
+ public void testCORSDisabled() throws IOException {
checkCORSRequest("", "http://bar.com";, null, null);
}
@@ -163,7 +159,7 @@ public class RestServerTest {
}
@Test
- public void testOptionsDoesNotIncludeWadlOutput() {
+ public void testOptionsDoesNotIncludeWadlOutput() throws IOException {
Map<String, String> configMap = new HashMap<>(baseWorkerProps());
DistributedConfig workerConfig = new DistributedConfig(configMap);
@@ -177,19 +173,26 @@ public class RestServerTest {
server = new RestServer(workerConfig);
server.start(new HerderProvider(herder), herder.plugins());
- Response response = request("/connectors")
- .accept(MediaType.WILDCARD)
- .options();
- Assert.assertEquals(MediaType.TEXT_PLAIN_TYPE,
response.getMediaType());
+ HttpOptions request = new HttpOptions("/connectors");
+ request.addHeader("Content-Type", MediaType.WILDCARD);
+ CloseableHttpClient httpClient = HttpClients.createMinimal();
+ HttpHost httpHost = new HttpHost(
+ server.advertisedUrl().getHost(),
+ server.advertisedUrl().getPort()
+ );
+ CloseableHttpResponse response = httpClient.execute(httpHost, request);
+ Assert.assertEquals(MediaType.TEXT_PLAIN,
response.getEntity().getContentType().getValue());
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ response.getEntity().writeTo(baos);
Assert.assertArrayEquals(
- response.getAllowedMethods().toArray(new String[0]),
- response.readEntity(String.class).split(", ")
+ request.getAllowedMethods(response).toArray(),
+ new String(baos.toByteArray(), StandardCharsets.UTF_8).split(", ")
);
-
PowerMock.verifyAll();
}
- public void checkCORSRequest(String corsDomain, String origin, String
expectedHeader, String method) {
+ public void checkCORSRequest(String corsDomain, String origin, String
expectedHeader, String method)
+ throws IOException {
Map<String, String> workerProps = baseWorkerProps();
workerProps.put(WorkerConfig.ACCESS_CONTROL_ALLOW_ORIGIN_CONFIG,
corsDomain);
workerProps.put(WorkerConfig.ACCESS_CONTROL_ALLOW_METHODS_CONFIG,
method);
@@ -210,64 +213,39 @@ public class RestServerTest {
PowerMock.replayAll();
-
server = new RestServer(workerConfig);
server.start(new HerderProvider(herder), herder.plugins());
+ HttpRequest request = new HttpGet("/connectors");
+ request.addHeader("Referer", origin + "/page");
+ request.addHeader("Origin", origin);
+ CloseableHttpClient httpClient = HttpClients.createMinimal();
+ HttpHost httpHost = new HttpHost(
+ server.advertisedUrl().getHost(),
+ server.advertisedUrl().getPort()
+ );
+ CloseableHttpResponse response = httpClient.execute(httpHost, request);
- Response response = request("/connectors")
- .header("Referer", origin + "/page")
- .header("Origin", origin)
- .get();
- assertEquals(200, response.getStatus());
-
- assertEquals(expectedHeader,
response.getHeaderString("Access-Control-Allow-Origin"));
-
- response = request("/connector-plugins/FileStreamSource/validate")
- .header("Referer", origin + "/page")
- .header("Origin", origin)
- .header("Access-Control-Request-Method", method)
- .options();
- assertEquals(404, response.getStatus());
- assertEquals(expectedHeader,
response.getHeaderString("Access-Control-Allow-Origin"));
- assertEquals(method,
response.getHeaderString("Access-Control-Allow-Methods"));
- PowerMock.verifyAll();
- }
-
- protected Invocation.Builder request(String path) {
- return request(path, null, null, null);
- }
-
- protected Invocation.Builder request(String path, Map<String, String>
queryParams) {
- return request(path, null, null, queryParams);
- }
-
- protected Invocation.Builder request(String path, String templateName,
Object templateValue) {
- return request(path, templateName, templateValue, null);
- }
+ Assert.assertEquals(200, response.getStatusLine().getStatusCode());
- protected Invocation.Builder request(String path, String templateName,
Object templateValue,
- Map<String, String> queryParams) {
- Client client = ClientBuilder.newClient();
- WebTarget target;
- URI pathUri = null;
- try {
- pathUri = new URI(path);
- } catch (URISyntaxException e) {
- // Ignore, use restConnect and assume this is a valid path part
- }
- if (pathUri != null && pathUri.isAbsolute()) {
- target = client.target(path);
- } else {
- target = client.target(server.advertisedUrl()).path(path);
+ if (expectedHeader != null) {
+ Assert.assertEquals(expectedHeader,
+
response.getFirstHeader("Access-Control-Allow-Origin").getValue());
}
- if (templateName != null && templateValue != null) {
- target = target.resolveTemplate(templateName, templateValue);
+
+ request = new
HttpOptions("/connector-plugins/FileStreamSource/validate");
+ request.addHeader("Referer", origin + "/page");
+ request.addHeader("Origin", origin);
+ request.addHeader("Access-Control-Request-Method", method);
+ response = httpClient.execute(httpHost, request);
+ Assert.assertEquals(404, response.getStatusLine().getStatusCode());
+ if (expectedHeader != null) {
+ Assert.assertEquals(expectedHeader,
+
response.getFirstHeader("Access-Control-Allow-Origin").getValue());
}
- if (queryParams != null) {
- for (Map.Entry<String, String> queryParam :
queryParams.entrySet()) {
- target = target.queryParam(queryParam.getKey(),
queryParam.getValue());
- }
+ if (method != null) {
+ Assert.assertEquals(method,
+
response.getFirstHeader("Access-Control-Allow-Methods").getValue());
}
- return target.request();
+ PowerMock.verifyAll();
}
}
diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle
index 08d8bf7..d3f70d1 100644
--- a/gradle/dependencies.gradle
+++ b/gradle/dependencies.gradle
@@ -52,6 +52,7 @@ versions += [
apacheds: "2.0.0-M24",
argparse4j: "0.7.0",
bcpkix: "1.59",
+ httpclient: "4.5.7",
easymock: "3.6",
jackson: "2.9.7",
jetty: "9.4.11.v20180605",
@@ -140,5 +141,6 @@ libs += [
zkclient: "com.101tec:zkclient:$versions.zkclient",
zookeeper: "org.apache.zookeeper:zookeeper:$versions.zookeeper",
jfreechart: "jfreechart:jfreechart:$versions.jfreechart",
- mavenArtifact: "org.apache.maven:maven-artifact:$versions.mavenArtifact"
+ mavenArtifact: "org.apache.maven:maven-artifact:$versions.mavenArtifact",
+ httpclient: "org.apache.httpcomponents:httpclient:$versions.httpclient"
]
