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

Reply via email to