From fccd392054c1c43ac61add9bd456ab6ede367693 Mon Sep 17 00:00:00 2001 From: thenatog Date: Wed, 30 May 2018 16:53:28 -0400 Subject: [PATCH 1/2] NIFI-5258 - Changed addHeader to setHeader which stops X-Frame-Options being added twice to responses. Changed the way docs webapp is created to allow adding the X-Frame-Options header to docs resources too. Added test to check response header. NIFI-5258 - Added unit test to check header returns as expected. NIFI-5258 - Fixed * import. --- .../nifi-web/nifi-jetty/pom.xml | 6 +++ .../apache/nifi/web/server/JettyServer.java | 33 +++++++++------ .../nifi/web/server/JettyServerTest.java | 41 +++++++++++++++++-- 3 files changed, 65 insertions(+), 15 deletions(-) diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/pom.xml b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/pom.xml index 3d1c6ba16ead..b9a46b6ee803 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/pom.xml +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/pom.xml @@ -194,6 +194,12 @@ 1.16.0 test + + org.springframework + spring-test + 5.0.6.RELEASE + test + diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java index 83b489f145bc..aa0caec9df37 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java @@ -85,9 +85,11 @@ import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.server.handler.HandlerCollection; import org.eclipse.jetty.server.handler.HandlerList; -import org.eclipse.jetty.server.handler.ResourceHandler; import org.eclipse.jetty.server.handler.gzip.GzipHandler; +import org.eclipse.jetty.servlet.DefaultServlet; import org.eclipse.jetty.servlet.FilterHolder; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceCollection; import org.eclipse.jetty.util.ssl.SslContextFactory; @@ -516,13 +518,10 @@ private WebAppContext loadWar(final File warFile, final String contextPath, fina private ContextHandler createDocsWebApp(final String contextPath) { try { - final ResourceHandler resourceHandler = new ResourceHandler(); - resourceHandler.setDirectoriesListed(false); - final File docsDir = getDocsDir("docs"); final Resource docsResource = Resource.newResource(docsDir); - // load the component documentation working directory + // Load the component documentation working directory final File componentDocsDirPath = props.getComponentDocumentationWorkingDirectory(); final File workingDocsDirectory = getWorkingDocsDirectory(componentDocsDirPath); final Resource workingDocsResource = Resource.newResource(workingDocsDirectory); @@ -530,16 +529,26 @@ private ContextHandler createDocsWebApp(final String contextPath) { final File webApiDocsDir = getWebApiDocsDir(); final Resource webApiDocsResource = Resource.newResource(webApiDocsDir); - // create resources for both docs locations + // Create resources for all docs locations final ResourceCollection resources = new ResourceCollection(docsResource, workingDocsResource, webApiDocsResource); - resourceHandler.setBaseResource(resources); - // create the context handler - final ContextHandler handler = new ContextHandler(contextPath); - handler.setHandler(resourceHandler); + + // The below ServletContext and Servlet API usage was derived from https://stackoverflow.com/a/34277268. + // Thanks go to Stack Overflow user Joakim Erdfelt. + + // We create a servlet context handler to encapsulate the static docs resources we want served. + ServletContextHandler docsHandler = new ServletContextHandler(); + docsHandler.setContextPath(contextPath); + docsHandler.setBaseResource(resources); + docsHandler.addFilter(new FilterHolder(FRAME_OPTIONS_FILTER), "/*", EnumSet.allOf(DispatcherType.class)); + + // This default servlet actually serves the static docs resources. + ServletHolder defaultServlet = new ServletHolder("default", DefaultServlet.class); + defaultServlet.setInitParameter("dirAllowed", "false"); //should this be set to false? org.eclipse.jetty.servlet.Default.dirAllowed = false + docsHandler.addServlet(defaultServlet, "/"); logger.info("Loading documents web app with context path set to " + contextPath); - return handler; + return docsHandler; } catch (Exception ex) { logger.error("Unhandled Exception in createDocsWebApp: " + ex.getMessage()); startUpFailure(ex); @@ -1021,7 +1030,7 @@ public void doFilter(final ServletRequest req, final ServletResponse resp, final // set frame options accordingly final HttpServletResponse response = (HttpServletResponse) resp; - response.addHeader(FRAME_OPTIONS, SAME_ORIGIN); + response.setHeader(FRAME_OPTIONS, SAME_ORIGIN); filterChain.doFilter(req, resp); } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/java/org/apache/nifi/web/server/JettyServerTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/java/org/apache/nifi/web/server/JettyServerTest.java index 29b43e02cc61..09ebee84a03a 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/java/org/apache/nifi/web/server/JettyServerTest.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/test/java/org/apache/nifi/web/server/JettyServerTest.java @@ -17,20 +17,30 @@ package org.apache.nifi.web.server; +import java.io.IOException; +import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.util.HashMap; import java.util.Map; - import org.apache.nifi.security.util.KeystoreType; import org.bouncycastle.jce.provider.BouncyCastleProvider; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.apache.nifi.util.NiFiProperties; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletContext; +import javax.servlet.ServletException; +import javax.servlet.Filter; +import javax.servlet.http.HttpServletRequest; import org.junit.Test; - +import org.mockito.Mockito; +import org.springframework.mock.web.MockHttpServletResponse; +import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.when; public class JettyServerTest { @Test @@ -142,4 +152,29 @@ public void testConfigureSslContextFactoryWithPkcsTrustStore() { verify(contextFactory).setTrustStoreType(trustStoreType); verify(contextFactory).setTrustStoreProvider(BouncyCastleProvider.PROVIDER_NAME); } + + @Test + public void testNoDuplicateXFrameOptions() throws NoSuchFieldException, IllegalAccessException, ServletException, IOException { + Field xOptionsFilter = JettyServer.class.getDeclaredField("FRAME_OPTIONS_FILTER"); + xOptionsFilter.setAccessible(true); + Filter filter = (Filter) xOptionsFilter.get(xOptionsFilter); + + HttpServletRequest mockRequest = Mockito.mock(HttpServletRequest.class); + Mockito.when(mockRequest.getRequestURI()).thenReturn("/"); + + MockHttpServletResponse mockResponse = new MockHttpServletResponse(); + FilterChain mockFilterChain = Mockito.mock(FilterChain.class); + ServletContext mockContext = Mockito.mock(ServletContext.class); + FilterConfig mockFilterConfig = Mockito.mock(FilterConfig.class); + + when(mockFilterConfig.getServletContext()).thenReturn(mockContext); + + filter.init(mockFilterConfig); + + // Call doFilter twice, then check the header only appears once. + filter.doFilter(mockRequest, mockResponse, mockFilterChain); + filter.doFilter(mockRequest, mockResponse, mockFilterChain); + + assertEquals(1, mockResponse.getHeaders("X-Frame-Options").size()); + } } From ff4db991963d2cc468dc92ec4bdb5c8c2089a0b1 Mon Sep 17 00:00:00 2001 From: thenatog Date: Wed, 6 Jun 2018 10:06:59 -0400 Subject: [PATCH 2/2] Removed SO username --- .../src/main/java/org/apache/nifi/web/server/JettyServer.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java index aa0caec9df37..fa8841f12a31 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java @@ -534,7 +534,6 @@ private ContextHandler createDocsWebApp(final String contextPath) { // The below ServletContext and Servlet API usage was derived from https://stackoverflow.com/a/34277268. - // Thanks go to Stack Overflow user Joakim Erdfelt. // We create a servlet context handler to encapsulate the static docs resources we want served. ServletContextHandler docsHandler = new ServletContextHandler(); @@ -1048,4 +1047,4 @@ public void destroy() { @FunctionalInterface interface ServerConnectorCreator { ServerConnector create(Server server, HttpConfiguration httpConfiguration); -} \ No newline at end of file +}