Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=33453

Recompile JSPs if last modified time of the source or any of its dependencies changes either forwards or backwards. Note that this introduces an incompatible change to the code generated for JSPs. Tomcat will automatically re-compile any JSPs and tag files found in the work directory when upgrading from 7.0.16 or earlier to 7.0.17 or later. If you later downgrade from 7.0.17 or later to 7.0.16 or earlier, you must empty the work directory as part of the downgrade process.

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

diff --git a/java/org/apache/catalina/loader/WebappClassLoader.java b/java/org/apache/catalina/loader/WebappClassLoader.java
index 3b7a8ec..3707aad 100644 (file)
--- a/java/org/apache/catalina/loader/WebappClassLoader.java
+++ b/java/org/apache/catalina/loader/WebappClassLoader.java
@@ -3025,6 +3025,8 @@ public class WebappClassLoader
                                                 }
                                                 os.write(buf, 0, n);
                                             }
+                                            resourceFile.setLastModified(
+                                                    jarEntry2.getTime());
                                         } catch (IOException e) {
                                             // Ignore
                                         } finally {

diff --git a/java/org/apache/jasper/JspCompilationContext.java b/java/org/apache/jasper/JspCompilationContext.java
index c336c75..1775974 100644 (file)
--- a/java/org/apache/jasper/JspCompilationContext.java
+++ b/java/org/apache/jasper/JspCompilationContext.java
@@ -385,6 +385,11 @@ public class JspCompilationContext {
         return jspUri;
     }
 
+    /**
+     * @deprecated Will be removed in Tomcat 8.0.x. Use
+     * {@link #getLastModified(String)} instead.
+     */
+    @Deprecated
     public long getJspLastModified() {
         long result = -1;
         URLConnection uc = null;
@@ -422,6 +427,44 @@ public class JspCompilationContext {
         return result;
     }
 
+
+    public Long getLastModified(String resource) {
+        long result = -1;
+        URLConnection uc = null;
+        try {
+            URL jspUrl = getResource(resource);
+            if (jspUrl == null) {
+                incrementRemoved();
+                return Long.valueOf(result);
+            }
+            uc = jspUrl.openConnection();
+            if (uc instanceof JarURLConnection) {
+                result = ((JarURLConnection) uc).getJarEntry().getTime();
+            } else {
+                result = uc.getLastModified();
+            }
+        } catch (IOException e) {
+            if (log.isDebugEnabled()) {
+                log.debug(Localizer.getMessage(
+                        "jsp.error.lastModified", getJspFile()), e);
+            }
+            result = -1;
+        } finally {
+            if (uc != null) {
+                try {
+                    uc.getInputStream().close();
+                } catch (IOException e) {
+                    if (log.isDebugEnabled()) {
+                        log.debug(Localizer.getMessage(
+                                "jsp.error.lastModified", getJspFile()), e);
+                    }
+                    result = -1;
+                }
+            }
+        }
+        return Long.valueOf(result);
+    }
+
     public boolean isTagFile() {
         return isTagFile;
     }

diff --git a/java/org/apache/jasper/compiler/Compiler.java b/java/org/apache/jasper/compiler/Compiler.java
index 1f89595..0e442a2 100644 (file)
--- a/java/org/apache/jasper/compiler/Compiler.java
+++ b/java/org/apache/jasper/compiler/Compiler.java
@@ -27,7 +27,8 @@ import java.net.JarURLConnection;
 import java.net.URL;
 import java.net.URLConnection;
 import java.util.Iterator;
-import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
 
 import org.apache.jasper.JasperException;
 import org.apache.jasper.JspCompilationContext;
@@ -370,6 +371,9 @@ public abstract class Compiler {
 
         try {
             String[] smap = generateJava();
+            File javaFile = new File(ctxt.getServletJavaFileName());
+            Long jspLastModified = ctxt.getLastModified(ctxt.getJspFile());
+            javaFile.setLastModified(jspLastModified.longValue());
             if (compileClass) {
                 generateClass(smap);
                 // Fix for bugzilla 41606
@@ -377,8 +381,12 @@ public abstract class Compiler {
                 String targetFileName = ctxt.getClassFileName();
                 if (targetFileName != null) {
                     File targetFile = new File(targetFileName);
-                    if (targetFile.exists() && jsw != null) {
-                        jsw.setServletClassLastModifiedTime(targetFile.lastModified());
+                    if (targetFile.exists()) {
+                        targetFile.setLastModified(jspLastModified.longValue());
+                        if (jsw != null) {
+                            jsw.setServletClassLastModifiedTime(
+                                    jspLastModified.longValue());
+                        }
                     }
                 }
             }
@@ -440,8 +448,8 @@ public abstract class Compiler {
             jsw.setLastModificationTest(System.currentTimeMillis());
         }
 
-        long jspRealLastModified = ctxt.getJspLastModified();
-        if (jspRealLastModified < 0) {
+        Long jspRealLastModified = ctxt.getLastModified(ctxt.getJspFile());
+        if (jspRealLastModified.longValue() < 0) {
             // Something went wrong - assume modification
             return true;
         }
@@ -463,7 +471,7 @@ public abstract class Compiler {
         if (checkClass && jsw != null) {
             jsw.setServletClassLastModifiedTime(targetLastModified);
         }
-        if (targetLastModified < jspRealLastModified) {
+        if (targetLastModified != jspRealLastModified.longValue()) {
             if (log.isDebugEnabled()) {
                 log.debug("Compiler: outdated: " + targetFile + " "
                         + targetLastModified);
@@ -477,16 +485,16 @@ public abstract class Compiler {
             return false;
         }
 
-        List<String> depends = jsw.getDependants();
+        Map<String,Long> depends = jsw.getDependants();
         if (depends == null) {
             return false;
         }
 
-        Iterator<String> it = depends.iterator();
+        Iterator<Entry<String,Long>> it = depends.entrySet().iterator();
         while (it.hasNext()) {
-            String include = it.next();
+            Entry<String,Long> include = it.next();
             try {
-                URL includeUrl = ctxt.getResource(include);
+                URL includeUrl = ctxt.getResource(include.getKey());
                 if (includeUrl == null) {
                     return true;
                 }
@@ -501,7 +509,7 @@ public abstract class Compiler {
                 }
                 iuc.getInputStream().close();
 
-                if (includeLastModified > targetLastModified) {
+                if (includeLastModified != include.getValue().longValue()) {
                     return true;
                 }
             } catch (Exception e) {

diff --git a/java/org/apache/jasper/compiler/Generator.java b/java/org/apache/jasper/compiler/Generator.java
index 1153279..998dcca 100644 (file)
--- a/java/org/apache/jasper/compiler/Generator.java
+++ b/java/org/apache/jasper/compiler/Generator.java
@@ -34,6 +34,8 @@ import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 import java.util.TimeZone;
 import java.util.Vector;
@@ -525,20 +527,23 @@ class Generator {
         out.println();
 
         // Static data for getDependants()
-        out.printil("private static java.util.List<java.lang.String> _jspx_dependants;");
+        out.printil("private static java.util.Map<java.lang.String,java.lang.Long> _jspx_dependants;");
         out.println();
-        List<String> dependants = pageInfo.getDependants();
-        Iterator<String> iter = dependants.iterator();
+        Map<String,Long> dependants = pageInfo.getDependants();
+        Iterator<Entry<String,Long>> iter = dependants.entrySet().iterator();
         if (!dependants.isEmpty()) {
             out.printil("static {");
             out.pushIndent();
-            out.printin("_jspx_dependants = new java.util.ArrayList<java.lang.String>(");
+            out.printin("_jspx_dependants = new java.util.HashMap<java.lang.String,java.lang.Long>(");
             out.print("" + dependants.size());
             out.println(");");
             while (iter.hasNext()) {
-                out.printin("_jspx_dependants.add(\"");
-                out.print(iter.next());
-                out.println("\");");
+                Entry<String,Long> entry = iter.next();
+                out.printin("_jspx_dependants.put(\"");
+                out.print(entry.getKey());
+                out.print("\", Long.valueOf(");
+                out.print(entry.getValue().toString());
+                out.println("L));");
             }
             out.popIndent();
             out.printil("}");
@@ -576,7 +581,7 @@ class Generator {
      */
     private void genPreambleMethods() {
         // Method used to get compile time file dependencies
-        out.printil("public java.util.List<java.lang.String> getDependants() {");
+        out.printil("public java.util.Map<java.lang.String,java.lang.Long> getDependants() {");
         out.pushIndent();
         out.printil("return _jspx_dependants;");
         out.popIndent();
@@ -3494,6 +3499,9 @@ class Generator {
         out.println(" * Version: " + ctxt.getServletContext().getServerInfo());
         out.println(" * Generated at: " + timestampFormat.format(new Date()) +
                 " UTC");
+        out.println(" * Note: The last modified time of this file was set to");
+        out.println(" *       the last modified time of the source file after");
+        out.println(" *       generation to assist with modification tracking.");
         out.println(" */");
     }
 

diff --git a/java/org/apache/jasper/compiler/ImplicitTagLibraryInfo.java b/java/org/apache/jasper/compiler/ImplicitTagLibraryInfo.java
index 47e1b86..7c18ab7 100644 (file)
--- a/java/org/apache/jasper/compiler/ImplicitTagLibraryInfo.java
+++ b/java/org/apache/jasper/compiler/ImplicitTagLibraryInfo.java
@@ -121,7 +121,7 @@ class ImplicitTagLibraryInfo extends TagLibraryInfo {
                             
                             // Add implicit TLD to dependency list
                             if (pi != null) {
-                                pi.addDependant(path);
+                                pi.addDependant(path, ctxt.getLastModified(path));
                             }
                             
                             ParserUtils pu = new ParserUtils();

diff --git a/java/org/apache/jasper/compiler/PageInfo.java b/java/org/apache/jasper/compiler/PageInfo.java
index f4274ba..72508e5 100644 (file)
--- a/java/org/apache/jasper/compiler/PageInfo.java
+++ b/java/org/apache/jasper/compiler/PageInfo.java
@@ -21,6 +21,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.Vector;
 
@@ -39,7 +40,7 @@ import org.apache.jasper.JasperException;
 class PageInfo {
 
     private Vector<String> imports;
-    private Vector<String> dependants;
+    private Map<String,Long> dependants;
 
     private BeanRepository beanRepository;
     private Set<String> varInfoNames;
@@ -108,7 +109,7 @@ class PageInfo {
         this.xmlPrefixMapper = new HashMap<String, LinkedList<String>>();
         this.nonCustomTagPrefixMap = new HashMap<String, Mark>();
         this.imports = new Vector<String>();
-        this.dependants = new Vector<String>();
+        this.dependants = new HashMap<String,Long>();
         this.includePrelude = new Vector<String>();
         this.includeCoda = new Vector<String>();
         this.pluginDcls = new Vector<String>();
@@ -146,12 +147,12 @@ class PageInfo {
         return jspFile;
     }
 
-    public void addDependant(String d) {
-        if (!dependants.contains(d) && !jspFile.equals(d))
-                dependants.add(d);
+    public void addDependant(String d, Long lastModified) {
+        if (!dependants.containsKey(d) && !jspFile.equals(d))
+                dependants.put(d, lastModified);
     }
 
-    public List<String> getDependants() {
+    public Map<String,Long> getDependants() {
         return dependants;
     }
 

diff --git a/java/org/apache/jasper/compiler/ParserController.java b/java/org/apache/jasper/compiler/ParserController.java
index 67f4797..986ada9 100644 (file)
--- a/java/org/apache/jasper/compiler/ParserController.java
+++ b/java/org/apache/jasper/compiler/ParserController.java
@@ -192,10 +192,13 @@ class ParserController implements TagConstants {
         if (parent != null) {
             // Included resource, add to dependent list
             if (jarFile == null) {
-                compiler.getPageInfo().addDependant(absFileName);
+                compiler.getPageInfo().addDependant(absFileName,
+                        ctxt.getLastModified(absFileName));
             } else {
+                String entry = absFileName.substring(1);
                 compiler.getPageInfo().addDependant(
-                        jarResource.getEntry(absFileName.substring(1)).toString());
+                        jarResource.getEntry(entry).toString(),
+                        Long.valueOf(jarFile.getEntry(entry).getTime()));
                         
             }
         }

diff --git a/java/org/apache/jasper/compiler/TagFileProcessor.java b/java/org/apache/jasper/compiler/TagFileProcessor.java
index bc459b0..e575977 100644 (file)
--- a/java/org/apache/jasper/compiler/TagFileProcessor.java
+++ b/java/org/apache/jasper/compiler/TagFileProcessor.java
@@ -21,6 +21,7 @@ import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.Map.Entry;
 import java.util.Vector;
 
 import javax.el.MethodExpression;
@@ -586,10 +587,12 @@ class TagFileProcessor {
             try {
                 Object tagIns = tagClazz.newInstance();
                 if (tagIns instanceof JspSourceDependent) {
-                    Iterator<String> iter = ((JspSourceDependent) tagIns)
-                            .getDependants().iterator();
+                    Iterator<Entry<String,Long>> iter = ((JspSourceDependent)
+                            tagIns).getDependants().entrySet().iterator();
                     while (iter.hasNext()) {
-                        parentPageInfo.addDependant(iter.next());
+                        Entry<String,Long> entry = iter.next();
+                        parentPageInfo.addDependant(entry.getKey(),
+                                entry.getValue());
                     }
                 }
             } catch (Exception e) {
@@ -628,16 +631,26 @@ class TagFileProcessor {
                             tagFileInfo.getTagInfo().getTagLibrary().getURI());
                     JarResource jarResource = location.getJarResource();
                     if (jarResource != null) {
-                        // Add TLD
-                        pageInfo.addDependant(jarResource.getEntry(location.getName()).toString());
-                        // Add Tag
-                        pageInfo.addDependant(jarResource.getEntry(tagFilePath.substring(1)).toString());
+                        try {
+                            // Add TLD
+                            pageInfo.addDependant(jarResource.getEntry(location.getName()).toString(),
+                                    Long.valueOf(jarResource.getJarFile().getEntry(location.getName()).getTime()));
+                            // Add Tag
+                            pageInfo.addDependant(jarResource.getEntry(tagFilePath.substring(1)).toString(),
+                                    Long.valueOf(jarResource.getJarFile().getEntry(tagFilePath.substring(1)).getTime()));
+                        } catch (IOException ioe) {
+                            throw new JasperException(ioe);
+                        }
                     }
                     else {
-                        pageInfo.addDependant(tagFilePath);
+                        pageInfo.addDependant(tagFilePath,
+                                compiler.getCompilationContext().getLastModified(
+                                        tagFilePath));
                     }
                 } else {
-                    pageInfo.addDependant(tagFilePath);
+                    pageInfo.addDependant(tagFilePath,
+                            compiler.getCompilationContext().getLastModified(
+                                    tagFilePath));
                 }
                 Class<?> c = loadTagFile(compiler, tagFilePath, n.getTagInfo(),
                         pageInfo);

diff --git a/java/org/apache/jasper/compiler/TagLibraryInfoImpl.java b/java/org/apache/jasper/compiler/TagLibraryInfoImpl.java
index da8fdd3..df8f327 100644 (file)
--- a/java/org/apache/jasper/compiler/TagLibraryInfoImpl.java
+++ b/java/org/apache/jasper/compiler/TagLibraryInfoImpl.java
@@ -170,7 +170,8 @@ class TagLibraryInfoImpl extends TagLibraryInfo implements TagConstants {
                 // Add TLD to dependency list
                 PageInfo pageInfo = ctxt.createCompiler().getPageInfo();
                 if (pageInfo != null) {
-                    pageInfo.addDependant(tldName);
+                    pageInfo.addDependant(tldName,
+                            ctxt.getLastModified(tldName));
                 }
             } else {
                 // Tag library is packaged in JAR file

diff --git a/java/org/apache/jasper/runtime/JspSourceDependent.java b/java/org/apache/jasper/runtime/JspSourceDependent.java
index 7217a3f..3f0766d 100644 (file)
--- a/java/org/apache/jasper/runtime/JspSourceDependent.java
+++ b/java/org/apache/jasper/runtime/JspSourceDependent.java
@@ -17,7 +17,7 @@
 
 package org.apache.jasper.runtime;
 
-import java.util.List;
+import java.util.Map;
 
 /**
  * Interface for tracking the source files dependencies, for the purpose
@@ -31,9 +31,9 @@ import java.util.List;
 public interface JspSourceDependent {
 
    /**
-    * Returns a list of files names that the current page has a source
-    * dependency on.
+    * Returns a map of file names and last modified time where the current page
+    * has a source dependency on the file.
     */
-    public List<String> getDependants();
+    public Map<String,Long> getDependants();
 
 }

diff --git a/java/org/apache/jasper/servlet/JspServletWrapper.java b/java/org/apache/jasper/servlet/JspServletWrapper.java
index a7d23be..f89e82f 100644 (file)
--- a/java/org/apache/jasper/servlet/JspServletWrapper.java
+++ b/java/org/apache/jasper/servlet/JspServletWrapper.java
@@ -19,6 +19,8 @@ package org.apache.jasper.servlet;
 
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
 
 import javax.servlet.RequestDispatcher;
 import javax.servlet.Servlet;
@@ -69,6 +71,14 @@ import org.apache.tomcat.InstanceManager;
 @SuppressWarnings("deprecation") // Have to support SingleThreadModel
 public class JspServletWrapper {
 
+    private static final Map<String,Long> ALWAYS_OUTDATED_DEPENDENCIES =
+        new HashMap<String,Long>();
+
+    static {
+        // If this is missing, 
+        ALWAYS_OUTDATED_DEPENDENCIES.put("/WEB-INF/web.xml", Long.valueOf(-1));
+    }
+
     // Logger
     private final Log log = LogFactory.getLog(JspServletWrapper.class);
 
@@ -266,7 +276,7 @@ public class JspServletWrapper {
     /**
      * Get a list of files that the current page has source dependency on.
      */
-    public java.util.List<String> getDependants() {
+    public java.util.Map<String,Long> getDependants() {
         try {
             Object target;
             if (isTagFile) {
@@ -281,6 +291,10 @@ public class JspServletWrapper {
             if (target != null && target instanceof JspSourceDependent) {
                 return ((JspSourceDependent) target).getDependants();
             }
+        } catch (AbstractMethodError ame) {
+            // Almost certainly a pre Tomcat 7.0.17 compiled JSP using the old
+            // version of the interface. Force a re-compile.
+            return ALWAYS_OUTDATED_DEPENDENCIES;
         } catch (Throwable ex) {
             ExceptionUtils.handleThrowable(ex);
         }

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index aa7eb27..dc6e7f2 100644 (file)
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -107,6 +107,15 @@
         Improve the message printed by TldLocationsCache and add configuration
         example to the <code>logging.properties</code> file. (kkolinko)
       </update>
+      <fix>
+        <bug>33453</bug>: Recompile JSPs if last modified time of the source or
+        any of its dependencies changes either forwards or backwards. Note that
+        this introduces an incompatible change to the code generated for JSPs.
+        Tomcat will automatically re-compile any JSPs and tag files found in the
+        work directory when upgrading from 7.0.16 or earlier to 7.0.17 or later.
+        If you later downgrade from 7.0.17 or later to 7.0.16 or earlier, you
+        must empty the work directory as part of the downgrade process. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Cluster">