Apply my patch from https://issues.apache.org/bugzilla/show_bug.cgi?id=48616#c20

This patch
- Reverts r905145,
- Provides an alternative fix for bug 48616 and bug 42390,
- Replaces Vector -> List, Hashtable -> HashMap in the affected API.

JspFragments are scriptless, so no need to declare or sync scripting
variables for fragments. Since errors in syncing the scripting variables for
JSP Fragments caused 48616 & 42390, this fixes both these bugs too.

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

diff --git a/java/org/apache/jasper/compiler/Generator.java b/java/org/apache/jasper/compiler/Generator.java
index 04945f6..238f793 100644 (file)
--- a/java/org/apache/jasper/compiler/Generator.java
+++ b/java/org/apache/jasper/compiler/Generator.java
@@ -167,26 +167,6 @@ class Generator {
         return b.toString();
     }
 
-    /**
-     * Finds the <jsp:body> subelement of the given parent node. If not
-     * found, null is returned.
-     */
-    protected static Node.JspBody findJspBody(Node parent) {
-        Node.JspBody result = null;
-
-        Node.Nodes subelements = parent.getBody();
-        for (int i = 0; (subelements != null) && (i < subelements.size()); i++) {
-            Node n = subelements.getNode(i);
-            if (n instanceof Node.JspBody) {
-                result = (Node.JspBody) n;
-                break;
-            }
-        }
-
-        return result;
-    }
-
-
     private String createJspId() {
         if (this.jspIdPrefix == null) {
             StringBuilder sb = new StringBuilder(32);
@@ -358,6 +338,9 @@ class Generator {
 
             @Override
             public void visit(Node.CustomTag n) throws JasperException {
+                // XXX - Actually there is no need to declare those
+                // "_jspx_" + varName + "_" + nestingLevel variables when we are
+                // inside a JspFragment.
 
                 if (n.getCustomNestingLevel() > 0) {
                     TagVariableInfo[] tagVarInfos = n.getTagVariableInfos();
@@ -991,6 +974,25 @@ class Generator {
             }
         }
 
+        /**
+         * Finds the &lt;jsp:body&gt; subelement of the given parent node. If not
+         * found, null is returned.
+         */
+        private Node.JspBody findJspBody(Node parent) {
+            Node.JspBody result = null;
+
+            Node.Nodes subelements = parent.getBody();
+            for (int i = 0; (subelements != null) && (i < subelements.size()); i++) {
+                Node n = subelements.getNode(i);
+                if (n instanceof Node.JspBody) {
+                    result = (Node.JspBody) n;
+                    break;
+                }
+            }
+
+            return result;
+        }
+
         @Override
         public void visit(Node.ForwardAction n) throws JasperException {
             Node.JspAttribute page = n.getPage();
@@ -2505,11 +2507,16 @@ class Generator {
         }
 
         private void declareScriptingVars(Node.CustomTag n, int scope) {
+            if (isFragment) {
+                // No need to declare Java variables, if we inside a
+                // JspFragment, because a fragment is always scriptless.
+                return;
+            }
 
-            Vector<Object> vec = n.getScriptingVars(scope);
+            List<Object> vec = n.getScriptingVars(scope);
             if (vec != null) {
                 for (int i = 0; i < vec.size(); i++) {
-                    Object elem = vec.elementAt(i);
+                    Object elem = vec.get(i);
                     if (elem instanceof VariableInfo) {
                         VariableInfo varInfo = (VariableInfo) elem;
                         if (varInfo.getDeclare()) {
@@ -2552,6 +2559,14 @@ class Generator {
             if (n.getCustomNestingLevel() == 0) {
                 return;
             }
+            if (isFragment) {
+                // No need to declare Java variables, if we inside a
+                // JspFragment, because a fragment is always scriptless.
+                // Thus, there is no need to save/ restore/ sync them.
+                // Note, that JspContextWrapper.syncFoo() methods will take
+                // care of saving/ restoring/ sync'ing of JspContext attributes.
+                return;
+            }
 
             TagVariableInfo[] tagVarInfos = n.getTagVariableInfos();
             VariableInfo[] varInfos = n.getVariableInfos();
@@ -2559,13 +2574,15 @@ class Generator {
                 return;
             }
 
+            List<Object> declaredVariables = n.getScriptingVars(scope);
+
             if (varInfos.length > 0) {
                 for (int i = 0; i < varInfos.length; i++) {
                     if (varInfos[i].getScope() != scope)
                         continue;
                     // If the scripting variable has been declared, skip codes
                     // for saving and restoring it.
-                    if (n.getScriptingVars(scope).contains(varInfos[i]))
+                    if (declaredVariables.contains(varInfos[i]))
                         continue;
                     String varName = varInfos[i].getVarName();
                     String tmpVarName = "_jspx_" + varName + "_"
@@ -2581,7 +2598,7 @@ class Generator {
                         continue;
                     // If the scripting variable has been declared, skip codes
                     // for saving and restoring it.
-                    if (n.getScriptingVars(scope).contains(tagVarInfos[i]))
+                    if (declaredVariables.contains(tagVarInfos[i]))
                         continue;
                     String varName = tagVarInfos[i].getNameGiven();
                     if (varName == null) {
@@ -2612,6 +2629,14 @@ class Generator {
             if (n.getCustomNestingLevel() == 0) {
                 return;
             }
+            if (isFragment) {
+                // No need to declare Java variables, if we inside a
+                // JspFragment, because a fragment is always scriptless.
+                // Thus, there is no need to save/ restore/ sync them.
+                // Note, that JspContextWrapper.syncFoo() methods will take
+                // care of saving/ restoring/ sync'ing of JspContext attributes.
+                return;
+            }
 
             TagVariableInfo[] tagVarInfos = n.getTagVariableInfos();
             VariableInfo[] varInfos = n.getVariableInfos();
@@ -2619,13 +2644,15 @@ class Generator {
                 return;
             }
 
+            List<Object> declaredVariables = n.getScriptingVars(scope);
+
             if (varInfos.length > 0) {
                 for (int i = 0; i < varInfos.length; i++) {
                     if (varInfos[i].getScope() != scope)
                         continue;
                     // If the scripting variable has been declared, skip codes
                     // for saving and restoring it.
-                    if (n.getScriptingVars(scope).contains(varInfos[i]))
+                    if (declaredVariables.contains(varInfos[i]))
                         continue;
                     String varName = varInfos[i].getVarName();
                     String tmpVarName = "_jspx_" + varName + "_"
@@ -2641,7 +2668,7 @@ class Generator {
                         continue;
                     // If the scripting variable has been declared, skip codes
                     // for saving and restoring it.
-                    if (n.getScriptingVars(scope).contains(tagVarInfos[i]))
+                    if (declaredVariables.contains(tagVarInfos[i]))
                         continue;
                     String varName = tagVarInfos[i].getNameGiven();
                     if (varName == null) {
@@ -2666,6 +2693,15 @@ class Generator {
          * given scope.
          */
         private void syncScriptingVars(Node.CustomTag n, int scope) {
+            if (isFragment) {
+                // No need to declare Java variables, if we inside a
+                // JspFragment, because a fragment is always scriptless.
+                // Thus, there is no need to save/ restore/ sync them.
+                // Note, that JspContextWrapper.syncFoo() methods will take
+                // care of saving/ restoring/ sync'ing of JspContext attributes.
+                return;
+            }
+
             TagVariableInfo[] tagVarInfos = n.getTagVariableInfos();
             VariableInfo[] varInfos = n.getVariableInfos();
 

diff --git a/java/org/apache/jasper/compiler/Node.java b/java/org/apache/jasper/compiler/Node.java
index 7633540..1e4993b 100644 (file)
--- a/java/org/apache/jasper/compiler/Node.java
+++ b/java/org/apache/jasper/compiler/Node.java
@@ -1433,11 +1433,11 @@ abstract class Node implements TagConstants {
 
         private boolean implementsDynamicAttributes;
 
-        private Vector<Object> atBeginScriptingVars;
+        private List<Object> atBeginScriptingVars;
 
-        private Vector<Object> atEndScriptingVars;
+        private List<Object> atEndScriptingVars;
 
-        private Vector<Object> nestedScriptingVars;
+        private List<Object> nestedScriptingVars;
 
         private Node.CustomTag customTagParent;
 
@@ -1657,7 +1657,7 @@ abstract class Node implements TagConstants {
             return this.numCount;
         }
 
-        public void setScriptingVars(Vector<Object> vec, int scope) {
+        public void setScriptingVars(List<Object> vec, int scope) {
             switch (scope) {
             case VariableInfo.AT_BEGIN:
                 this.atBeginScriptingVars = vec;
@@ -1675,8 +1675,8 @@ abstract class Node implements TagConstants {
          * Gets the scripting variables for the given scope that need to be
          * declared.
          */
-        public Vector<Object> getScriptingVars(int scope) {
-            Vector<Object> vec = null;
+        public List<Object> getScriptingVars(int scope) {
+            List<Object> vec = null;
 
             switch (scope) {
             case VariableInfo.AT_BEGIN:

diff --git a/java/org/apache/jasper/compiler/ScriptingVariabler.java b/java/org/apache/jasper/compiler/ScriptingVariabler.java
index 7996237..6a1048a 100644 (file)
--- a/java/org/apache/jasper/compiler/ScriptingVariabler.java
+++ b/java/org/apache/jasper/compiler/ScriptingVariabler.java
@@ -59,11 +59,11 @@ class ScriptingVariabler {
     static class ScriptingVariableVisitor extends Node.Visitor {
 
         private ErrorDispatcher err;
-        private Hashtable<String,Integer> scriptVars;
-        
+        private Map<String, Integer> scriptVars;
+
         public ScriptingVariableVisitor(ErrorDispatcher err) {
             this.err = err;
-            scriptVars = new Hashtable<String,Integer>();
+            scriptVars = new HashMap<String,Integer>();
         }
 
         @Override
@@ -83,7 +83,7 @@ class ScriptingVariabler {
                 return;
             }
 
-            Vector<Object> vec = new Vector<Object>();
+            List<Object> vec = new ArrayList<Object>();
 
             Integer ownRange = null;
             Node.CustomTag parent = n.getCustomTagParent();
@@ -107,11 +107,8 @@ class ScriptingVariabler {
                     String varName = varInfos[i].getVarName();
                     
                     Integer currentRange = scriptVars.get(varName);
-                    // If a fragment helper has been used for the parent tag
-                    // the scripting variables always need to be declared
                     if (currentRange == null ||
-                            ownRange.compareTo(currentRange) > 0 ||
-                            parent != null && isImplementedAsFragment(parent)) {
+                            ownRange.compareTo(currentRange) > 0) {
                         scriptVars.put(varName, ownRange);
                         vec.add(varInfos[i]);
                     }
@@ -134,11 +131,8 @@ class ScriptingVariabler {
                     }
 
                     Integer currentRange = scriptVars.get(varName);
-                    // If a fragment helper has been used for the parent tag
-                    // the scripting variables always need to be declared
                     if (currentRange == null ||
-                            ownRange.compareTo(currentRange) > 0 ||
-                            parent != null && isImplementedAsFragment(parent)) {
+                            ownRange.compareTo(currentRange) > 0) {
                         scriptVars.put(varName, ownRange);
                         vec.add(tagVarInfos[i]);
                     }
@@ -149,22 +143,6 @@ class ScriptingVariabler {
         }
     }
 
-    private static boolean isImplementedAsFragment(Node.CustomTag n) {
-        // Replicates logic from Generator to determine if a fragment
-        // helper will be used
-        if (n.implementsSimpleTag()) {
-            if (Generator.findJspBody(n) == null) {
-                if (!n.hasEmptyBody()) {
-                    return true;
-                }
-                return false;
-            }
-            return true;
-        }
-        return false;
-    }
-
-    
     public static void set(Node.Nodes page, ErrorDispatcher err)
             throws JasperException {
         page.visit(new CustomTagCounter());