From 7618eae91534178a375e021a65c141da19833c9d Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 6 Sep 2019 09:26:06 -0500 Subject: [PATCH] Issue #4064 - MinimalServlets test and ServletHolder fix + Also made ContextHandler warning message about features that are unimplemented (and you should use ServletContextHandler) more clear. (this helped with diagnosing where the bug was in ServletHolder) Signed-off-by: Joakim Erdfelt --- .../jetty/embedded/MinimalServlets.java | 25 +++++--- .../jetty/embedded/MinimalServletsTest.java | 64 +++++++++++++++++++ .../jetty/server/handler/ContextHandler.java | 46 ++++++------- .../eclipse/jetty/servlet/ServletHolder.java | 7 +- 4 files changed, 106 insertions(+), 36 deletions(-) create mode 100644 examples/embedded/src/test/java/org/eclipse/jetty/embedded/MinimalServletsTest.java diff --git a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/MinimalServlets.java b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/MinimalServlets.java index 5ce9a24016c..dab7237a89b 100644 --- a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/MinimalServlets.java +++ b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/MinimalServlets.java @@ -19,7 +19,6 @@ package org.eclipse.jetty.embedded; import java.io.IOException; -import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -29,13 +28,13 @@ import org.eclipse.jetty.servlet.ServletHandler; public class MinimalServlets { - public static void main(String[] args) throws Exception + + public static Server createServer(int port) { - // Create a basic jetty server object that will listen on port 8080. // Note that if you set this to port 0 then a randomly available port // will be assigned that you can either look in the logs for the port, // or programmatically obtain it for use in test cases. - Server server = new Server(8080); + Server server = new Server(port); // The ServletHandler is a dead simple way to create a context handler // that is backed by an instance of a Servlet. @@ -51,13 +50,19 @@ public class MinimalServlets // through a web.xml @WebServlet annotation, or anything similar. handler.addServletWithMapping(HelloServlet.class, "/*"); + return server; + } + + public static void main(String[] args) throws Exception + { + // Create a basic jetty server object that will listen on port 8080. + Server server = createServer(8080); + // Start things up! server.start(); // The use of server.join() the will make the current thread join and - // wait until the server is done executing. - // See - // http://docs.oracle.com/javase/7/docs/api/java/lang/Thread.html#join() + // wait until the server thread is done executing. server.join(); } @@ -66,11 +71,11 @@ public class MinimalServlets { @Override protected void doGet(HttpServletRequest request, - HttpServletResponse response) throws ServletException, - IOException + HttpServletResponse response) throws IOException { - response.setContentType("text/html"); response.setStatus(HttpServletResponse.SC_OK); + response.setContentType("text/html"); + response.setCharacterEncoding("utf-8"); response.getWriter().println("

Hello from HelloServlet

