From db82e9839735f5bcb0ec00fc1f13e0562e939b8c Mon Sep 17 00:00:00 2001 From: markt Date: Fri, 1 Oct 2010 17:29:03 +0000 Subject: [PATCH] Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49916 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 | 5 ++ java/org/apache/catalina/Globals.java | 10 --- java/org/apache/catalina/Wrapper.java | 14 ---- .../catalina/core/ApplicationDispatcher.java | 11 --- java/org/apache/catalina/core/StandardContext.java | 15 +--- java/org/apache/catalina/core/StandardWrapper.java | 89 ++-------------------- .../apache/catalina/core/StandardWrapperValve.java | 23 ------ java/org/apache/catalina/deploy/WebXml.java | 9 +-- .../org/apache/catalina/startup/ContextConfig.java | 36 +++++++++ java/org/apache/jasper/servlet/JspServlet.java | 53 +++++++++++-- webapps/docs/changelog.xml | 6 ++ 11 files changed, 102 insertions(+), 169 deletions(-) diff --git a/java/org/apache/catalina/Context.java b/java/org/apache/catalina/Context.java index 66cd42768..d03f71865 100644 --- a/java/org/apache/catalina/Context.java +++ b/java/org/apache/catalina/Context.java @@ -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(); } diff --git a/java/org/apache/catalina/Globals.java b/java/org/apache/catalina/Globals.java index f74d00d00..d0337ced8 100644 --- a/java/org/apache/catalina/Globals.java +++ b/java/org/apache/catalina/Globals.java @@ -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 - * <jsp-file> 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). diff --git a/java/org/apache/catalina/Wrapper.java b/java/org/apache/catalina/Wrapper.java index 72df8db5d..b761a4ae3 100644 --- a/java/org/apache/catalina/Wrapper.java +++ b/java/org/apache/catalina/Wrapper.java @@ -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). */ diff --git a/java/org/apache/catalina/core/ApplicationDispatcher.java b/java/org/apache/catalina/core/ApplicationDispatcher.java index fceb6d992..c4b896c1c 100644 --- a/java/org/apache/catalina/core/ApplicationDispatcher.java +++ b/java/org/apache/catalina/core/ApplicationDispatcher.java @@ -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", diff --git a/java/org/apache/catalina/core/StandardContext.java b/java/org/apache/catalina/core/StandardContext.java index c2d713474..531c13e96 100644 --- a/java/org/apache/catalina/core/StandardContext.java +++ b/java/org/apache/catalina/core/StandardContext.java @@ -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); diff --git a/java/org/apache/catalina/core/StandardWrapper.java b/java/org/apache/catalina/core/StandardWrapper.java index 07731a67d..e981872af 100644 --- a/java/org/apache/catalina/core/StandardWrapper.java +++ b/java/org/apache/catalina/core/StandardWrapper.java @@ -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 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, diff --git a/java/org/apache/catalina/core/StandardWrapperValve.java b/java/org/apache/catalina/core/StandardWrapperValve.java index 5512b3ccb..63af4f33c 100644 --- a/java/org/apache/catalina/core/StandardWrapperValve.java +++ b/java/org/apache/catalina/core/StandardWrapperValve.java @@ -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); diff --git a/java/org/apache/catalina/deploy/WebXml.java b/java/org/apache/catalina/deploy/WebXml.java index f2b29c896..230e9fc80 100644 --- a/java/org/apache/catalina/deploy/WebXml.java +++ b/java/org/apache/catalina/deploy/WebXml.java @@ -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()); } diff --git a/java/org/apache/catalina/startup/ContextConfig.java b/java/org/apache/catalina/startup/ContextConfig.java index 342a00d5c..dbf5efee6 100644 --- a/java/org/apache/catalina/startup/ContextConfig.java +++ b/java/org/apache/catalina/startup/ContextConfig.java @@ -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 initParam: jspServletDef.getParameterMap().entrySet()) { + servletDef.addInitParameter(initParam.getKey(), initParam.getValue()); + } + } + protected WebXml createWebXml() { return new WebXml(); } diff --git a/java/org/apache/jasper/servlet/JspServlet.java b/java/org/apache/jasper/servlet/JspServlet.java index 166831d7a..52e65ec18 100644 --- a/java/org/apache/jasper/servlet/JspServlet.java +++ b/java/org/apache/jasper/servlet/JspServlet.java @@ -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(){ + 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 in declaration - jspUri = jspFile; - } else { + if (jspUri == null) { + // JSP specified via in 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() diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 4d2028c0d..f71294232 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -269,6 +269,12 @@ jsp:attribute elements now supports the use of expressions and expression language. (markt) + + 49916: 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) + -- 2.11.0