kafka-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From gwens...@apache.org
Subject [kafka] branch trunk updated: KAFKA-7799; Use httpcomponents-client in RestServerTest.
Date Tue, 12 Feb 2019 20:03:35 GMT
This is an automated email from the ASF dual-hosted git repository.

gwenshap pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git


The following commit(s) were added to refs/heads/trunk by this push:
     new ec42e03  KAFKA-7799; Use httpcomponents-client in RestServerTest.
ec42e03 is described below

commit ec42e0378e7c66b8ceadc480bca6f98ad1df0643
Author: Alex Diachenko <sansanichfb@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 <sansanichfb@gmail.com>
    
    Reviewers: Randall Hauch, Konstantine Karantasis, Gwen Shapira
    
    Closes #6236 from avocader/KAFKA-7799
---
 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 67b9c3a..9d8af44 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"
 ]


Mime
View raw message