Merge remote-tracking branch 'origin/jetty-9.4.x'

apache-jsp/src/test/java/org/eclipse/jetty/jsp/TestJspFileNameToClass.java

@@ -0,0 +1,58 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2016 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.jsp;
+
+import org.eclipse.jetty.servlet.ServletHolder;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestJspFileNameToClass
+{
+
+    @Test
+    public void testJspFileNameToClassName() throws Exception
+    {
+        ServletHolder h = new ServletHolder();
+        h.setName("test");
+
+
+        Assert.assertEquals(null,  h.getClassNameForJsp(null));
+
+        Assert.assertEquals(null,  h.getClassNameForJsp(""));
+
+        Assert.assertEquals(null,  h.getClassNameForJsp("/blah/"));
+
+        Assert.assertEquals(null,  h.getClassNameForJsp("//blah///"));
+
+        Assert.assertEquals(null,  h.getClassNameForJsp("/a/b/c/blah/"));
+
+        Assert.assertEquals("org.apache.jsp.a.b.c.blah",  h.getClassNameForJsp("/a/b/c/blah"));
+
+        Assert.assertEquals("org.apache.jsp.blah_jsp", h.getClassNameForJsp("/blah.jsp"));
+
+        Assert.assertEquals("org.apache.jsp.blah_jsp", h.getClassNameForJsp("//blah.jsp"));
+
+        Assert.assertEquals("org.apache.jsp.blah_jsp", h.getClassNameForJsp("blah.jsp"));
+
+        Assert.assertEquals("org.apache.jsp.a.b.c.blah_jsp", h.getClassNameForJsp("/a/b/c/blah.jsp"));
+
+        Assert.assertEquals("org.apache.jsp.a.b.c.blah_jsp", h.getClassNameForJsp("a/b/c/blah.jsp"));
+    }
+    
+}

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

@@ -83,7 +83,7 @@ public abstract class BaseHolder<T> extends AbstractLifeCycle implements Dumpabl
     {
         //if no class already loaded and no classname, make permanently unavailable
         if (_class==null && (_className==null || _className.equals("")))
-            throw new UnavailableException("No class in holder");
+            throw new UnavailableException("No class in holder "+toString());
         
         //try to load class
         if (_class==null)
@@ -97,7 +97,7 @@ public abstract class BaseHolder<T> extends AbstractLifeCycle implements Dumpabl
             catch (Exception e)
             {
                 LOG.warn(e);
-                throw new UnavailableException(e.getMessage());
+                throw new UnavailableException("Class loading error for holder "+toString());
             }
         }
     }

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

@@ -335,7 +335,7 @@ public class ServletHolder extends Holder<Servlet> implements UserIdentity.Scope
                     if (jsp!=null)
                     {
                         if (LOG.isDebugEnabled())
-                            LOG.debug("JSP file {} for {} mapped to Servlet class {}",_forcedPath, getName(),jsp.getClassName());
+                            LOG.debug("JSP file {} for {} mapped to JspServlet class {}",_forcedPath, getName(),jsp.getClassName());
                         setClassName(jsp.getClassName());
                         //copy jsp init params that don't exist for this servlet
                         for (Map.Entry<String, String> entry:jsp.getInitParameters().entrySet())
@@ -351,6 +351,8 @@ public class ServletHolder extends Holder<Servlet> implements UserIdentity.Scope
                     }
                 }
             }
+            else
+                LOG.warn("Bad jsp-file {} conversion to classname in holder {}", _forcedPath, getName());
         }
 
 
@@ -910,7 +912,7 @@ public class ServletHolder extends Holder<Servlet> implements UserIdentity.Scope
      * @param jsp the jsp-file
      * @return the simple classname of the jsp
      */
-    protected String getNameOfJspClass (String jsp)
+    public String getNameOfJspClass (String jsp)
     {
         if (StringUtil.isBlank(jsp))
             return ""; //empty
@@ -933,16 +935,18 @@ public class ServletHolder extends Holder<Servlet> implements UserIdentity.Scope
         catch (Exception e)
         {
             String tmp = jsp.replace('.','_');
-            LOG.warn("Unable to make identifier for jsp "+jsp +" trying "+tmp+" instead");
             if (LOG.isDebugEnabled())
+            {
+                LOG.warn("JspUtil.makeJavaIdentifier failed for jsp "+jsp +" using "+tmp+" instead");
                 LOG.warn(e);
+            }
             return tmp;
         }
     }
 
 
     /* ------------------------------------------------------------ */
-    protected String getPackageOfJspClass (String jsp)
+    public String getPackageOfJspClass (String jsp)
     {
         if (jsp == null)
             return "";
@@ -959,11 +963,23 @@ public class ServletHolder extends Holder<Servlet> implements UserIdentity.Scope
         }
         catch (Exception e)
         {
-            String tmp = jsp.substring(1,i).replace('/','.').trim();
+            String tmp = jsp;
+            
+            //remove any leading slash
+            int s = 0;
+            if ('/' == (tmp.charAt(0)))
+                s = 1;
+            
+            //remove the element after last slash, which should be name of jsp
+            tmp = tmp.substring(s,i);
+
+            tmp = tmp.replace('/','.').trim();
             tmp = (".".equals(tmp)? "": tmp);
-            LOG.warn("Unable to make package for jsp "+jsp +" trying "+tmp+" instead");
             if (LOG.isDebugEnabled())
+            {
+                LOG.warn("JspUtil.makeJavaPackage failed for "+jsp +" using "+tmp+" instead");
                 LOG.warn(e);
+            }
             return tmp;
         }
     }
