Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49916
authormarkt <markt@13f79535-47bb-0310-9956-ffa450edef68>
Fri, 1 Oct 2010 17:29:03 +0000 (17:29 +0000)
committermarkt <markt@13f79535-47bb-0310-9956-ffa450edef68>
Fri, 1 Oct 2010 17:29:03 +0000 (17:29 +0000)
Switch to using an initialisation parameter to pass JSP file information from Catalina to Jasper. This simplifies the Catalina code as well as making it easier for Geronimo and others to integrate Jasper.
Patch provided by David Jencks.

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1003589 13f79535-47bb-0310-9956-ffa450edef68

java/org/apache/catalina/Context.java
java/org/apache/catalina/Globals.java
java/org/apache/catalina/Wrapper.java
java/org/apache/catalina/core/ApplicationDispatcher.java
java/org/apache/catalina/core/StandardContext.java
java/org/apache/catalina/core/StandardWrapper.java
java/org/apache/catalina/core/StandardWrapperValve.java
java/org/apache/catalina/deploy/WebXml.java
java/org/apache/catalina/startup/ContextConfig.java
java/org/apache/jasper/servlet/JspServlet.java
webapps/docs/changelog.xml

index 66cd427..d03f718 100644 (file)
@@ -1217,5 +1217,10 @@ public interface Context extends Container {
      */
     public boolean getPaused();
 
+    
+    /**
+     * Is this context using version 2.2 of the Servlet spec?
+     */
+    boolean isServlet22();
 }
 
