You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2010/02/18 11:27:34 UTC

svn commit: r911310 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/jasper/compiler/Generator.java java/org/apache/jasper/compiler/ScriptingVariabler.java webapps/docs/changelog.xml

Author: markt
Date: Thu Feb 18 10:27:33 2010
New Revision: 911310

URL: http://svn.apache.org/viewvc?rev=911310&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48616
This is a regression caused by the fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=42390
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. (kkolinko)

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java
    tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ScriptingVariabler.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=911310&r1=911309&r2=911310&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Thu Feb 18 10:27:33 2010
@@ -143,18 +143,6 @@
   +1: markt, kkolinko
   -1: 
 
-* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48616
-  This is a regression caused by the fix for
-  https://issues.apache.org/bugzilla/show_bug.cgi?id=42390
-  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.
-  https://issues.apache.org/bugzilla/show_bug.cgi?id=48616#c21
-  (https://issues.apache.org/bugzilla/attachment.cgi?id=24992)
-  +1: kkolinko, markt, mturk
-  -1:
-
-
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48612
   Prevent exception on shutdown
   Port of r896193 and r905343

Modified: tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java?rev=911310&r1=911309&r2=911310&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/Generator.java Thu Feb 18 10:27:33 2010
@@ -333,6 +333,9 @@
             }
 
             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();
@@ -2484,6 +2487,11 @@
         }
 
         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 vec = n.getScriptingVars(scope);
             if (vec != null) {
@@ -2531,6 +2539,14 @@
             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();
@@ -2591,6 +2607,14 @@
             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();
@@ -2645,6 +2669,15 @@
          * 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();
 

Modified: tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ScriptingVariabler.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ScriptingVariabler.java?rev=911310&r1=911309&r2=911310&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ScriptingVariabler.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/jasper/compiler/ScriptingVariabler.java Thu Feb 18 10:27:33 2010
@@ -58,17 +58,17 @@
     static class ScriptingVariableVisitor extends Node.Visitor {
 
 	private ErrorDispatcher err;
-	private Hashtable scriptVars;
+	private Map<String, Integer> scriptVars;
 	
 	public ScriptingVariableVisitor(ErrorDispatcher err) {
 	    this.err = err;
-	    scriptVars = new Hashtable();
+	    scriptVars = new HashMap<String, Integer>();
 	}
 
 	public void visit(Node.CustomTag n) throws JasperException {
 	    setScriptingVars(n, VariableInfo.AT_BEGIN);
 	    setScriptingVars(n, VariableInfo.NESTED);
-	    new ScriptingVariableVisitor(err).visitBody(n);
+	    visitBody(n);
 	    setScriptingVars(n, VariableInfo.AT_END);
 	}
 
@@ -104,7 +104,7 @@
 		    }
 		    String varName = varInfos[i].getVarName();
 		    
-		    Integer currentRange = (Integer) scriptVars.get(varName);
+		    Integer currentRange = scriptVars.get(varName);
 		    if (currentRange == null
 			    || ownRange.compareTo(currentRange) > 0) {
 			scriptVars.put(varName, ownRange);
@@ -127,7 +127,7 @@
 			}
 		    }
 
-		    Integer currentRange = (Integer) scriptVars.get(varName);
+		    Integer currentRange = scriptVars.get(varName);
 		    if (currentRange == null
 			    || ownRange.compareTo(currentRange) > 0) {
 			scriptVars.put(varName, ownRange);

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=911310&r1=911309&r2=911310&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Thu Feb 18 10:27:33 2010
@@ -57,6 +57,12 @@
         Add some debug logging to the compiler where exceptions were previously
         swallowed. (markt)
       </add>
+      <fix>
+        <bug>48616</bug>: Don't declare or syncrhonize scripting variables for
+        JSP fragments since they are scriptless. This is an alternative fix for
+        <bug>42390</bug> that avoids both the original problem and the
+        regression in the first fix. (kkolinko) 
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Cluster">



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org