You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/05/06 23:19:13 UTC
[GitHub] [kafka] jeffhuang26 commented on a change in pull request #8620: KAFKA-9944: Added supporting customized HTTP response headers for Kafka Connect.
jeffhuang26 commented on a change in pull request #8620:
URL: https://github.com/apache/kafka/pull/8620#discussion_r421147218
##########
File path: connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/RestServerTest.java
##########
@@ -392,6 +395,98 @@ public void testDisableAdminEndpoint() throws IOException {
Assert.assertEquals(404, response.getStatusLine().getStatusCode());
}
+ @Test
+ public void testValidCustomizedHttpResponseHeaders() throws IOException {
+ String headerConfig =
+ "add X-XSS-Protection: 1; mode=block, \"add Cache-Control: no-cache, no-store, must-revalidate\"";
+ Map<String, String> expectedHeaders = new HashMap<>();
+ expectedHeaders.put("X-XSS-Protection", "1; mode=block");
+ expectedHeaders.put("Cache-Control", "no-cache, no-store, must-revalidate");
+ checkCustomizedHttpResponseHeaders(headerConfig, expectedHeaders);
+ }
+
+ @Test
+ public void testDefaultCustomizedHttpResponseHeaders() throws IOException {
+ String headerConfig = "";
+ Map<String, String> expectedHeaders = new HashMap<>();
+ checkCustomizedHttpResponseHeaders(headerConfig, expectedHeaders);
+ }
+
+ @Test(expected = ConfigException.class)
+ public void testInvalidHeaderConfigFormat() {
+ String headerConfig = "set add X-XSS-Protection: 1";
+ Map<String, String> workerProps = baseWorkerProps();
+ workerProps.put(WorkerConfig.RESPONSE_HTTP_HEADERS_CONFIG, headerConfig);
+ WorkerConfig workerConfig = new DistributedConfig(workerProps);
+ }
+
+ @Test(expected = ConfigException.class)
+ public void testMissedAction() {
+ String headerConfig = "X-Frame-Options: DENY";
+ Map<String, String> workerProps = baseWorkerProps();
+ workerProps.put(WorkerConfig.RESPONSE_HTTP_HEADERS_CONFIG, headerConfig);
+ WorkerConfig workerConfig = new DistributedConfig(workerProps);
+ }
+
+ @Test(expected = ConfigException.class)
+ public void testMissedHeaderName() {
+ String headerConfig = "add :DENY";
+ Map<String, String> workerProps = baseWorkerProps();
+ workerProps.put(WorkerConfig.RESPONSE_HTTP_HEADERS_CONFIG, headerConfig);
+ WorkerConfig workerConfig = new DistributedConfig(workerProps);
+ }
+
+ @Test(expected = ConfigException.class)
+ public void testMissedHeaderValue() {
+ String headerConfig = "add X-Frame-Options";
+ Map<String, String> workerProps = baseWorkerProps();
+ workerProps.put(WorkerConfig.RESPONSE_HTTP_HEADERS_CONFIG, headerConfig);
+ WorkerConfig workerConfig = new DistributedConfig(workerProps);
+ }
+
+ @Test(expected = ConfigException.class)
+ public void testInvalidHeaderConfigAction() {
+ String headerConfig = "badaction X-XSS-Protection: 1; mode=block";
+ Map<String, String> workerProps = baseWorkerProps();
+ workerProps.put(WorkerConfig.RESPONSE_HTTP_HEADERS_CONFIG, headerConfig);
+ WorkerConfig workerConfig = new DistributedConfig(workerProps);
+ }
+
+ public void checkCustomizedHttpResponseHeaders(String headerConfig, Map<String, String> expectedHeaders)
+ throws IOException {
+ Map<String, String> workerProps = baseWorkerProps();
+ workerProps.put("offset.storage.file.filename", "/tmp");
+ workerProps.put(WorkerConfig.RESPONSE_HTTP_HEADERS_CONFIG, headerConfig);
+ WorkerConfig workerConfig = new DistributedConfig(workerProps);
+
+ EasyMock.expect(herder.kafkaClusterId()).andReturn(KAFKA_CLUSTER_ID);
+ EasyMock.expect(herder.plugins()).andStubReturn(plugins);
+ EasyMock.expect(plugins.newPlugins(Collections.emptyList(),
+ workerConfig,
+ ConnectRestExtension.class)).andStubReturn(Collections.emptyList());
+
+ EasyMock.expect(herder.connectors()).andReturn(Arrays.asList("a", "b"));
+
+ PowerMock.replayAll();
+
+ server = new RestServer(workerConfig);
+ server.initializeServer();
+ server.initializeResources(herder);
+ HttpRequest request = new HttpGet("/connectors");
+ CloseableHttpClient httpClient = HttpClients.createMinimal();
+ HttpHost httpHost = new HttpHost(server.advertisedUrl().getHost(), server.advertisedUrl().getPort());
+ CloseableHttpResponse response = httpClient.execute(httpHost, request);
+ Assert.assertEquals(200, response.getStatusLine().getStatusCode());
+ if (!headerConfig.isEmpty()) {
+ expectedHeaders.forEach((k, v) ->
+ Assert.assertEquals(response.getFirstHeader(k).getValue(), v));
+ } else {
+ Assert.assertNull(response.getFirstHeader("X-Frame-Options"));
+ }
+ response.close();
+ server.stop();
+ }
+
Review comment:
It is good idea to assert each checking using assertValidHeaderConfig and assertInvalidHeaderConfig. I think latest version will cover validation of header config. Please let me know.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org