index f74d00d..d0337ce 100644 (file)
@@ -119,16 +119,6 @@ public final class Globals {
     public static final String ERROR_MESSAGE_ATTR =
         "javax.servlet.error.message";
 
-
-    /**
-     * The request attribute under which we expose the value of the
-     * <code>&lt;jsp-file&gt;</code> value associated with this servlet,
-     * if any.
-     */
-    public static final String JSP_FILE_ATTR =
-        "org.apache.catalina.jsp_file";
-
-
     /**
      * The request attribute under which we store the key size being used for
      * this SSL connection (as an object of type java.lang.Integer).
index 72df8db..b761a4a 100644 (file)
@@ -85,20 +85,6 @@ public interface Wrapper extends Container {
 
 
     /**
-     * Return the context-relative URI of the JSP file for this servlet.
-     */
-    public String getJspFile();
-
-
-    /**
-     * Set the context-relative URI of the JSP file for this servlet.
-     *
-     * @param jspFile JSP file URI
-     */
-    public void setJspFile(String jspFile);
-
-
-    /**
      * Return the load-on-startup order value (negative value means
      * load on first call).
      */
index fceb6d9..c4b896c 100644 (file)
@@ -664,11 +664,6 @@ final class ApplicationDispatcher
         
         // Call the service() method for the allocated servlet instance
         try {
-            String jspFile = wrapper.getJspFile();
-            if (jspFile != null)
-                request.setAttribute(Globals.JSP_FILE_ATTR, jspFile);
-            else
-                request.removeAttribute(Globals.JSP_FILE_ATTR);
             support.fireInstanceEvent(InstanceEvent.BEFORE_DISPATCH_EVENT,
                                       servlet, request, response);
             // for includes/forwards
@@ -676,23 +671,19 @@ final class ApplicationDispatcher
                filterChain.doFilter(request, response);
              }
             // Servlet Service Method is called by the FilterChain
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             support.fireInstanceEvent(InstanceEvent.AFTER_DISPATCH_EVENT,
                                       servlet, request, response);
         } catch (ClientAbortException e) {
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             support.fireInstanceEvent(InstanceEvent.AFTER_DISPATCH_EVENT,
                                       servlet, request, response);
             ioException = e;
         } catch (IOException e) {
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             support.fireInstanceEvent(InstanceEvent.AFTER_DISPATCH_EVENT,
                                       servlet, request, response);
             wrapper.getLogger().error(sm.getString("applicationDispatcher.serviceException",
                              wrapper.getName()), e);
             ioException = e;
         } catch (UnavailableException e) {
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             support.fireInstanceEvent(InstanceEvent.AFTER_DISPATCH_EVENT,
                                       servlet, request, response);
             wrapper.getLogger().error(sm.getString("applicationDispatcher.serviceException",
@@ -700,7 +691,6 @@ final class ApplicationDispatcher
             servletException = e;
             wrapper.unavailable(e);
         } catch (ServletException e) {
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             support.fireInstanceEvent(InstanceEvent.AFTER_DISPATCH_EVENT,
                                       servlet, request, response);
             Throwable rootCause = StandardWrapper.getRootCause(e);
@@ -710,7 +700,6 @@ final class ApplicationDispatcher
             }
             servletException = e;
         } catch (RuntimeException e) {
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             support.fireInstanceEvent(InstanceEvent.AFTER_DISPATCH_EVENT,
                                       servlet, request, response);
             wrapper.getLogger().error(sm.getString("applicationDispatcher.serviceException",
index c2d7134..531c13e 100644 (file)
@@ -2455,19 +2455,6 @@ public class StandardContext extends ContainerBase
             }
         }
 
-        String jspFile = wrapper.getJspFile();
-        if ((jspFile != null) && !jspFile.startsWith("/")) {
-            if (isServlet22()) {
-                if(log.isDebugEnabled())
-                    log.debug(sm.getString("standardContext.wrapper.warning", 
-                                       jspFile));
-                wrapper.setJspFile("/" + jspFile);
-            } else {
-                throw new IllegalArgumentException
-                    (sm.getString("standardContext.wrapper.error", jspFile));
-            }
-        }
-
         super.addChild(child);
 
         if (isJspServlet && oldJspServlet != null) {
@@ -5134,7 +5121,7 @@ public class StandardContext extends ContainerBase
     /**
      * Are we processing a version 2.2 deployment descriptor?
      */
-    protected boolean isServlet22() {
+    public boolean isServlet22() {
 
         if (this.publicId == null)
             return (false);
index 07731a6..e981872 100644 (file)
@@ -155,12 +155,6 @@ public class StandardWrapper extends ContainerBase
 
 
     /**
-     * The context-relative URI of the JSP file for this servlet.
-     */
-    protected String jspFile = null;
-
-
-    /**
      * The load-on-startup order value (negative value means load on
      * first call) for this servlet.
      */
@@ -369,35 +363,6 @@ public class StandardWrapper extends ContainerBase
 
 
     /**
-     * Return the context-relative URI of the JSP file for this servlet.
-     */
-    public String getJspFile() {
-
-        return (this.jspFile);
-
-    }
-
-
-    /**
-     * Set the context-relative URI of the JSP file for this servlet.
-     *
-     * @param jspFile JSP file URI
-     */
-    public void setJspFile(String jspFile) {
-
-        String oldJspFile = this.jspFile;
-        this.jspFile = jspFile;
-        support.firePropertyChange("jspFile", oldJspFile, this.jspFile);
-
-        // Each jsp-file needs to be represented by its own JspServlet and
-        // corresponding JspMonitoring mbean, because it may be initialized
-        // with its own init params
-        isJspServlet = true;
-
-    }
-
-
-    /**
      * Return the load-on-startup order value (negative value means
      * load on first call).
      */
@@ -1046,31 +1011,8 @@ public class StandardWrapper extends ContainerBase
         Servlet servlet;
         try {
             long t1=System.currentTimeMillis();
-            // If this "servlet" is really a JSP file, get the right class.
-            // HOLD YOUR NOSE - this is a kludge that avoids having to do special
-            // case Catalina-specific code in Jasper - it also requires that the
-            // servlet path be replaced by the <jsp-file> element content in
-            // order to be completely effective
-            String actualClass = servletClass;
-            if ((actualClass == null) && (jspFile != null)) {
-                Wrapper jspWrapper = (Wrapper)
-                    ((Context) getParent()).findChild(Constants.JSP_SERVLET_NAME);
-                if (jspWrapper != null) {
-                    actualClass = jspWrapper.getServletClass();
-                    // Merge init parameters
-                    String paramNames[] = jspWrapper.findInitParameters();
-                    for (int i = 0; i < paramNames.length; i++) {
-                        if (parameters.get(paramNames[i]) == null) {
-                            parameters.put
-                                (paramNames[i], 
-                                 jspWrapper.findInitParameter(paramNames[i]));
-                        }
-                    }
-                }
-            }
-
             // Complain if no servlet class has been specified
-            if (actualClass == null) {
+            if (servletClass == null) {
                 unavailable(null);
                 throw new ServletException
                     (sm.getString("standardWrapper.notClass", getName()));
@@ -1078,12 +1020,12 @@ public class StandardWrapper extends ContainerBase
 
             InstanceManager instanceManager = ((StandardContext)getParent()).getInstanceManager();
             try {
-                servlet = (Servlet) instanceManager.newInstance(actualClass);
+                servlet = (Servlet) instanceManager.newInstance(servletClass);
             } catch (ClassCastException e) {
                 unavailable(null);
                 // Restore the context ClassLoader
                 throw new ServletException
-                    (sm.getString("standardWrapper.notServlet", actualClass), e);
+                    (sm.getString("standardWrapper.notServlet", servletClass), e);
             } catch (Throwable e) {
                 ExceptionUtils.handleThrowable(e);
                 unavailable(null);
@@ -1091,12 +1033,12 @@ public class StandardWrapper extends ContainerBase
                 // Added extra log statement for Bugzilla 36630:
                 // http://issues.apache.org/bugzilla/show_bug.cgi?id=36630
                 if(log.isDebugEnabled()) {
-                    log.debug(sm.getString("standardWrapper.instantiate", actualClass), e);
+                    log.debug(sm.getString("standardWrapper.instantiate", servletClass), e);
                 }
 
                 // Restore the context ClassLoader
                 throw new ServletException
-                    (sm.getString("standardWrapper.instantiate", actualClass), e);
+                    (sm.getString("standardWrapper.instantiate", servletClass), e);
             }
 
             if (multipartConfigElement == null) {
@@ -1110,7 +1052,7 @@ public class StandardWrapper extends ContainerBase
 
             // Special handling for ContainerServlet instances
             if ((servlet instanceof ContainerServlet) &&
-                  (isContainerProvidedServlet(actualClass) ||
+                  (isContainerProvidedServlet(servletClass) ||
                     ((Context)getParent()).getPrivileged() )) {
                 ((ContainerServlet) servlet).setWrapper(this);
             }
@@ -1166,25 +1108,6 @@ public class StandardWrapper extends ContainerBase
                 servlet.init(facade);
             }
 
-            // Invoke jspInit on JSP pages
-            if ((loadOnStartup >= 0) && (jspFile != null)) {
-                // Invoking jspInit
-                DummyRequest req = new DummyRequest();
-                req.setServletPath(jspFile);
-                req.setQueryString(Constants.PRECOMPILE + "=true");
-                DummyResponse res = new DummyResponse();
-
-                if( Globals.IS_SECURITY_ENABLED) {
-                    Object[] args = new Object[]{req, res};
-                    SecurityUtil.doAsPrivilege("service",
-                                               servlet,
-                                               classTypeUsedInService,
-                                               args);
-                    args = null;
-                } else {
-                    servlet.service(req, res);
-                }
-            }
             instanceInitialized = true;
 
             instanceSupport.fireInstanceEvent(InstanceEvent.AFTER_INIT_EVENT,
index 5512b3c..63af4f3 100644 (file)
@@ -175,7 +175,6 @@ final class StandardWrapperValve
         try {
             response.sendAcknowledgement();
         } catch (IOException e) {
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             container.getLogger().warn(sm.getString("standardWrapper.acknowledgeException",
                              wrapper.getName()), e);
             throwable = e;
@@ -209,11 +208,6 @@ final class StandardWrapperValve
         // Call the filter chain for this request
         // NOTE: This also calls the servlet's service() method
         try {
-            String jspFile = wrapper.getJspFile();
-            if (jspFile != null)
-                request.setAttribute(Globals.JSP_FILE_ATTR, jspFile);
-            else
-                request.removeAttribute(Globals.JSP_FILE_ATTR);
             if ((servlet != null) && (filterChain != null)) {
                 // Swallow output if needed
                 if (context.getSwallowOutput()) {
@@ -249,20 +243,16 @@ final class StandardWrapperValve
                 }
 
             }
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
         } catch (ClientAbortException e) {
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             throwable = e;
             exception(request, response, e);
         } catch (IOException e) {
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             container.getLogger().error(sm.getString(
                     "standardWrapper.serviceException", wrapper.getName(),
                     context.getName()), e);
             throwable = e;
             exception(request, response, e);
         } catch (UnavailableException e) {
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             container.getLogger().error(sm.getString(
                     "standardWrapper.serviceException", wrapper.getName(),
                     context.getName()), e);
@@ -283,7 +273,6 @@ final class StandardWrapperValve
             // Do not save exception in 'throwable', because we
             // do not want to do exception(request, response, e) processing
         } catch (ServletException e) {
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             Throwable rootCause = StandardWrapper.getRootCause(e);
             if (!(rootCause instanceof ClientAbortException)) {
                 container.getLogger().error(sm.getString(
@@ -295,7 +284,6 @@ final class StandardWrapperValve
             exception(request, response, e);
         } catch (Throwable e) {
             ExceptionUtils.handleThrowable(e);
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             container.getLogger().error(sm.getString(
                     "standardWrapper.serviceException", wrapper.getName(),
                     context.getName()), e);
@@ -419,11 +407,6 @@ final class StandardWrapperValve
         // Call the filter chain for this request
         // NOTE: This also calls the servlet's event() method
         try {
-            String jspFile = wrapper.getJspFile();
-            if (jspFile != null)
-                request.setAttribute(Globals.JSP_FILE_ATTR, jspFile);
-            else
-                request.removeAttribute(Globals.JSP_FILE_ATTR);
             if ((servlet != null) && (filterChain != null)) {
 
                 // Swallow output if needed
@@ -442,27 +425,22 @@ final class StandardWrapperValve
                 }
 
             }
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
         } catch (ClientAbortException e) {
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             throwable = e;
             exception(request, response, e);
         } catch (IOException e) {
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             container.getLogger().error(sm.getString(
                     "standardWrapper.serviceException", wrapper.getName(),
                     context.getName()), e);
             throwable = e;
             exception(request, response, e);
         } catch (UnavailableException e) {
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             container.getLogger().error(sm.getString(
                     "standardWrapper.serviceException", wrapper.getName(),
                     context.getName()), e);
             // Do not save exception in 'throwable', because we
             // do not want to do exception(request, response, e) processing
         } catch (ServletException e) {
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             Throwable rootCause = StandardWrapper.getRootCause(e);
             if (!(rootCause instanceof ClientAbortException)) {
                 container.getLogger().error(sm.getString(
@@ -474,7 +452,6 @@ final class StandardWrapperValve
             exception(request, response, e);
         } catch (Throwable e) {
             ExceptionUtils.handleThrowable(e);
-            request.removeAttribute(Globals.JSP_FILE_ATTR);
             container.getLogger().error(sm.getString(
                     "standardWrapper.serviceException", wrapper.getName(),
                     context.getName()), e);
index f2b29c8..230e9fc 100644 (file)
@@ -1227,12 +1227,9 @@ public class WebXml {
             // Description is ignored
             // Display name is ignored
             // Icons are ignored
-            // Only set this if it is non-null else every servlet will get
-            // marked as the JSP servlet
-            String jspFile = servlet.getJspFile();
-            if (jspFile != null) {
-                wrapper.setJspFile(jspFile);
-            }
+            
+            // jsp-file gets passed to the JSP Servlet as an init-param
+
             if (servlet.getLoadOnStartup() != null) {
                 wrapper.setLoadOnStartup(servlet.getLoadOnStartup().intValue());
             }
index 342a00d..dbf5efe 100644 (file)
@@ -1253,6 +1253,11 @@ public class ContextConfig
                 ok = webXml.merge(orderedFragments);
             }
 
+            // Step 6.5 Convert explicitly mentioned jsps to servlets
+            if (!false) {
+                convertJsps(webXml);
+            }
+
             // Step 7. Apply merged web.xml to Context
             if (ok) {
                 webXml.configureContext(context);
@@ -1302,10 +1307,41 @@ public class ContextConfig
             }
         } else {
             // Apply unmerged web.xml to Context
+            convertJsps(webXml);
             webXml.configureContext(context);
         }
     }
 
+    private void convertJsps(WebXml webXml) {
+        ServletDef jspServlet = webXml.getServlets().get("jsp");
+        for (ServletDef servletDef: webXml.getServlets().values()) {
+            if (servletDef.getJspFile() != null) {
+                convertJsp(servletDef, jspServlet);
+            }
+        }
+    }
+
+    private void convertJsp(ServletDef servletDef, ServletDef jspServletDef) {
+        servletDef.setServletClass(org.apache.catalina.core.Constants.JSP_SERVLET_CLASS);
+        String jspFile = servletDef.getJspFile();
+        servletDef.getParameterMap().put("jspFile", jspFile);
+        if ((jspFile != null) && !jspFile.startsWith("/")) {
+            if (context.isServlet22()) {
+                if(log.isDebugEnabled())
+                    log.debug(sm.getString("standardContext.wrapper.warning",
+                                       jspFile));
+                jspFile = "/" + jspFile;
+            } else {
+                throw new IllegalArgumentException
+                    (sm.getString("standardContext.wrapper.error", jspFile));
+            }
+        }
+        servletDef.setJspFile(null);
+        for (Map.Entry<String, String> initParam: jspServletDef.getParameterMap().entrySet()) {
+            servletDef.addInitParameter(initParam.getKey(), initParam.getValue());
+        }
+    }
+
     protected WebXml createWebXml() {
         return new WebXml();
     }
index 166831d..52e65ec 100644 (file)
@@ -20,6 +20,10 @@ package org.apache.jasper.servlet;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.lang.reflect.Constructor;
+import java.net.MalformedURLException;
+import java.security.AccessController;
+import java.security.PrivilegedActionException;
+import java.security.PrivilegedExceptionAction;
 
 import javax.servlet.ServletConfig;
 import javax.servlet.ServletContext;
@@ -64,6 +68,9 @@ public class JspServlet extends HttpServlet implements PeriodicEventListener {
     private ServletConfig config;
     private Options options;
     private JspRuntimeContext rctxt;
+    //jspFile for a jsp configured explicitly as a servlet, in environments where this configuration is
+    //translated into an init-param for this servlet.
+    private String jspFile;
 
 
     /*
@@ -105,7 +112,36 @@ public class JspServlet extends HttpServlet implements PeriodicEventListener {
             options = new EmbeddedServletOptions(config, context);
         }
         rctxt = new JspRuntimeContext(context, options);
-        
+        if (config.getInitParameter("jspFile") != null) {
+            jspFile = config.getInitParameter("jspFile");
+            try {
+                if (null == context.getResource(jspFile)) {
+                    throw new ServletException("missing jspFile");
+                }
+            } catch (MalformedURLException e) {
+                throw new ServletException("Can not locate jsp file", e);
+            }
+            try {
+                serviceJspFile(null, null, jspFile, null, true);
+                if (SecurityUtil.isPackageProtectionEnabled()){
+                   AccessController.doPrivileged(new PrivilegedExceptionAction<Object>(){
+                        public Object run() throws IOException, ServletException {
+                            serviceJspFile(null, null, jspFile, null, true);
+                            return null;
+                        }
+                    });
+                } else {
+                    serviceJspFile(null, null, jspFile, null, true);
+                }
+            } catch (IOException e) {
+                throw new ServletException("Could not precompile jsp: " + jspFile, e);
+            } catch (PrivilegedActionException e) {
+                Throwable t = e.getCause();
+                if (t instanceof ServletException) throw (ServletException)t;
+                throw new ServletException("Could not precompile jsp: " + jspFile, e);
+            }
+        }
+
         if (log.isDebugEnabled()) {
             log.debug(Localizer.getMessage("jsp.message.scratch.dir.is",
                     options.getScratchDir().toString()));
@@ -214,14 +250,15 @@ public class JspServlet extends HttpServlet implements PeriodicEventListener {
     public void service (HttpServletRequest request, 
                              HttpServletResponse response)
                 throws ServletException, IOException {
+        //jspFile may be configured as an init-param for this servlet instance
+        String jspUri = jspFile;
 
-        String jspUri = null;
-
-        String jspFile = (String) request.getAttribute(Constants.JSP_FILE);
-        if (jspFile != null) {
-            // JSP is specified via <jsp-file> in <servlet> declaration
-            jspUri = jspFile;
-        } else {
+        if (jspUri == null) {
+            // JSP specified via <jsp-file> in <servlet> declaration and supplied through
+            //custom servlet container code
+            jspUri = (String) request.getAttribute(Constants.JSP_FILE);
+        }
+        if (jspUri == null) {
             /*
              * Check to see if the requested JSP has been the target of a
              * RequestDispatcher.include()
index 4d2028c..f712942 100644 (file)
         <code>jsp:attribute</code> elements now supports the use of expressions
         and expression language. (markt)
       </fix>
+      <fix>
+        <bug>49916</bug>: Switch to using an initialisation parameter to pass
+        JSP file information from Catalina to Jasper. This simplifies the
+        Catalina code as well as making it easier for Geronimo and others to
+        integrate Jasper. Patch provided by David Jencks. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Cluster">