"); } } diff --git a/examples/embedded/src/test/java/org/eclipse/jetty/embedded/MinimalServletsTest.java b/examples/embedded/src/test/java/org/eclipse/jetty/embedded/MinimalServletsTest.java new file mode 100644 index 00000000000..c1abf3da16d --- /dev/null +++ b/examples/embedded/src/test/java/org/eclipse/jetty/embedded/MinimalServletsTest.java @@ -0,0 +1,64 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.embedded; + +import java.io.IOException; +import java.net.HttpURLConnection; +import java.net.URI; + +import org.eclipse.jetty.server.Server; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; + +public class MinimalServletsTest +{ + private Server server; + + @BeforeEach + public void startServer() throws Exception + { + server = MinimalServlets.createServer(0); + server.start(); + } + + @AfterEach + public void stopServer() throws Exception + { + server.stop(); + } + + @Test + public void testGetHello() throws IOException + { + URI uri = server.getURI().resolve("/hello"); + HttpURLConnection http = (HttpURLConnection)uri.toURL().openConnection(); + assertThat("HTTP Response Status", http.getResponseCode(), is(HttpURLConnection.HTTP_OK)); + + // HttpUtil.dumpResponseHeaders(http); + + // test response content + String responseBody = HttpUtil.getResponseBody(http); + assertThat("Response Content", responseBody, containsString("Hello")); + } +} diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index 4297a04a031..6d08d5c7d11 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -126,7 +126,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu public static final int DEFAULT_LISTENER_TYPE_INDEX = 1; public static final int EXTENDED_LISTENER_TYPE_INDEX = 0; - private static final String __unimplmented = "Unimplemented - use org.eclipse.jetty.servlet.ServletContextHandler"; + private static final String UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER = "Unimplemented {} - use org.eclipse.jetty.servlet.ServletContextHandler"; private static final Logger LOG = Log.getLogger(ContextHandler.class); @@ -2479,7 +2479,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu @Override public JspConfigDescriptor getJspConfigDescriptor() { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "getJspConfigDescriptor()"); return null; } @@ -2673,130 +2673,130 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu @Override public Dynamic addFilter(String filterName, Class filterClass) { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "addFilter(String, Class)"); return null; } @Override public Dynamic addFilter(String filterName, Filter filter) { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "addFilter(String, Filter)"); return null; } @Override public Dynamic addFilter(String filterName, String className) { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "addFilter(String, String)"); return null; } @Override public javax.servlet.ServletRegistration.Dynamic addServlet(String servletName, Class servletClass) { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "addServlet(String, Class)"); return null; } @Override public javax.servlet.ServletRegistration.Dynamic addServlet(String servletName, Servlet servlet) { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "addServlet(String, Servlet)"); return null; } @Override public javax.servlet.ServletRegistration.Dynamic addServlet(String servletName, String className) { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "addServlet(String, String)"); return null; } @Override public T createFilter(Class c) throws ServletException { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "createFilter(Class)"); return null; } @Override public T createServlet(Class c) throws ServletException { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "createServlet(Class)"); return null; } @Override public Set getDefaultSessionTrackingModes() { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "getDefaultSessionTrackingModes()"); return null; } @Override public Set getEffectiveSessionTrackingModes() { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "getEffectiveSessionTrackingModes()"); return null; } @Override public FilterRegistration getFilterRegistration(String filterName) { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "getFilterRegistration(String)"); return null; } @Override public Map getFilterRegistrations() { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "getFilterRegistrations()"); return null; } @Override public ServletRegistration getServletRegistration(String servletName) { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "getServletRegistration(String)"); return null; } @Override public Map getServletRegistrations() { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "getServletRegistrations()"); return null; } @Override public SessionCookieConfig getSessionCookieConfig() { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "getSessionCookieConfig()"); return null; } @Override public void setSessionTrackingModes(Set sessionTrackingModes) { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "setSessionTrackingModes(Set)"); } @Override public void addListener(String className) { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "addListener(String)"); } @Override public void addListener(T t) { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "addListener(T)"); } @Override public void addListener(Class listenerClass) { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "addListener(Class)"); } @Override @@ -2843,14 +2843,14 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu @Override public JspConfigDescriptor getJspConfigDescriptor() { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "getJspConfigDescriptor()"); return null; } @Override public void declareRoles(String... roleNames) { - LOG.warn(__unimplmented); + LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "declareRoles(String...)"); } @Override diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java index 9fb2b7e4ff6..599dd3be680 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java @@ -1265,9 +1265,10 @@ public class ServletHolder extends Holder implements UserIdentity.Scope try { ServletContext ctx = getServletHandler().getServletContext(); - if (ctx == null) - return getHeldClass().getDeclaredConstructor().newInstance(); - return ctx.createServlet(getHeldClass()); + if (ctx instanceof ServletContextHandler.Context) + return ctx.createServlet(getHeldClass()); + return getHeldClass().getDeclaredConstructor().newInstance(); + } catch (ServletException ex) {