From 7e20e1c49c5d0eb1e13fbf8161ec934c3915c498 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Sun, 23 Apr 2017 07:07:27 -0700 Subject: [PATCH 1/3] Issue #698 - Upgrade to Jetty 9.4.4 --- pom.xml | 2 +- ...WebSocketServletContextHandlerFactory.java | 10 ++- .../java/spark/InitExceptionHandlerTest.java | 8 +- ...ocketServletContextHandlerFactoryTest.java | 82 ++++++++++++------- .../spark/staticfiles/StaticFilesTest.java | 18 ++-- 5 files changed, 76 insertions(+), 44 deletions(-) diff --git a/pom.xml b/pom.xml index 874fdbb2db..b6e97443dd 100644 --- a/pom.xml +++ b/pom.xml @@ -30,7 +30,7 @@ 1.8 - 9.3.6.v20151106 + 9.4.4.v20170414 UTF-8 1.6.4 1.10.19 diff --git a/src/main/java/spark/embeddedserver/jetty/websocket/WebSocketServletContextHandlerFactory.java b/src/main/java/spark/embeddedserver/jetty/websocket/WebSocketServletContextHandlerFactory.java index d2be97f13f..3d56659309 100644 --- a/src/main/java/spark/embeddedserver/jetty/websocket/WebSocketServletContextHandlerFactory.java +++ b/src/main/java/spark/embeddedserver/jetty/websocket/WebSocketServletContextHandlerFactory.java @@ -19,9 +19,10 @@ import java.util.Map; import java.util.Optional; +import org.eclipse.jetty.http.pathmap.ServletPathSpec; import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.websocket.server.NativeWebSocketConfiguration; import org.eclipse.jetty.websocket.server.WebSocketUpgradeFilter; -import org.eclipse.jetty.websocket.server.pathmap.ServletPathSpec; import org.eclipse.jetty.websocket.servlet.WebSocketCreator; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -50,9 +51,14 @@ public static ServletContextHandler create(Map if (webSocketIdleTimeoutMillis.isPresent()) { webSocketUpgradeFilter.getFactory().getPolicy().setIdleTimeout(webSocketIdleTimeoutMillis.get()); } + // Since we are configuring WebSockets before the ServletContextHandler and WebSocketUpgradeFilter is + // even initialized / started, then we have to pre-populate the configuration that will eventually + // be used by Jetty's WebSocketUpgradeFilter. + NativeWebSocketConfiguration webSocketConfiguration = (NativeWebSocketConfiguration) webSocketServletContextHandler + .getServletContext().getAttribute(NativeWebSocketConfiguration.class.getName()); for (String path : webSocketHandlers.keySet()) { WebSocketCreator webSocketCreator = WebSocketCreatorFactory.create(webSocketHandlers.get(path)); - webSocketUpgradeFilter.addMapping(new ServletPathSpec(path), webSocketCreator); + webSocketConfiguration.addMapping(new ServletPathSpec(path), webSocketCreator); } } catch (Exception ex) { logger.error("creation of websocket context handler failed.", ex); diff --git a/src/test/java/spark/InitExceptionHandlerTest.java b/src/test/java/spark/InitExceptionHandlerTest.java index d579342353..09457d972a 100644 --- a/src/test/java/spark/InitExceptionHandlerTest.java +++ b/src/test/java/spark/InitExceptionHandlerTest.java @@ -1,12 +1,12 @@ package spark; +import static spark.Service.ignite; + import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; -import static spark.Service.ignite; - public class InitExceptionHandlerTest { private static Service service1; @@ -16,18 +16,16 @@ public class InitExceptionHandlerTest { @BeforeClass public static void setUpClass() throws Exception { - service1 = ignite(); service1.port(1122); service1.init(); service1.awaitInitialization(); service2 = ignite(); - service2.port(1122); + service2.port(70000); service2.initExceptionHandler((e) -> errorMessage = "Custom init error"); service2.init(); service2.awaitInitialization(); - } @Test diff --git a/src/test/java/spark/embeddedserver/jetty/websocket/WebSocketServletContextHandlerFactoryTest.java b/src/test/java/spark/embeddedserver/jetty/websocket/WebSocketServletContextHandlerFactoryTest.java index 1aa7e85080..b61f277b81 100644 --- a/src/test/java/spark/embeddedserver/jetty/websocket/WebSocketServletContextHandlerFactoryTest.java +++ b/src/test/java/spark/embeddedserver/jetty/websocket/WebSocketServletContextHandlerFactoryTest.java @@ -1,10 +1,21 @@ package spark.embeddedserver.jetty.websocket; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; + +import javax.servlet.ServletContext; + +import org.eclipse.jetty.http.pathmap.MappedResource; +import org.eclipse.jetty.http.pathmap.PathSpec; import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.websocket.server.NativeWebSocketConfiguration; import org.eclipse.jetty.websocket.server.WebSocketServerFactory; import org.eclipse.jetty.websocket.server.WebSocketUpgradeFilter; -import org.eclipse.jetty.websocket.server.pathmap.PathMappings; -import org.eclipse.jetty.websocket.server.pathmap.PathSpec; import org.eclipse.jetty.websocket.servlet.WebSocketCreator; import org.junit.Test; import org.junit.runner.RunWith; @@ -12,12 +23,6 @@ import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; -import java.util.HashMap; -import java.util.Map; -import java.util.Optional; - -import static org.junit.Assert.*; - @RunWith(PowerMockRunner.class) public class WebSocketServletContextHandlerFactoryTest { @@ -41,22 +46,30 @@ public void testCreate_whenNoIdleTimeoutIsPresent() throws Exception { webSocketHandlers.put(webSocketPath, new WebSocketHandlerClassWrapper(WebSocketTestHandler.class)); servletContextHandler = WebSocketServletContextHandlerFactory.create(webSocketHandlers, Optional.empty()); - + + ServletContext servletContext = servletContextHandler.getServletContext(); + WebSocketUpgradeFilter webSocketUpgradeFilter = - (WebSocketUpgradeFilter) servletContextHandler.getAttribute("org.eclipse.jetty.websocket.server.WebSocketUpgradeFilter"); + (WebSocketUpgradeFilter) servletContext.getAttribute("org.eclipse.jetty.websocket.server.WebSocketUpgradeFilter"); assertNotNull("Should return a WebSocketUpgradeFilter because we configured it to have one", webSocketUpgradeFilter); + + NativeWebSocketConfiguration webSocketConfiguration = + (NativeWebSocketConfiguration) servletContext.getAttribute(NativeWebSocketConfiguration.class.getName()); + + MappedResource mappedResource = webSocketConfiguration.getMatch("/websocket"); + PathSpec pathSpec = mappedResource.getPathSpec(); - PathMappings.MappedResource mappedResource = webSocketUpgradeFilter.getMappings().getMatch("/websocket"); - WebSocketCreatorFactory.SparkWebSocketCreator sc = (WebSocketCreatorFactory.SparkWebSocketCreator) mappedResource.getResource(); - PathSpec pathSpec = (PathSpec) mappedResource.getPathSpec(); - - assertEquals("Should return the WebSocket path specified when contexst handler was created", - webSocketPath, pathSpec.getPathSpec()); - - assertTrue("Should return true because handler should be an instance of the one we passed when it was created", - sc.getHandler() instanceof WebSocketTestHandler); - + assertEquals("Should return the WebSocket path specified when context handler was created", + webSocketPath, pathSpec.getDeclaration()); + + // Because spark works on a non-initialized / non-started ServletContextHandler and WebSocketUpgradeFilter + // the stored WebSocketCreator is wrapped for persistence through the start/stop of those contexts. + // You cannot unwrap or cast to that WebSocketTestHandler this way. + // Only websockets that are added during a live context can be cast this way. + // WebSocketCreator sc = mappedResource.getResource(); + // assertTrue("Should return true because handler should be an instance of the one we passed when it was created", + // sc.getHandler() instanceof WebSocketTestHandler); } @Test @@ -69,25 +82,34 @@ public void testCreate_whenTimeoutIsPresent() throws Exception { webSocketHandlers.put(webSocketPath, new WebSocketHandlerClassWrapper(WebSocketTestHandler.class)); servletContextHandler = WebSocketServletContextHandlerFactory.create(webSocketHandlers, Optional.of(timeout)); + + ServletContext servletContext = servletContextHandler.getServletContext(); WebSocketUpgradeFilter webSocketUpgradeFilter = - (WebSocketUpgradeFilter) servletContextHandler.getAttribute("org.eclipse.jetty.websocket.server.WebSocketUpgradeFilter"); + (WebSocketUpgradeFilter) servletContext.getAttribute("org.eclipse.jetty.websocket.server.WebSocketUpgradeFilter"); assertNotNull("Should return a WebSocketUpgradeFilter because we configured it to have one", webSocketUpgradeFilter); + + NativeWebSocketConfiguration webSocketConfiguration = + (NativeWebSocketConfiguration) servletContext.getAttribute(NativeWebSocketConfiguration.class.getName()); - WebSocketServerFactory webSocketServerFactory = webSocketUpgradeFilter.getFactory(); + WebSocketServerFactory webSocketServerFactory = webSocketConfiguration.getFactory(); assertEquals("Timeout value should be the same as the timeout specified when context handler was created", timeout.longValue(), webSocketServerFactory.getPolicy().getIdleTimeout()); - PathMappings.MappedResource mappedResource = webSocketUpgradeFilter.getMappings().getMatch("/websocket"); - WebSocketCreatorFactory.SparkWebSocketCreator sc = (WebSocketCreatorFactory.SparkWebSocketCreator) mappedResource.getResource(); - PathSpec pathSpec = (PathSpec) mappedResource.getPathSpec(); + MappedResource mappedResource = webSocketConfiguration.getMatch("/websocket"); + PathSpec pathSpec = mappedResource.getPathSpec(); assertEquals("Should return the WebSocket path specified when context handler was created", - webSocketPath, pathSpec.getPathSpec()); - - assertTrue("Should return true because handler should be an instance of the one we passed when it was created", - sc.getHandler() instanceof WebSocketTestHandler); + webSocketPath, pathSpec.getDeclaration()); + + // Because spark works on a non-initialized / non-started ServletContextHandler and WebSocketUpgradeFilter + // the stored WebSocketCreator is wrapped for persistence through the start/stop of those contexts. + // You cannot unwrap or cast to that WebSocketTestHandler this way. + // Only websockets that are added during a live context can be cast this way. + // WebSocketCreator sc = mappedResource.getResource(); + // assertTrue("Should return true because handler should be an instance of the one we passed when it was created", + // sc.getHandler() instanceof WebSocketTestHandler); } @Test @@ -105,4 +127,4 @@ public void testCreate_whenWebSocketContextHandlerCreationFails_thenThrowExcepti assertNull("Should return null because Websocket context handler was not created", servletContextHandler); } -} \ No newline at end of file +} diff --git a/src/test/java/spark/staticfiles/StaticFilesTest.java b/src/test/java/spark/staticfiles/StaticFilesTest.java index 9f734e8e36..6c140ef7ee 100644 --- a/src/test/java/spark/staticfiles/StaticFilesTest.java +++ b/src/test/java/spark/staticfiles/StaticFilesTest.java @@ -16,11 +16,16 @@ */ package spark.staticfiles; +import static spark.Spark.exception; +import static spark.Spark.get; +import static spark.Spark.staticFiles; + import java.io.File; import java.io.FileWriter; import java.io.IOException; import java.net.URLEncoder; +import org.hamcrest.CoreMatchers; import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; @@ -32,10 +37,6 @@ import spark.examples.exception.NotFoundException; import spark.util.SparkTestUtil; -import static spark.Spark.exception; -import static spark.Spark.get; -import static spark.Spark.staticFiles; - /** * Test static files */ @@ -142,8 +143,13 @@ public void testDirectoryTraversalProtectionLocal() throws Exception { String path = "/" + URLEncoder.encode("..\\spark\\", "UTF-8") + "Spark.class"; SparkTestUtil.UrlResponse response = doGet(path); - Assert.assertEquals(404, response.status); - Assert.assertEquals(NOT_FOUND_BRO, response.body); + // Attempt to access context above root is either a 400 or 404 depending on environment + Assert.assertThat( response.status, CoreMatchers.anyOf( + CoreMatchers.is(400), + CoreMatchers.is(404))); + Assert.assertThat( response.body, CoreMatchers.anyOf( + CoreMatchers.containsString("Bad Message 400"), + CoreMatchers.containsString(NOT_FOUND_BRO))); testGet(); } From eddd72ae041f57f8766b492ff0cb264d1de8c481 Mon Sep 17 00:00:00 2001 From: David Date: Sun, 23 Apr 2017 21:48:20 +0200 Subject: [PATCH 2/3] Fix imports --- src/test/java/spark/InitExceptionHandlerTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/spark/InitExceptionHandlerTest.java b/src/test/java/spark/InitExceptionHandlerTest.java index 2d0357d821..0623b44f21 100644 --- a/src/test/java/spark/InitExceptionHandlerTest.java +++ b/src/test/java/spark/InitExceptionHandlerTest.java @@ -1,12 +1,12 @@ package spark; -import static spark.Service.ignite; - import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; +import static spark.Service.ignite; + public class InitExceptionHandlerTest { private static int NON_VALID_PORT = Integer.MAX_VALUE; From 4ceff395f784798e1f15c033b57883446d02f2f2 Mon Sep 17 00:00:00 2001 From: David Date: Sun, 23 Apr 2017 21:49:08 +0200 Subject: [PATCH 3/3] Fix imports --- src/test/java/spark/staticfiles/StaticFilesTest.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/test/java/spark/staticfiles/StaticFilesTest.java b/src/test/java/spark/staticfiles/StaticFilesTest.java index 686c33937b..d338d2f348 100644 --- a/src/test/java/spark/staticfiles/StaticFilesTest.java +++ b/src/test/java/spark/staticfiles/StaticFilesTest.java @@ -16,16 +16,11 @@ */ package spark.staticfiles; -import static spark.Spark.exception; -import static spark.Spark.get; -import static spark.Spark.staticFiles; - import java.io.File; import java.io.FileWriter; import java.io.IOException; import java.net.URLEncoder; -import org.hamcrest.CoreMatchers; import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; @@ -37,6 +32,10 @@ import spark.examples.exception.NotFoundException; import spark.util.SparkTestUtil; +import static spark.Spark.exception; +import static spark.Spark.get; +import static spark.Spark.staticFiles; + /** * Test static files */ @@ -142,7 +141,9 @@ public void testStaticFilePageHtml() throws Exception { public void testDirectoryTraversalProtectionLocal() throws Exception { String path = "/" + URLEncoder.encode("..\\spark\\", "UTF-8") + "Spark.class"; SparkTestUtil.UrlResponse response = doGet(path); + Assert.assertEquals(400, response.status); + testGet(); }