@@ -973,9 +989,13 @@ public class ServletHolder extends Holder<Servlet> implements UserIdentity.Scope
     /**
      * @return the package for all jsps
      */
-    protected String getJspPackagePrefix ()
+    public String getJspPackagePrefix ()
     {
-        String jspPackageName = (String)getServletHandler().getServletContext().getInitParameter(JSP_GENERATED_PACKAGE_NAME );
+        String jspPackageName = null;
+
+        if (getServletHandler() != null && getServletHandler().getServletContext() != null)
+            jspPackageName = (String)getServletHandler().getServletContext().getInitParameter(JSP_GENERATED_PACKAGE_NAME );
+
         if (jspPackageName == null)
             jspPackageName = "org.apache.jsp";
 
@@ -988,7 +1008,7 @@ public class ServletHolder extends Holder<Servlet> implements UserIdentity.Scope
      * @param jsp the jsp-file from web.xml
      * @return the fully qualified classname
      */
-    protected String getClassNameForJsp (String jsp)
+    public String getClassNameForJsp (String jsp)
     {
         if (jsp == null)
             return null;
@@ -1269,6 +1289,6 @@ public class ServletHolder extends Holder<Servlet> implements UserIdentity.Scope
     @Override
     public String toString()
     {
-        return String.format("%s@%x==%s,%d,%b",_name,hashCode(),_className,_initOrder,_servlet!=null);
+        return String.format("%s@%x==%s,jsp=%s,order=%d,inst=%b",_name,hashCode(),_className,_forcedPath,_initOrder,_servlet!=null);
     }
 }

jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletHolderTest.java

@@ -18,12 +18,14 @@
 
 package org.eclipse.jetty.servlet;
 
+import javax.servlet.UnavailableException;
+
+import org.eclipse.jetty.server.handler.ContextHandler;
+import org.eclipse.jetty.util.MultiException;
+import org.eclipse.jetty.util.log.StacklessLogging;
 import org.junit.Assert;
 import org.junit.Test;
 
-import java.util.Arrays;
-import java.util.Collections;
-
 public class ServletHolderTest {
 
     @Test
@@ -52,4 +54,87 @@ public class ServletHolderTest {
         Assert.assertTrue(two.compareTo(three) < 0);
         Assert.assertTrue(one.compareTo(three) < 0);
     }
+    
+
+    @Test
+    public void testJspFileNameToClassName() throws Exception
+    {
+        ServletHolder h = new ServletHolder();
+        h.setName("test");
+
+
+        Assert.assertEquals(null,  h.getClassNameForJsp(null));
+
+        Assert.assertEquals(null,  h.getClassNameForJsp(""));
+
+        Assert.assertEquals(null,  h.getClassNameForJsp("/blah/"));
+
+        Assert.assertEquals(null,  h.getClassNameForJsp("//blah///"));
+
+        Assert.assertEquals(null,  h.getClassNameForJsp("/a/b/c/blah/"));
+
+        Assert.assertEquals("org.apache.jsp.a.b.c.blah",  h.getClassNameForJsp("/a/b/c/blah"));
+
+        Assert.assertEquals("org.apache.jsp.blah_jsp", h.getClassNameForJsp("/blah.jsp"));
+
+        Assert.assertEquals("org.apache.jsp.blah_jsp", h.getClassNameForJsp("//blah.jsp"));
+
+        Assert.assertEquals("org.apache.jsp.blah_jsp", h.getClassNameForJsp("blah.jsp"));
+
+        Assert.assertEquals("org.apache.jsp.a.b.c.blah_jsp", h.getClassNameForJsp("/a/b/c/blah.jsp"));
+
+        Assert.assertEquals("org.apache.jsp.a.b.c.blah_jsp", h.getClassNameForJsp("a/b/c/blah.jsp"));
+    }
+
+
+    @Test
+    public void testNoClassName() throws Exception
+    {
+        try (StacklessLogging stackless = new StacklessLogging(ServletHandler.class, ContextHandler.class, ServletContextHandler.class))
+        {
+            ServletContextHandler context = new ServletContextHandler(); 
+            ServletHandler handler = context.getServletHandler();
+            ServletHolder holder = new ServletHolder();
+            holder.setName("foo");
+            holder.setForcedPath("/blah/");
+            handler.addServlet(holder);
+            handler.start();
+            Assert.fail("No class in ServletHolder");
+        }
+        catch (UnavailableException e)
+        {
+            Assert.assertTrue(e.getMessage().contains("foo"));
+        }
+        catch (MultiException e)
+        {
+            MultiException m = (MultiException)e;
+            Assert.assertTrue(m.getCause().getMessage().contains("foo"));
+        }
+    }
+    
+    @Test
+    public void testUnloadableClassName() throws Exception
+    {
+        try (StacklessLogging stackless = new StacklessLogging(BaseHolder.class, ServletHandler.class, ContextHandler.class, ServletContextHandler.class))
+        {
+            ServletContextHandler context = new ServletContextHandler(); 
+            ServletHandler handler = context.getServletHandler();
+            ServletHolder holder = new ServletHolder();
+            holder.setName("foo");
+            holder.setClassName("a.b.c.class");
+            handler.addServlet(holder);
+            handler.start();
+            Assert.fail("Unloadable class");
+        }
+        catch (UnavailableException e)
+        {
+            Assert.assertTrue(e.getMessage().contains("foo"));
+        }
+        catch (MultiException e)
+        {
+            MultiException m = (MultiException)e;
+            Assert.assertTrue(m.getCause().getMessage().contains("foo"));
+        }
+    }
+   
 }