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 2009/07/05 13:30:22 UTC
svn commit: r791224 - in /tomcat/trunk/java/org/apache/jasper/compiler:
Generator.java PageInfo.java
Author: markt
Date: Sun Jul 5 11:30:22 2009
New Revision: 791224
URL: http://svn.apache.org/viewvc?rev=791224&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=38797
Revert previous fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=37933 and implement an alternative that doesn't have the side effects described in 38797
Modified:
tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
tomcat/trunk/java/org/apache/jasper/compiler/PageInfo.java
Modified: tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Generator.java?rev=791224&r1=791223&r2=791224&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/Generator.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/Generator.java Sun Jul 5 11:30:22 2009
@@ -31,6 +31,7 @@
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
+import java.util.Set;
import java.util.Vector;
import javax.el.MethodExpression;
@@ -86,6 +87,8 @@
private ErrorDispatcher err;
private BeanRepository beanInfo;
+
+ private Set<String> varInfoNames;
private JspCompilationContext ctxt;
@@ -1107,18 +1110,26 @@
+ ")_jspx_page_context.findAttribute("
+ "\""
+ name + "\"))." + methodName + "())));");
- } else {
- // The object could be a custom action with an associated
+ } else if (varInfoNames.contains(name)) {
+ // The object is a custom action with an associated
// VariableInfo entry for this name.
// Get the class name and then introspect at runtime.
out
.printil("out.write(org.apache.jasper.runtime.JspRuntimeLibrary.toString"
+ "(org.apache.jasper.runtime.JspRuntimeLibrary.handleGetProperty"
- + "(_jspx_page_context.getAttribute(\""
+ + "(_jspx_page_context.findAttribute(\""
+ name
- + "\", PageContext.PAGE_SCOPE), \""
+ + "\"), \""
+ property
+ "\")));");
+ } else {
+ StringBuilder msg =
+ new StringBuilder("jsp:getProperty for bean with name '");
+ msg.append(name);
+ msg.append(
+ "'. Name was not previously introduced as per JSP.5.3");
+
+ throw new JasperException(msg.toString());
}
n.setEndJavaLine(out.getJavaLine());
@@ -1782,6 +1793,18 @@
// restore previous writer
out = outSave;
}
+
+ // Add the named objects to the lits of 'introduced' names to enable
+ // a later test as per JSP.5.3
+ VariableInfo[] infos = n.getVariableInfos();
+ if (infos != null && infos.length > 0) {
+ for (int i = 0; i < infos.length; i++) {
+ VariableInfo info = infos[i];
+ if (info != null && info.getVarName() != null)
+ pageInfo.getVarInfoNames().add(info.getVarName());
+ }
+ }
+
}
private static final String DOUBLE_QUOTE = "\\\"";
@@ -3364,6 +3387,7 @@
isPoolingEnabled = false;
}
beanInfo = pageInfo.getBeanRepository();
+ varInfoNames = pageInfo.getVarInfoNames();
breakAtLF = ctxt.getOptions().getMappedFile();
if (isPoolingEnabled) {
tagHandlerPoolNames = new Vector<String>();
Modified: tomcat/trunk/java/org/apache/jasper/compiler/PageInfo.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/PageInfo.java?rev=791224&r1=791223&r2=791224&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/PageInfo.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/PageInfo.java Sun Jul 5 11:30:22 2009
@@ -21,6 +21,7 @@
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
+import java.util.Set;
import java.util.Vector;
import org.apache.el.ExpressionFactoryImpl;
@@ -42,6 +43,7 @@
private Vector<String> dependants;
private BeanRepository beanRepository;
+ private Set<String> varInfoNames;
private HashMap<String,TagLibraryInfo> taglibsMap;
private HashMap<String, String> jspPrefixMapper;
private HashMap<String, LinkedList<String>> xmlPrefixMapper;
@@ -98,6 +100,7 @@
this.jspFile = jspFile;
this.beanRepository = beanRepository;
+ this.varInfoNames = new HashSet<String>();
this.taglibsMap = new HashMap<String, TagLibraryInfo>();
this.jspPrefixMapper = new HashMap<String, String>();
this.xmlPrefixMapper = new HashMap<String, LinkedList<String>>();
@@ -707,4 +710,8 @@
public void setTrimDirectiveWhitespaces(boolean trimDirectiveWhitespaces) {
this.trimDirectiveWhitespaces = trimDirectiveWhitespaces;
}
+
+ public Set<String> getVarInfoNames() {
+ return varInfoNames;
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r791224 - in /tomcat/trunk/java/org/apache/jasper/compiler:
Generator.java PageInfo.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2009/7/5 Mark Thomas <ma...@apache.org>:
> Konstantin Kolinko wrote:
>> 2009/7/5 <ma...@apache.org>:
>>> Author: markt
>>> Date: Sun Jul 5 11:30:22 2009
>>> New Revision: 791224
>>>
>>> URL: http://svn.apache.org/viewvc?rev=791224&view=rev
>>> Log:
>>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=38797
>>> Revert previous fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=37933 and implement an alternative that doesn't have the side effects described in 38797
>>>
>>> Modified:
>>> tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
>>> tomcat/trunk/java/org/apache/jasper/compiler/PageInfo.java
>>>
>>> --- tomcat/trunk/java/org/apache/jasper/compiler/Generator.java (original)
>>> +++ tomcat/trunk/java/org/apache/jasper/compiler/Generator.java Sun Jul 5 11:30:22 2009
>>> ...
>>> + // Add the named objects to the lits of 'introduced' names to enable
>>> + // a later test as per JSP.5.3
>>> + VariableInfo[] infos = n.getVariableInfos();
>>> + if (infos != null && infos.length > 0) {
>>> + for (int i = 0; i < infos.length; i++) {
>>> + VariableInfo info = infos[i];
>>> + if (info != null && info.getVarName() != null)
>>> + pageInfo.getVarInfoNames().add(info.getVarName());
>>> + }
>>> + }
>>
>> I do not think that the above fragment is right.
>>
>> I have not tested it, but it looks like
>> 1. It does not take VariableInfo#scope into account
> What part of JSP.5.3 makes you think that it should?
>
>> 2. I think that it does not work for tag files that declare variables.
> Again, what part of JSP.5.3 makes you think that it should?
>
OK.
I just thought that it is strange, that only VariableInfo is
mentioned, and it is not the only way to introduce variables (there it
says "object").
but now looking at JSP 1.2 spec, the problem was already there: it
explicitly mentions VariableInfo, but not mentions TagVariableInfo.
Also, it is even more restrictive than 2.0 and 2.1 versions: it does
not mention that ignoring this error is allowed.
So, let's do it as written, verbatim. - as you did.
>> Also,
>> 3. Node.UseBean node does not add anything to that set of variable names.
> It doesn't need to. That is what the BeanRepository instance is for.
>
Oh, I see that now.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r791224 - in /tomcat/trunk/java/org/apache/jasper/compiler:
Generator.java PageInfo.java
Posted by Mark Thomas <ma...@apache.org>.
Konstantin Kolinko wrote:
> 2009/7/5 <ma...@apache.org>:
>> Author: markt
>> Date: Sun Jul 5 11:30:22 2009
>> New Revision: 791224
>>
>> URL: http://svn.apache.org/viewvc?rev=791224&view=rev
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=38797
>> Revert previous fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=37933 and implement an alternative that doesn't have the side effects described in 38797
>>
>> Modified:
>> tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
>> tomcat/trunk/java/org/apache/jasper/compiler/PageInfo.java
>>
>> --- tomcat/trunk/java/org/apache/jasper/compiler/Generator.java (original)
>> +++ tomcat/trunk/java/org/apache/jasper/compiler/Generator.java Sun Jul 5 11:30:22 2009
>> ...
>> + // Add the named objects to the lits of 'introduced' names to enable
>> + // a later test as per JSP.5.3
>> + VariableInfo[] infos = n.getVariableInfos();
>> + if (infos != null && infos.length > 0) {
>> + for (int i = 0; i < infos.length; i++) {
>> + VariableInfo info = infos[i];
>> + if (info != null && info.getVarName() != null)
>> + pageInfo.getVarInfoNames().add(info.getVarName());
>> + }
>> + }
>
> I do not think that the above fragment is right.
>
> I have not tested it, but it looks like
> 1. It does not take VariableInfo#scope into account
What part of JSP.5.3 makes you think that it should?
> 2. I think that it does not work for tag files that declare variables.
Again, what part of JSP.5.3 makes you think that it should?
> Also,
> 3. Node.UseBean node does not add anything to that set of variable names.
It doesn't need to. That is what the BeanRepository instance is for.
As an aside, the TCK passes with this patch applied. Then again, that
doesn't always mean much as it passes without the patch as well.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r791224 - in /tomcat/trunk/java/org/apache/jasper/compiler:
Generator.java PageInfo.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2009/7/5 <ma...@apache.org>:
> Author: markt
> Date: Sun Jul 5 11:30:22 2009
> New Revision: 791224
>
> URL: http://svn.apache.org/viewvc?rev=791224&view=rev
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=38797
> Revert previous fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=37933 and implement an alternative that doesn't have the side effects described in 38797
>
> Modified:
> tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
> tomcat/trunk/java/org/apache/jasper/compiler/PageInfo.java
>
> --- tomcat/trunk/java/org/apache/jasper/compiler/Generator.java (original)
> +++ tomcat/trunk/java/org/apache/jasper/compiler/Generator.java Sun Jul 5 11:30:22 2009
>...
> + // Add the named objects to the lits of 'introduced' names to enable
> + // a later test as per JSP.5.3
> + VariableInfo[] infos = n.getVariableInfos();
> + if (infos != null && infos.length > 0) {
> + for (int i = 0; i < infos.length; i++) {
> + VariableInfo info = infos[i];
> + if (info != null && info.getVarName() != null)
> + pageInfo.getVarInfoNames().add(info.getVarName());
> + }
> + }
I do not think that the above fragment is right.
I have not tested it, but it looks like
1. It does not take VariableInfo#scope into account
2. I think that it does not work for tag files that declare variables.
Also,
3. Node.UseBean node does not add anything to that set of variable names.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org