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 <joakim.erdfelt@gmail.com>

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.
+        // wait until the server thread is done executing.
-        // See
-        // http://docs.oracle.com/javase/7/docs/api/java/lang/Thread.html#join()
         server.join();
     }
 
@@ -66,11 +71,11 @@ public class MinimalServlets
     {
         @Override
         protected void doGet(HttpServletRequest request,
-                             HttpServletResponse response) throws ServletException,
+                             HttpServletResponse response) throws IOException
-            IOException
         {
-            response.setContentType("text/html");
             response.setStatus(HttpServletResponse.SC_OK);
+            response.setContentType("text/html");
+            response.setCharacterEncoding("utf-8");
             response.getWriter().println("<h1>Hello from HelloServlet</h1>");
         }
     }

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"));
+    }
+}

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<? extends Filter> 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<? extends Servlet> 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 extends Filter> T createFilter(Class<T> c) throws ServletException
         {
-            LOG.warn(__unimplmented);
+            LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "createFilter(Class)");
             return null;
         }
 
         @Override
         public <T extends Servlet> T createServlet(Class<T> c) throws ServletException
         {
-            LOG.warn(__unimplmented);
+            LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "createServlet(Class)");
             return null;
         }
 
         @Override
         public Set<SessionTrackingMode> getDefaultSessionTrackingModes()
         {
-            LOG.warn(__unimplmented);
+            LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "getDefaultSessionTrackingModes()");
             return null;
         }
 
         @Override
         public Set<SessionTrackingMode> 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<String, ? extends FilterRegistration> 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<String, ? extends ServletRegistration> 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<SessionTrackingMode> sessionTrackingModes)
         {
-            LOG.warn(__unimplmented);
+            LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "setSessionTrackingModes(Set<SessionTrackingMode>)");
         }
 
         @Override
         public void addListener(String className)
         {
-            LOG.warn(__unimplmented);
+            LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "addListener(String)");
         }
 
         @Override
         public <T extends EventListener> void addListener(T t)
         {
-            LOG.warn(__unimplmented);
+            LOG.warn(UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER, "addListener(T)");
         }
 
         @Override
         public void addListener(Class<? extends EventListener> 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

jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java

@@ -1265,9 +1265,10 @@ public class ServletHolder extends Holder<Servlet> implements UserIdentity.Scope
         try
         {
             ServletContext ctx = getServletHandler().getServletContext();
-            if (ctx == null)
+            if (ctx instanceof ServletContextHandler.Context)
-                return getHeldClass().getDeclaredConstructor().newInstance();
+                return ctx.createServlet(getHeldClass());
-            return ctx.createServlet(getHeldClass());
+            return getHeldClass().getDeclaredConstructor().newInstance();
+
         }
         catch (ServletException ex)
         {