On 28/06/2011 11:38, Konstantin Kolinko wrote:
> 2011/6/28 Mark Thomas <[email protected]>:
>> On 28/06/2011 01:08, [email protected] wrote:
>>> Modified: tomcat/tc6.0.x/trunk/STATUS.txt
>>> URL:
>>> http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1140383&r1=1140382&r2=1140383&view=diff
>>>
>>> http://svn.apache.org/viewvc?rev=1138950&view=rev
>>> http://svn.apache.org/viewvc?rev=1138953&view=rev
>>> +1: markt, schultz
>>> - -1:
>>> + -1: kkolinko: 1) It would be OK if it were mangling only reserved words
>>> + (where the tag wouldn't compile previously, so there were no
>>> regressions),
>>> + but JspUtils.makeJavaIdentifier() mangles '_' character, which will
>>> break
>>> + code that was previously working. E.g. <%=hello_world%>
>>> + 2) Maybe it would be better to prefix the names with [_]jsp_ and use
>>> some
>>> + numbered suffix, to never collide with names that people may use in
>>> their
>>> + code.
>>
>> I don't see anything in the JSP specification that says tag file
>> attributes must be exposed as Java variables with the same name. Given
>> that a tag file attribute can have any name valid in XML, that
>> requirement would be impossible to meet since may of those names would
>> be invalid as Java identifiers.
>>
>> There is a requirement to expose attributes (with the same name) as page
>> scoped variables. If I switch the test case to use ${hello_world} rather
>> than <%=hello_world%> it works. However, that causes it's own set of
>> problems when a Java keyword is used as an attribute name due to the
>> restrictions of the EL specification.
>>
>> AFAICT, the only solution that is guaranteed (by the specification) to
>> work for any attribute name in a tag file is:
>> ${pageScope['attributename']}
>> Unless I have missed something in the specification (always a
>> possibility) that anything else works is a fortunate side-effect of the
>> implementation.
>>
>
> Interesting. Though I used this feature a lot, since TC 5.5.
>
> Eclipse IDE used to hilite these usages in red several years ago, but
> nowadays it has support for such usage.
>
>> On this basis, I think the new behaviour is compliant with the
>> specification.
>
> There is one more problem: you cannot introduce new variables that
> will be seen by user's code, unless those are prefixed with reserved
> prefix of "jsp_" ( "_jsp_") per JSP.9.1.2. That is because someone can
> declare a local variable with the same name.
JSP.9.1.2 certainly implies that but enforcing that would break the
<%=attributeName%> usage. I'm happy not worrying about that for now.
>> That said, I take the point regarding regressions. I'm sure lots of
>> folks will have used <%=attributeName%> or ${attributeName} as a
>> shortcut although I don't currently see anything to support that usage
>> in the JSP specification.
I was thinking along these lines:
Index: java/org/apache/jasper/compiler/JspUtil.java
===================================================================
--- java/org/apache/jasper/compiler/JspUtil.java (revision 1140509)
+++ java/org/apache/jasper/compiler/JspUtil.java (working copy)
@@ -810,10 +810,8 @@
}
for (int i = 0; i < identifier.length(); i++) {
char ch = identifier.charAt(i);
- if (Character.isJavaIdentifierPart(ch) && ch != '_') {
+ if (Character.isJavaIdentifierPart(ch)) {
modifiedIdentifier.append(ch);
- } else if (ch == '.') {
- modifiedIdentifier.append('_');
} else {
modifiedIdentifier.append(mangleChar(ch));
}
Essentially, no longer treat '.' as a special case. That removes the
needs to escape _ which should prevent any regressions. However, I do
wonder why '.' was treated as a special case in the first place. Time to
do some testing...
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
