You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by jo...@apache.org on 2020/01/16 18:15:17 UTC
[nifi] branch master updated: NIFI-7023 This closes #3991. Removed
SLF4J and Log4J transitive dependencies from Zookeeper so tests log
correctly. Changed template handling. Added unit tests.
This is an automated email from the ASF dual-hosted git repository.
joewitt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nifi.git
The following commit(s) were added to refs/heads/master by this push:
new 34f2a59 NIFI-7023 This closes #3991. Removed SLF4J and Log4J transitive dependencies from Zookeeper so tests log correctly. Changed template handling. Added unit tests.
34f2a59 is described below
commit 34f2a592df8996b5f9e65039a35ecd8c31417fbd
Author: Andy LoPresto <al...@apache.org>
AuthorDate: Wed Jan 15 19:16:57 2020 -0800
NIFI-7023 This closes #3991. Removed SLF4J and Log4J transitive dependencies from Zookeeper so tests log correctly.
Changed template handling.
Added unit tests.
Signed-off-by: Joe Witt <jo...@apache.org>
---
.../nifi-framework/nifi-framework-core/pom.xml | 10 ++
.../apache/nifi/web/api/ProcessGroupResource.java | 137 ++++++++--------
.../nifi/web/api/ProcessGroupResourceTest.groovy | 172 +++++++++++++++++++++
3 files changed, 254 insertions(+), 65 deletions(-)
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/pom.xml b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/pom.xml
index 07467eb..6ef7cc2 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/pom.xml
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/pom.xml
@@ -161,6 +161,16 @@
<dependency>
<groupId>org.apache.zookeeper</groupId>
<artifactId>zookeeper</artifactId>
+ <exclusions>
+ <exclusion>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-log4j12</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>log4j</groupId>
+ <artifactId>log4j</artifactId>
+ </exclusion>
+ </exclusions>
</dependency>
<dependency>
<groupId>org.apache.curator</groupId>
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProcessGroupResource.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProcessGroupResource.java
index 142e25c..d3b0b5b 100644
--- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProcessGroupResource.java
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProcessGroupResource.java
@@ -24,7 +24,58 @@ import io.swagger.annotations.ApiParam;
import io.swagger.annotations.ApiResponse;
import io.swagger.annotations.ApiResponses;
import io.swagger.annotations.Authorization;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.UUID;
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import javax.servlet.http.HttpServletRequest;
+import javax.ws.rs.Consumes;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.GET;
+import javax.ws.rs.HttpMethod;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.Context;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.MultivaluedHashMap;
+import javax.ws.rs.core.MultivaluedMap;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.core.Response.Status;
+import javax.ws.rs.core.UriBuilder;
+import javax.ws.rs.core.UriInfo;
+import javax.xml.bind.JAXBContext;
+import javax.xml.bind.JAXBElement;
+import javax.xml.bind.JAXBException;
+import javax.xml.bind.Unmarshaller;
+import javax.xml.stream.XMLStreamReader;
import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.text.StringEscapeUtils;
import org.apache.nifi.authorization.AuthorizableLookup;
import org.apache.nifi.authorization.AuthorizeAccess;
import org.apache.nifi.authorization.AuthorizeControllerServiceReference;
@@ -120,57 +171,6 @@ import org.slf4j.LoggerFactory;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
-import javax.servlet.http.HttpServletRequest;
-import javax.ws.rs.Consumes;
-import javax.ws.rs.DELETE;
-import javax.ws.rs.DefaultValue;
-import javax.ws.rs.GET;
-import javax.ws.rs.HttpMethod;
-import javax.ws.rs.POST;
-import javax.ws.rs.PUT;
-import javax.ws.rs.Path;
-import javax.ws.rs.PathParam;
-import javax.ws.rs.Produces;
-import javax.ws.rs.QueryParam;
-import javax.ws.rs.core.Context;
-import javax.ws.rs.core.HttpHeaders;
-import javax.ws.rs.core.MediaType;
-import javax.ws.rs.core.MultivaluedHashMap;
-import javax.ws.rs.core.MultivaluedMap;
-import javax.ws.rs.core.Response;
-import javax.ws.rs.core.Response.Status;
-import javax.ws.rs.core.UriBuilder;
-import javax.ws.rs.core.UriInfo;
-import javax.xml.bind.JAXBContext;
-import javax.xml.bind.JAXBElement;
-import javax.xml.bind.JAXBException;
-import javax.xml.bind.Unmarshaller;
-import javax.xml.stream.XMLStreamReader;
-import java.io.IOException;
-import java.io.InputStream;
-import java.net.URI;
-import java.net.URISyntaxException;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Set;
-import java.util.UUID;
-import java.util.concurrent.ArrayBlockingQueue;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ThreadFactory;
-import java.util.concurrent.ThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
-import java.util.function.Consumer;
-import java.util.function.Function;
-import java.util.stream.Collectors;
-
/**
* RESTful endpoint for managing a Group.
*/
@@ -1100,18 +1100,10 @@ public class ProcessGroupResource extends ApplicationResource {
return false;
}
- if (desiredState == ScheduledState.STOPPED && status.getAggregateSnapshot().getActiveThreadCount() != 0) {
- return false;
- }
-
- return true;
+ return desiredState != ScheduledState.STOPPED || status.getAggregateSnapshot().getActiveThreadCount() == 0;
});
- if (!allProcessorsMatch) {
- return false;
- }
-
- return true;
+ return allProcessorsMatch;
}
/**
@@ -3631,6 +3623,7 @@ public class ProcessGroupResource extends ApplicationResource {
// unmarshal the template
final TemplateDTO template;
try {
+ // TODO: Potentially refactor the template parsing to a service layer outside of the resource for web request handling
JAXBContext context = JAXBContext.newInstance(TemplateDTO.class);
Unmarshaller unmarshaller = context.createUnmarshaller();
XMLStreamReader xsr = XmlUtils.createSafeReader(in);
@@ -3642,12 +3635,12 @@ public class ProcessGroupResource extends ApplicationResource {
return Response.status(Response.Status.OK).entity(responseXml).type("application/xml").build();
} catch (IllegalArgumentException iae) {
logger.warn("Unable to import template.", iae);
- String responseXml = String.format("<errorResponse status=\"%s\" statusText=\"%s\"/>", Response.Status.BAD_REQUEST.getStatusCode(), iae.getMessage());
+ String responseXml = String.format("<errorResponse status=\"%s\" statusText=\"%s\"/>", Response.Status.BAD_REQUEST.getStatusCode(), sanitizeErrorResponse(iae.getMessage()));
return Response.status(Response.Status.OK).entity(responseXml).type("application/xml").build();
} catch (Exception e) {
logger.warn("An error occurred while importing a template.", e);
String responseXml = String.format("<errorResponse status=\"%s\" statusText=\"Unable to import the specified template: %s\"/>",
- Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), e.getMessage());
+ Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), sanitizeErrorResponse(e.getMessage()));
return Response.status(Response.Status.OK).entity(responseXml).type("application/xml").build();
}
@@ -3684,6 +3677,20 @@ public class ProcessGroupResource extends ApplicationResource {
}
/**
+ * Returns the sanitized error response which can safely be displayed on the error page.
+ *
+ * @param errorResponse the initial error response
+ * @return the HTML-escaped error response
+ */
+ private String sanitizeErrorResponse(String errorResponse) {
+ if (errorResponse == null || StringUtils.isEmpty(errorResponse)) {
+ return "";
+ }
+
+ return StringEscapeUtils.escapeHtml4(errorResponse);
+ }
+
+ /**
* Imports the specified template.
*
* @param httpServletRequest request
@@ -3751,12 +3758,12 @@ public class ProcessGroupResource extends ApplicationResource {
return generateCreatedResponse(URI.create(template.getUri()), entity).build();
} catch (IllegalArgumentException | IllegalStateException e) {
logger.info("Unable to import template: " + e);
- String responseXml = String.format("<errorResponse status=\"%s\" statusText=\"%s\"/>", Response.Status.BAD_REQUEST.getStatusCode(), e.getMessage());
+ String responseXml = String.format("<errorResponse status=\"%s\" statusText=\"%s\"/>", Response.Status.BAD_REQUEST.getStatusCode(), sanitizeErrorResponse(e.getMessage()));
return Response.status(Response.Status.OK).entity(responseXml).type("application/xml").build();
} catch (Exception e) {
logger.warn("An error occurred while importing a template.", e);
String responseXml = String.format("<errorResponse status=\"%s\" statusText=\"Unable to import the specified template: %s\"/>",
- Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), e.getMessage());
+ Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), sanitizeErrorResponse(e.getMessage()));
return Response.status(Response.Status.OK).entity(responseXml).type("application/xml").build();
}
}
diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/groovy/org/apache/nifi/web/api/ProcessGroupResourceTest.groovy b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/groovy/org/apache/nifi/web/api/ProcessGroupResourceTest.groovy
new file mode 100644
index 0000000..8ecf905
--- /dev/null
+++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/groovy/org/apache/nifi/web/api/ProcessGroupResourceTest.groovy
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.web.api
+
+import org.apache.nifi.authorization.AuthorizeAccess
+import org.apache.nifi.util.NiFiProperties
+import org.apache.nifi.web.NiFiServiceFacade
+import org.apache.nifi.web.api.dto.FlowSnippetDTO
+import org.apache.nifi.web.api.dto.TemplateDTO
+import org.apache.nifi.web.api.entity.TemplateEntity
+import org.junit.After
+import org.junit.Before
+import org.junit.BeforeClass
+import org.junit.Rule
+import org.junit.Test
+import org.junit.rules.TestName
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
+import javax.servlet.http.HttpServletRequest
+import javax.ws.rs.core.Response
+import javax.ws.rs.core.UriInfo
+
+@RunWith(JUnit4.class)
+class ProcessGroupResourceTest extends GroovyTestCase {
+ private static final Logger logger = LoggerFactory.getLogger(ProcessGroupResourceTest.class)
+
+ @Rule
+ public TestName testName = new TestName()
+
+ @BeforeClass
+ static void setUpOnce() throws Exception {
+ logger.metaClass.methodMissing = { String name, args ->
+ logger.debug("[${name?.toUpperCase()}] ${(args as List).join(" ")}")
+ }
+ }
+
+ @Before
+ void setUp() throws Exception {
+ }
+
+ @After
+ void tearDown() throws Exception {
+ }
+
+ /** This test creates a malformed template upload request to exercise error handling and sanitization */
+ @Test
+ void testUploadShouldHandleMalformedTemplate() {
+ // Arrange
+ ProcessGroupResource pgResource = new ProcessGroupResource()
+
+ // Mocking the returned template object to throw a specific exception would be nice
+ final String TEMPLATE_WITH_XSS_PLAIN = "<?xml version=\"1.0\" encoding='><script xmlns=\"http://www.w3.org/1999/xhtml\">alert(JSON.stringify(localstorage));</script><errorResponse test='?>"
+ logger.info("Malformed template XML: ${TEMPLATE_WITH_XSS_PLAIN}")
+ InputStream contentInputStream = new ByteArrayInputStream(TEMPLATE_WITH_XSS_PLAIN.bytes)
+
+ HttpServletRequest mockRequest = [:] as HttpServletRequest
+ UriInfo mockUriInfo = [:] as UriInfo
+ String groupId = "1"
+
+ // Build a malformed template object which can be unmarshalled from XML
+
+ // Act
+
+ // Try to submit the malformed template
+ Response response = pgResource.uploadTemplate(mockRequest, mockUriInfo, groupId, false, contentInputStream)
+ logger.info("Response: ${response}")
+
+ // Assert
+
+ // Assert that the expected error response was returned
+ assert response.status == Response.Status.OK.statusCode
+
+ // Assert that the error response is sanitized
+ String responseEntity = response.entity as String
+ logger.info("Error response: ${responseEntity}")
+ assert !(responseEntity =~ /<script.*>/)
+ }
+
+ /** This test creates a malformed template import request to exercise error handling and sanitization */
+ @Test
+ void testImportShouldHandleMalformedTemplate() {
+ // Arrange
+ ProcessGroupResource pgResource = new ProcessGroupResource()
+
+ // Configure parent fields for write lock process
+ pgResource.properties = [isNode: { -> return false }] as NiFiProperties
+ pgResource.serviceFacade = [
+ authorizeAccess : { AuthorizeAccess a -> },
+ verifyCanAddTemplate: { String gid, String templateName -> },
+ importTemplate : { TemplateDTO template, String gid, Optional<String> seedId ->
+ logger.mock("Called importTemplate;")
+ template
+ }
+ ] as NiFiServiceFacade
+ pgResource.templateResource = [
+ populateRemainingTemplateContent: { TemplateDTO td -> }
+ ] as TemplateResource
+
+ final String TEMPLATE_WITH_XSS_PLAIN = "<?xml version=\"1.0\" encoding='><script xmlns=\"http://www.w3.org/1999/xhtml\">alert(JSON.stringify(localstorage));</script><errorResponse test='?>"
+ logger.info("Malformed template XML: ${TEMPLATE_WITH_XSS_PLAIN}")
+
+ TemplateDTO mockIAETemplate = [
+ getName : { -> "mockIAETemplate" },
+ getUri : { ->
+ throw new IllegalArgumentException("Expected exception with <script> element")
+ },
+ getSnippet: { -> new FlowSnippetDTO() }
+ ] as TemplateDTO
+
+ TemplateDTO mockExceptionTemplate = [
+ getName : { -> "mockExceptionTemplate" },
+ getUri : { ->
+ throw new RuntimeException("Expected exception with <script> element")
+ },
+ getSnippet: { -> new FlowSnippetDTO() }
+ ] as TemplateDTO
+
+ TemplateEntity mockIAETemplateEntity = [getTemplate: { ->
+ mockIAETemplate
+ }] as TemplateEntity
+
+ TemplateEntity mockExceptionTemplateEntity = [getTemplate: { ->
+ mockExceptionTemplate
+ }] as TemplateEntity
+
+ // Override the request object and store it for ApplicationResource#withWriteLock
+ HttpServletRequest mockRequest = [getHeader: { String headerName ->
+ logger.mock("Requesting header ${headerName}; returning null")
+ null
+ }] as HttpServletRequest
+
+ // Set the persisted request object so the parent ApplicationResource can use it
+ pgResource.httpServletRequest = mockRequest
+ String groupId = "1"
+
+ // Act
+ List<Response> responses = [mockIAETemplateEntity, mockExceptionTemplateEntity].collect { TemplateEntity te ->
+ // Try to submit the malformed template which throws some kind of exception
+ Response response = pgResource.importTemplate(mockRequest, groupId, te)
+ logger.info("Response: ${response}")
+ response
+ }
+
+ // Assert
+ responses.each { Response r ->
+ // Assert that the expected error response was returned
+ assert r.status == Response.Status.OK.statusCode
+
+ // Assert that the error response is sanitized
+ String entity = r.entity as String
+ logger.info("Error response: ${entity}")
+ assert !(entity =~ /<script.*>/)
+ }
+ }
+}