Author: markt Date: Wed Jun 28 20:02:32 2017 New Revision: 1800201 URL: http://svn.apache.org/viewvc?rev=1800201&view=rev Log: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=49176 When generating JSP runtime error messages that quote the relevant JSP source code, switch from using the results of the JSP page parsing process to using the JSR 045 source map data to identify the correct part of the JSP source from the stack trace. This significantly reduces the memory footprint of Jasper in development mode, provides a small performance improvement for error page generation and enables source quotes to continue to be provided after a Tomcat restart.
Added: tomcat/trunk/java/org/apache/jasper/compiler/SmapInput.java (with props) tomcat/trunk/test/org/apache/jasper/compiler/TestSmapStratum.java (with props) Removed: tomcat/trunk/java/org/apache/jasper/compiler/SmapGenerator.java tomcat/trunk/test/org/apache/jasper/compiler/TestSmapGenerator.java Modified: tomcat/trunk/java/org/apache/jasper/JspC.java tomcat/trunk/java/org/apache/jasper/JspCompilationContext.java tomcat/trunk/java/org/apache/jasper/compiler/AntCompiler.java tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java tomcat/trunk/java/org/apache/jasper/compiler/JDTCompiler.java tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java tomcat/trunk/java/org/apache/jasper/compiler/SmapStratum.java tomcat/trunk/java/org/apache/jasper/compiler/SmapUtil.java tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/jasper/JspC.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/JspC.java?rev=1800201&r1=1800200&r2=1800201&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/JspC.java (original) +++ tomcat/trunk/java/org/apache/jasper/JspC.java Wed Jun 28 20:02:32 2017 @@ -1252,7 +1252,7 @@ public class JspC extends Task implement targetClassName = null; } if (targetPackage != null) { - clctxt.setServletPackageName(targetPackage); + clctxt.setBasePackageName(targetPackage); } originalClassLoader = Thread.currentThread().getContextClassLoader(); Modified: tomcat/trunk/java/org/apache/jasper/JspCompilationContext.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/JspCompilationContext.java?rev=1800201&r1=1800200&r2=1800201&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/JspCompilationContext.java (original) +++ tomcat/trunk/java/org/apache/jasper/JspCompilationContext.java Wed Jun 28 20:02:32 2017 @@ -445,7 +445,7 @@ public class JspCompilationContext { } /** - * Package name for the generated class is make up of the base package + * Package name for the generated class is made up of the base package * name, which is user settable, and the derived package name. The * derived package name directly mirrors the file hierarchy of the JSP page. * @return the package name @@ -478,11 +478,19 @@ public class JspCompilationContext { } /** + * @return The base package name into which all servlet and associated code + * is generated + */ + public String getBasePackageName() { + return basePackageName; + } + + /** * The package name into which the servlet class is generated. - * @param servletPackageName The package name to use + * @param basePackageName The package name to use */ - public void setServletPackageName(String servletPackageName) { - this.basePackageName = servletPackageName; + public void setBasePackageName(String basePackageName) { + this.basePackageName = basePackageName; } /** Modified: tomcat/trunk/java/org/apache/jasper/compiler/AntCompiler.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/AntCompiler.java?rev=1800201&r1=1800200&r2=1800201&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/AntCompiler.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/AntCompiler.java Wed Jun 28 20:02:32 2017 @@ -21,6 +21,7 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; import java.io.PrintStream; +import java.util.Map; import java.util.StringTokenizer; import org.apache.jasper.Constants; @@ -117,7 +118,7 @@ public class AntCompiler extends Compile * Compile the servlet from .java file to .class file */ @Override - protected void generateClass(String[] smap) + protected void generateClass(Map<String,SmapStratum> smaps) throws FileNotFoundException, JasperException, Exception { long t1 = 0; @@ -279,7 +280,7 @@ public class AntCompiler extends Compile // JSR45 Support if (!options.isSmapSuppressed()) { - SmapUtil.installSmap(smap); + SmapUtil.installSmap(smaps); } } Modified: tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java?rev=1800201&r1=1800200&r2=1800201&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/Compiler.java Wed Jun 28 20:02:32 2017 @@ -80,29 +80,30 @@ public abstract class Compiler { // --------------------------------------------------------- Public Methods - /** - * <p> - * Retrieves the parsed nodes of the JSP page, if they are available. May - * return null. Used in development mode for generating detailed error - * messages. http://bz.apache.org/bugzilla/show_bug.cgi?id=37062. - * </p> - * @return the page nodes - */ - public Node.Nodes getPageNodes() { - return this.pageNodes; + public SmapStratum getSmap(String className) { + + Map<String,SmapStratum> smaps = ctxt.getRuntimeContext().getSmaps(); + SmapStratum smap = smaps.get(className); + + if (smap == null && !options.isSmapSuppressed()) { + // Tomcat was restarted so cached SMAP has been lost. However, it + // was written to the class file so it can be recovered. + smap = SmapUtil.loadSmap(className, ctxt.getJspLoader()); + if (smap != null) { + smaps.put(className, smap); + } + } + + return smap; } /** * Compile the jsp file into equivalent servlet in .java file * - * @return a smap for the current JSP page, if one is generated, null - * otherwise * @throws Exception Error generating Java source */ - protected String[] generateJava() throws Exception { - - String[] smapStr = null; + protected Map<String,SmapStratum> generateJava() throws Exception { long t1, t2, t3, t4; @@ -212,7 +213,6 @@ public abstract class Compiler { // generate prototype .java file for the tag file try (ServletWriter writer = setupContextWriter(javaFileName)) { Generator.generate(writer, this, pageNodes); - return null; } } @@ -278,9 +278,14 @@ public abstract class Compiler { throw e; } + Map<String,SmapStratum> smaps = null; + // JSR45 Support if (!options.isSmapSuppressed()) { - smapStr = SmapUtil.generateSmap(ctxt, pageNodes); + smaps = SmapUtil.generateSmap(ctxt, pageNodes); + // Add them to the web application wide cache for future lookup in + // error handling etc. + ctxt.getRuntimeContext().getSmaps().putAll(smaps); } // If any proto type .java and .class files was generated, @@ -290,7 +295,7 @@ public abstract class Compiler { // generate .class again from the new .java file just generated. tfp.removeProtoTypeFiles(ctxt.getClassFileName()); - return smapStr; + return smaps; } private ServletWriter setupContextWriter(String javaFileName) @@ -316,12 +321,15 @@ public abstract class Compiler { /** * Servlet compilation. This compiles the generated sources into * Servlets. - * @param smap The SMAP files for source debugging + * + * @param smaps The source maps for the class(es) generated from the source + * file + * * @throws FileNotFoundException Source files not found * @throws JasperException Compilation error * @throws Exception Some other error */ - protected abstract void generateClass(String[] smap) + protected abstract void generateClass(Map<String,SmapStratum> smaps) throws FileNotFoundException, JasperException, Exception; /** @@ -371,12 +379,12 @@ public abstract class Compiler { } try { - String[] smap = generateJava(); + Map<String,SmapStratum> smaps = generateJava(); File javaFile = new File(ctxt.getServletJavaFileName()); Long jspLastModified = ctxt.getLastModified(ctxt.getJspFile()); javaFile.setLastModified(jspLastModified.longValue()); if (compileClass) { - generateClass(smap); + generateClass(smaps); // Fix for bugzilla 41606 // Set JspServletWrapper.servletClassLastModifiedTime after successful compile File targetFile = new File(ctxt.getClassFileName()); @@ -399,14 +407,7 @@ public abstract class Compiler { tfp = null; errDispatcher = null; pageInfo = null; - - // Only get rid of the pageNodes if in production. - // In development mode, they are used for detailed - // error messages. - // http://bz.apache.org/bugzilla/show_bug.cgi?id=37062 - if (!this.options.getDevelopment()) { - pageNodes = null; - } + pageNodes = null; if (ctxt.getWriter() != null) { ctxt.getWriter().close(); Modified: tomcat/trunk/java/org/apache/jasper/compiler/JDTCompiler.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/JDTCompiler.java?rev=1800201&r1=1800200&r2=1800201&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/JDTCompiler.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/JDTCompiler.java Wed Jun 28 20:02:32 2017 @@ -69,7 +69,7 @@ public class JDTCompiler extends org.apa * Compile the servlet from .java file to .class file */ @Override - protected void generateClass(String[] smap) + protected void generateClass(Map<String,SmapStratum> smaps) throws FileNotFoundException, JasperException, Exception { long t1 = 0; @@ -470,7 +470,7 @@ public class JDTCompiler extends org.apa // JSR45 Support if (! options.isSmapSuppressed()) { - SmapUtil.installSmap(smap); + SmapUtil.installSmap(smaps); } } } Modified: tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java?rev=1800201&r1=1800200&r2=1800201&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/JspRuntimeContext.java Wed Jun 28 20:02:32 2017 @@ -169,6 +169,14 @@ public final class JspRuntimeContext { */ private FastRemovalDequeue<JspServletWrapper> jspQueue = null; + /** + * Map of class name to associated source map. This is maintained here as + * multiple JSPs can depend on the same file (included JSP, tag file, etc.) + * so a web application scoped Map is required. + */ + private final Map<String,SmapStratum> smaps = new ConcurrentHashMap<>(); + + // ------------------------------------------------------ Public Methods /** @@ -389,9 +397,13 @@ public final class JspRuntimeContext { } - // -------------------------------------------------------- Private Methods + public Map<String,SmapStratum> getSmaps() { + return smaps; + } + // -------------------------------------------------------- Private Methods + /** * Method used to initialize classpath for compiles. * @return the compilation classpath Added: tomcat/trunk/java/org/apache/jasper/compiler/SmapInput.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/SmapInput.java?rev=1800201&view=auto ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/SmapInput.java (added) +++ tomcat/trunk/java/org/apache/jasper/compiler/SmapInput.java Wed Jun 28 20:02:32 2017 @@ -0,0 +1,39 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jasper.compiler; + +public class SmapInput { + + private final String fileName; + private final int lineNumber; + + + public SmapInput(String fileName, int lineNumber) { + this.fileName = fileName; + this.lineNumber = lineNumber; + } + + + public String getFileName() { + return fileName; + } + + + public int getLineNumber() { + return lineNumber; + } +} Propchange: tomcat/trunk/java/org/apache/jasper/compiler/SmapInput.java ------------------------------------------------------------------------------ svn:eol-style = native Modified: tomcat/trunk/java/org/apache/jasper/compiler/SmapStratum.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/SmapStratum.java?rev=1800201&r1=1800200&r2=1800201&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/SmapStratum.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/SmapStratum.java Wed Jun 28 20:02:32 2017 @@ -36,7 +36,7 @@ public class SmapStratum { * Represents a single LineSection in an SMAP, associated with * a particular stratum. */ - private static class LineInfo { + static class LineInfo { private int inputStartLine = -1; private int outputStartLine = -1; private int lineFileID = 0; @@ -83,6 +83,10 @@ public class SmapStratum { this.outputLineIncrement = outputLineIncrement; } + public int getMaxOutputLineNumber() { + return outputStartLine + inputLineCount * outputLineIncrement; + } + /** * @return the current LineInfo as a String, print all values only when * appropriate (but LineInfoID if and only if it's been @@ -113,23 +117,14 @@ public class SmapStratum { //********************************************************************* // Private state - private final List<String> fileNameList; - private final List<String> filePathList; - private final List<LineInfo> lineData; + private final List<String> fileNameList = new ArrayList<>(); + private final List<String> filePathList = new ArrayList<>(); + private final List<LineInfo> lineData = new ArrayList<>(); private int lastFileID; - - //********************************************************************* - // Constructor - - /** - * Constructs a new SmapStratum object with the stratum name JSP. - */ - public SmapStratum() { - fileNameList = new ArrayList<>(); - filePathList = new ArrayList<>(); - lineData = new ArrayList<>(); - lastFileID = 0; - } + // .java file + private String outputFileName; + // .class file + private String classFileName; //********************************************************************* // Methods to add mapping information @@ -270,20 +265,59 @@ public class SmapStratum { lineData.add(li); } + + public void addLineInfo(LineInfo li) { + lineData.add(li); + } + + + public void setOutputFileName(String outputFileName) { + this.outputFileName = outputFileName; + } + + + public void setClassFileName(String classFileName) { + this.classFileName = classFileName; + } + + + public String getClassFileName() { + return classFileName; + } + + //********************************************************************* // Methods to retrieve information - /** - * @return the given stratum as a String: a StratumSection, - * followed by at least one FileSection and at least one LineSection. - */ - public String getString() { - // check state and initialize buffer - if (fileNameList.size() == 0 || lineData.size() == 0) - return null; + @Override + public String toString() { + return getSmapStringInternal(); + } + + + public String getSmapString() { + + if (outputFileName == null) { + throw new IllegalStateException(); + } + return getSmapStringInternal(); + } + + + private String getSmapStringInternal() { StringBuilder out = new StringBuilder(); + // start the SMAP + out.append("SMAP\n"); + out.append(outputFileName + '\n'); + out.append("JSP\n"); + + // print our StratumSection, FileSection, and LineSections + if (fileNameList.size() == 0 || lineData.size() == 0) { + throw new IllegalStateException(); + } + // print StratumSection out.append("*S JSP\n"); @@ -313,12 +347,40 @@ public class SmapStratum { out.append(li.getString()); } + // end the SMAP + out.append("*E\n"); + return out.toString(); } - @Override - public String toString() { - return getString(); - } + public SmapInput getInputLineNumber(int outputLineNumber) { + // For a given Java line number, provide the associated line number + // in the JSP/tag source + int inputLineNumber = -1; + int fileId = 0; + + for (LineInfo lineInfo : lineData) { + if (lineInfo.lineFileIDSet) { + fileId = lineInfo.lineFileID; + } + if (lineInfo.outputStartLine > outputLineNumber) { + // Didn't find match + break; + } + + if (lineInfo.getMaxOutputLineNumber() < outputLineNumber) { + // Too early + continue; + } + + // This is the match + int inputOffset = + (outputLineNumber - lineInfo.outputStartLine) / lineInfo.outputLineIncrement; + + inputLineNumber = lineInfo.inputStartLine + inputOffset; + } + + return new SmapInput(filePathList.get(fileId), inputLineNumber); + } } Modified: tomcat/trunk/java/org/apache/jasper/compiler/SmapUtil.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/SmapUtil.java?rev=1800201&r1=1800200&r2=1800201&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/compiler/SmapUtil.java (original) +++ tomcat/trunk/java/org/apache/jasper/compiler/SmapUtil.java Wed Jun 28 20:02:32 2017 @@ -17,20 +17,24 @@ package org.apache.jasper.compiler; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.io.UnsupportedEncodingException; +import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.Map; import org.apache.jasper.JasperException; import org.apache.jasper.JspCompilationContext; +import org.apache.jasper.compiler.SmapStratum.LineInfo; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; @@ -49,7 +53,9 @@ public class SmapUtil { //********************************************************************* // Constants - private static final String SMAP_ENCODING = "UTF-8"; + private static final Charset SMAP_ENCODING = StandardCharsets.UTF_8; + + private static final Log log = LogFactory.getLog(SmapUtil.class); //********************************************************************* // Public entry points @@ -63,10 +69,10 @@ public class SmapUtil { * @return a SMAP for the page * @throws IOException Error writing SMAP */ - public static String[] generateSmap( - JspCompilationContext ctxt, - Node.Nodes pageNodes) - throws IOException { + public static Map<String,SmapStratum> generateSmap(JspCompilationContext ctxt, + Node.Nodes pageNodes) throws IOException { + + Map<String,SmapStratum> smapInfo = new HashMap<>(); // Scan the nodes for presence of Jasper generated inner classes PreScanVisitor psVisitor = new PreScanVisitor(); @@ -76,48 +82,42 @@ public class SmapUtil { } HashMap<String, SmapStratum> map = psVisitor.getMap(); - // set up our SMAP generator - SmapGenerator g = new SmapGenerator(); - - // now, assemble info about our own stratum (JSP) using JspLineMap + // Assemble info about our own stratum (JSP) using JspLineMap SmapStratum s = new SmapStratum(); - g.setOutputFileName(unqualify(ctxt.getServletJavaFileName())); - // Map out Node.Nodes evaluateNodes(pageNodes, s, map, ctxt.getOptions().getMappedFile()); s.optimizeLineSection(); - g.setStratum(s); + s.setOutputFileName(unqualify(ctxt.getServletJavaFileName())); + + String classFileName = ctxt.getClassFileName(); + s.setClassFileName(classFileName); + + smapInfo.put(ctxt.getFQCN(), s); if (ctxt.getOptions().isSmapDumped()) { - File outSmap = new File(ctxt.getClassFileName() + ".smap"); + File outSmap = new File(classFileName + ".smap"); PrintWriter so = new PrintWriter( new OutputStreamWriter( new FileOutputStream(outSmap), SMAP_ENCODING)); - so.print(g.getString()); + so.print(s.getSmapString()); so.close(); } - String classFileName = ctxt.getClassFileName(); - int innerClassCount = map.size(); - String [] smapInfo = new String[2 + innerClassCount*2]; - smapInfo[0] = classFileName; - smapInfo[1] = g.getString(); - - int count = 2; for (Map.Entry<String, SmapStratum> entry : map.entrySet()) { String innerClass = entry.getKey(); s = entry.getValue(); s.optimizeLineSection(); - g = new SmapGenerator(); - g.setOutputFileName(unqualify(ctxt.getServletJavaFileName())); - g.setStratum(s); - + s.setOutputFileName(unqualify(ctxt.getServletJavaFileName())); String innerClassFileName = classFileName.substring(0, classFileName.indexOf(".class")) + '$' + innerClass + ".class"; + s.setClassFileName(innerClassFileName); + + smapInfo.put(ctxt.getFQCN() + "." + innerClass, s); + if (ctxt.getOptions().isSmapDumped()) { File outSmap = new File(innerClassFileName + ".smap"); PrintWriter so = @@ -125,27 +125,24 @@ public class SmapUtil { new OutputStreamWriter( new FileOutputStream(outSmap), SMAP_ENCODING)); - so.print(g.getString()); + so.print(s.getSmapString()); so.close(); } - smapInfo[count] = innerClassFileName; - smapInfo[count+1] = g.getString(); - count += 2; } return smapInfo; } - public static void installSmap(String[] smap) + public static void installSmap(Map<String,SmapStratum> smapInfo) throws IOException { - if (smap == null) { + if (smapInfo == null) { return; } - for (int i = 0; i < smap.length; i += 2) { - File outServlet = new File(smap[i]); + for (Map.Entry<String,SmapStratum> entry : smapInfo.entrySet()) { + File outServlet = new File(entry.getValue().getClassFileName()); SDEInstaller.install(outServlet, - smap[i+1].getBytes(StandardCharsets.ISO_8859_1)); + entry.getValue().getSmapString().getBytes(StandardCharsets.ISO_8859_1)); } } @@ -692,4 +689,128 @@ public class SmapUtil { } } + public static SmapStratum loadSmap(String className, ClassLoader cl) { + // Extract SMAP from class file. First line "SMAP" is not included + String smap = getSmap(className, cl); + + if (smap == null) { + return null; + } + + SmapStratum smapStratum = new SmapStratum(); + + String[] lines = smap.split("\n"); + int lineIndex = 0; + + // First line is output file name + smapStratum.setOutputFileName(lines[lineIndex]); + + // There is only one stratum (JSP) so skip to the start of the file + // section + lineIndex = 4; + + while (!lines[lineIndex].equals("*L")) { + int i = lines[lineIndex].lastIndexOf(' '); + String fileName = lines[lineIndex].substring(i + 1); + smapStratum.addFile(fileName, lines[++lineIndex]); + lineIndex++; + } + + // Skip *L + lineIndex++; + + while (!lines[lineIndex].equals("*E")) { + LineInfo li = new LineInfo(); + // Split into in and out + String[] inOut = lines[lineIndex].split(":"); + // Split in on comma (might not be one) + String[] in = inOut[0].split(","); + if (in.length == 2) { + // There is a count + li.setInputLineCount(Integer.parseInt(in[1])); + } + // Check for fileID + String[] start = in[0].split("#"); + if (start.length == 2) { + // There is a file ID + li.setLineFileID(Integer.parseInt(start[1])); + } + li.setInputStartLine(Integer.parseInt(start[0])); + // Split out + String[] out = inOut[1].split(","); + if (out.length == 2) { + // There is an increment + li.setOutputLineIncrement(Integer.parseInt(out[1])); + } + li.setOutputStartLine(Integer.parseInt(out[0])); + + smapStratum.addLineInfo(li); + + lineIndex++; + } + + return smapStratum; + } + + + private static String getSmap(String className, ClassLoader cl) { + Charset encoding = StandardCharsets.ISO_8859_1; + boolean found = false; + String smap = null; + + InputStream is = null; + try { + is = cl.getResourceAsStream(className.replaceAll("\\.","/") + ".smap"); + if (is != null) { + encoding = SMAP_ENCODING; + found = true; + } else { + is = cl.getResourceAsStream(className.replaceAll("\\.","/") + ".class"); + // Alternative approach would be to read the class file as per the + // JLS. That would require duplicating a lot of BCEL functionality. + int b = is.read(); + while (b != -1) { + if (b == 'S') { + if ((b = is.read()) != 'M') { + continue; + } + if ((b = is.read()) != 'A') { + continue; + } + if ((b = is.read()) != 'P') { + continue; + } + if ((b = is.read()) != '\n') { + continue; + } + found = true; + break; + } + b = is.read(); + } + } + + if (found) { + ByteArrayOutputStream baos = new ByteArrayOutputStream(1024); + byte[] buf = new byte[1024]; + int numRead; + while ( (numRead = is.read(buf) ) >= 0) { + baos.write(buf, 0, numRead); + } + + smap = new String(baos.toByteArray(), encoding); + } + } catch (IOException ioe) { + log.warn(Localizer.getMessage("jsp.warning.loadSmap", className), ioe); + } finally { + if (is != null) { + try { + is.close(); + } catch (IOException ioe) { + log.warn(Localizer.getMessage("jsp.warning.loadSmap", className), ioe); + } + } + } + return smap; + } } Modified: tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties?rev=1800201&r1=1800200&r2=1800201&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/jasper/resources/LocalStrings.properties Wed Jun 28 20:02:32 2017 @@ -119,6 +119,7 @@ jsp.warning.recompileOnFail=Warning: Inv jsp.warning.development=Warning: Invalid value for the initParam development. Will use the default value of "true" jsp.warning.fork=Warning: Invalid value for the initParam fork. Will use the default value of "true" jsp.warning.dumpSmap=Warning: Invalid value for the initParam dumpSmap. Will use the default value of "false" +jsp.warning.loadSmap=Unable to load SMAP data for class [{0}] jsp.warning.genchararray=Warning: Invalid value for the initParam genStringAsCharArray. Will use the default value of "false" jsp.warning.suppressSmap=Warning: Invalid value for the initParam suppressSmap. Will use the default value of "false" jsp.warning.displaySourceFragment=Warning: Invalid value for the initParam displaySourceFragment. Will use the default value of "true" @@ -361,7 +362,7 @@ jsp.error.prefix.use_before_dcl=The pref jsp.error.lastModified=Unable to determine last modified date for file [{0}] jsp.info.ignoreSetting=Ignored setting for [{0}] of [{1}] because a SecurityManager was enabled -jsp.exception=An exception occurred processing JSP page [{0}] at line [{1}] +jsp.exception=An exception occurred processing [{0}] at line [{1}] # JSP 2.1 jsp.error.el.template.deferred=#{...} is not allowed in template text Modified: tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java?rev=1800201&r1=1800200&r2=1800201&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java (original) +++ tomcat/trunk/java/org/apache/jasper/servlet/JspServletWrapper.java Wed Jun 28 20:02:32 2017 @@ -36,10 +36,11 @@ import javax.servlet.jsp.tagext.TagInfo; import org.apache.jasper.JasperException; import org.apache.jasper.JspCompilationContext; import org.apache.jasper.Options; -import org.apache.jasper.compiler.ErrorDispatcher; import org.apache.jasper.compiler.JavacErrorDetail; import org.apache.jasper.compiler.JspRuntimeContext; import org.apache.jasper.compiler.Localizer; +import org.apache.jasper.compiler.SmapInput; +import org.apache.jasper.compiler.SmapStratum; import org.apache.jasper.runtime.ExceptionUtils; import org.apache.jasper.runtime.InstanceManagerFactory; import org.apache.jasper.runtime.JspSourceDependent; @@ -545,45 +546,49 @@ public class JspServletWrapper { realException = ((ServletException) ex).getRootCause(); } - // First identify the stack frame in the trace that represents the JSP + // Find the first stack frame that represents code generated by + // Jasper StackTraceElement[] frames = realException.getStackTrace(); StackTraceElement jspFrame = null; - for (int i=0; i<frames.length; ++i) { - if ( frames[i].getClassName().equals(this.getServlet().getClass().getName()) ) { - jspFrame = frames[i]; + String servletPackageName = ctxt.getBasePackageName(); + for (StackTraceElement frame : frames) { + if (frame.getClassName().startsWith(servletPackageName)) { + jspFrame = frame; break; } } + SmapStratum smap = null; - if (jspFrame == null || - this.ctxt.getCompiler().getPageNodes() == null) { + if (jspFrame != null) { + smap = ctxt.getCompiler().getSmap(jspFrame.getClassName()); + } + + if (smap == null) { // If we couldn't find a frame in the stack trace corresponding // to the generated servlet class or we don't have a copy of the - // parsed JSP to hand, we can't really add anything + // smap to hand, we can't really add anything return new JasperException(ex); } + @SuppressWarnings("null") int javaLineNumber = jspFrame.getLineNumber(); - JavacErrorDetail detail = ErrorDispatcher.createJavacError( - jspFrame.getMethodName(), - this.ctxt.getCompiler().getPageNodes(), - null, - javaLineNumber, - ctxt); + SmapInput source = smap.getInputLineNumber(javaLineNumber); // If the line number is less than one we couldn't find out // where in the JSP things went wrong - int jspLineNumber = detail.getJspBeginLineNumber(); - if (jspLineNumber < 1) { + if (source.getLineNumber() < 1) { throw new JasperException(ex); } + JavacErrorDetail detail = new JavacErrorDetail(jspFrame.getMethodName(), javaLineNumber, + source.getFileName(), source.getLineNumber(), null, ctxt); + if (options.getDisplaySourceFragment()) { return new JasperException(Localizer.getMessage ("jsp.exception", detail.getJspFileName(), - "" + jspLineNumber) + System.lineSeparator() + + "" + source.getLineNumber()) + System.lineSeparator() + System.lineSeparator() + detail.getJspExtract() + System.lineSeparator() + System.lineSeparator() + "Stacktrace:", ex); @@ -592,7 +597,7 @@ public class JspServletWrapper { return new JasperException(Localizer.getMessage ("jsp.exception", detail.getJspFileName(), - "" + jspLineNumber), ex); + "" + source.getLineNumber()), ex); } catch (Exception je) { // If anything goes wrong, just revert to the original behaviour if (ex instanceof JasperException) { Added: tomcat/trunk/test/org/apache/jasper/compiler/TestSmapStratum.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/jasper/compiler/TestSmapStratum.java?rev=1800201&view=auto ============================================================================== --- tomcat/trunk/test/org/apache/jasper/compiler/TestSmapStratum.java (added) +++ tomcat/trunk/test/org/apache/jasper/compiler/TestSmapStratum.java Wed Jun 28 20:02:32 2017 @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jasper.compiler; + +import org.junit.Assert; +import org.junit.Test; + +public class TestSmapStratum { + + @Test + public void test01() { + // Formerly part of the main() method in SmapGenerator + + SmapStratum s = new SmapStratum(); + s.addFile("foo.jsp"); + s.addFile("bar.jsp", "/foo/foo/bar.jsp"); + s.addLineData(1, "foo.jsp", 1, 1, 1); + s.addLineData(2, "foo.jsp", 1, 6, 1); + s.addLineData(3, "foo.jsp", 2, 10, 5); + s.addLineData(20, "/foo/foo/bar.jsp", 1, 30, 1); + s.setOutputFileName("foo.java"); + + Assert.assertEquals( + "SMAP\n" + + "foo.java\n" + + "JSP\n" + + "*S JSP\n" + + "*F\n" + + "+ 0 foo.jsp\n" + + "foo.jsp\n" + + "+ 1 bar.jsp\n" + + "foo/foo/bar.jsp\n" + + "*L\n" + + "1:1\n" + + "2:6\n" + + "3,2:10,5\n" + + "20#1:30\n" + + "*E\n", + s.getSmapString()); + } +} Propchange: tomcat/trunk/test/org/apache/jasper/compiler/TestSmapStratum.java ------------------------------------------------------------------------------ svn:eol-style = native Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1800201&r1=1800200&r2=1800201&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Wed Jun 28 20:02:32 2017 @@ -102,6 +102,20 @@ </fix> </changelog> </subsection> + <subsection name="Jasper"> + <changelog> + <fix> + <bug>49176</bug>: When generating JSP runtime error messages that quote + the relevant JSP source code, switch from using the results of the JSP + page parsing process to using the JSR 045 source map data to identify + the correct part of the JSP source from the stack trace. This + significantly reduces the memory footprint of Jasper in development + mode, provides a small performance improvement for error page generation + and enables source quotes to continue to be provided after a Tomcat + restart. (markt) + </fix> + </changelog> + </subsection> <subsection name="Tribes"> <changelog> <add> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org