You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kafka.apache.org by gw...@apache.org on 2019/02/14 23:57:34 UTC
[kafka] branch 2.2 updated: KAFKA-7799;
Use httpcomponents-client in RestServerTest.
This is an automated email from the ASF dual-hosted git repository.
gwenshap pushed a commit to branch 2.2
in repository https://gitbox.apache.org/repos/asf/kafka.git
The following commit(s) were added to refs/heads/2.2 by this push:
new 714995b KAFKA-7799; Use httpcomponents-client in RestServerTest.
714995b is described below
commit 714995bbb48904ae8f21ab588c7544a99e4ba494
Author: Alex Diachenko <sa...@gmail.com>
AuthorDate: Tue Feb 12 12:03:08 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 <sa...@gmail.com>
Reviewers: Randall Hauch, Konstantine Karantasis, Gwen Shapira
Closes #6236 from avocader/KAFKA-7799
(cherry picked from commit ec42e0378e7c66b8ceadc480bca6f98ad1df0643)
---
build.gradle | 1 +
checkstyle/import-control.xml | 1 +
.../kafka/connect/runtime/rest/RestServerTest.java | 138 +++++++++------------
gradle/dependencies.gradle | 2 +
4 files changed, 62 insertions(+), 80 deletions(-)
diff --git a/build.gradle b/build.gradle
index 8915fe8..420edf7 100644
--- a/build.gradle
+++ b/build.gradle
@@ -1482,6 +1482,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 0d316c5..4702011 100644
--- a/checkstyle/import-control.xml
+++ b/checkstyle/import-control.xml
@@ -341,6 +341,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 d5802cb..7b98554 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();
@@ -88,12 +84,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);
}
@@ -166,7 +162,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);
@@ -180,19 +176,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);
@@ -213,64 +216,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 9eac3d4..a4284df 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.60",
+ httpclient: "4.5.7",
easymock: "4.0.2",
jackson: "2.9.8",
jetty: "9.4.14.v20181114",
@@ -150,4 +151,5 @@ libs += [
jfreechart: "jfreechart:jfreechart:$versions.jfreechart",
mavenArtifact: "org.apache.maven:maven-artifact:$versions.mavenArtifact",
zstd: "com.github.luben:zstd-jni:$versions.zstd",
+ httpclient: "org.apache.httpcomponents:httpclient:$versions.httpclient"
]