tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject Re: svn commit: r1214855 - in /tomcat/trunk: java/org/apache/jasper/compiler/Parser.java test/org/apache/jasper/compiler/TestParser.java test/webapp-3.0/bug52335.jsp
Date Thu, 15 Dec 2011 23:22:17 GMT
Konstantin Kolinko <knst.kolinko@gmail.com> wrote:

>1. The test & fix is wrong.
><\%  should print <%
>
>2. Note " // Output the first character" comment in
>parseTemplateText().
>That is special handling of the first character.
>
>My understanding of the code before this change is that
>the loop in parseTemplateText() always breaks when '<' is encountered.
>
>I think that when that '<' is not start of a tag and similar, then
>parseTemplateText() is just called again.
>On this second call the '<' will be the first character and that is
>why it is handled specially before the  loop.
>(That is just from code review. I have not tried to debug it yet).
>
>Best regards,
>Konstantin Kolinko
>
>
>2011/12/15  <markt@apache.org>:
>> Author: markt
>> Date: Thu Dec 15 16:50:25 2011
>> New Revision: 1214855
>>
>> URL: http://svn.apache.org/viewvc?rev=1214855&view=rev
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=52335
>> % is only escaped as <\%, not \%
>>
>> Added:
>>    tomcat/trunk/test/webapp-3.0/bug52335.jsp
>> Modified:
>>    tomcat/trunk/java/org/apache/jasper/compiler/Parser.java
>>    tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java
>>
>> Modified: tomcat/trunk/java/org/apache/jasper/compiler/Parser.java
>> URL:
>http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Parser.java?rev=1214855&r1=1214854&r2=1214855&view=diff
>>
>==============================================================================
>> --- tomcat/trunk/java/org/apache/jasper/compiler/Parser.java
>(original)
>> +++ tomcat/trunk/java/org/apache/jasper/compiler/Parser.java Thu Dec
>15 16:50:25 2011
>> @@ -1278,7 +1278,7 @@ class Parser implements TagConstants {
>>
>>     /*
>>      * Parse for a template text string until '<' or "${" or "#{" is
>encountered,
>> -     * recognizing escape sequences "\%", "\$", and "\#".
>> +     * recognizing escape sequences "<\%", "\$", and "\#".
>>      */
>>     private void parseTemplateText(Node parent) throws
>JasperException {
>>
>> @@ -1297,8 +1297,33 @@ class Parser implements TagConstants {
>>         while (reader.hasMoreInput()) {
>>             ch = reader.nextChar();
>>             if (ch == '<') {
>> -                reader.pushChar();
>> -                break;
>> +                // Check for <\%
>> +                ch = reader.nextChar();
>> +                if (ch == -1) {
>> +                    reader.pushChar();
>> +                    break;
>> +                } else if (ch == '\\') {
>> +                    ch = reader.nextChar();
>> +                    if (ch == -1) {
>> +                        reader.pushChar();
>> +                        reader.pushChar();
>> +                        break;
>> +                    } else if (ch == '%') {
>> +                        ttext.write('<');
>> +                        ttext.write('\\');
>> +                        ttext.write('%');
>> +                        continue;
>> +                    } else {
>> +                        reader.pushChar();
>> +                        reader.pushChar();
>> +                        reader.pushChar();
>> +                        break;
>> +                    }
>> +                } else {
>> +                    reader.pushChar();
>> +                    reader.pushChar();
>> +                    break;
>> +                }
>>             } else if ((ch == '$' || ch == '#') &&
>!pageInfo.isELIgnored()) {
>>                 if (!reader.hasMoreInput()) {
>>                     ttext.write(ch);
>> @@ -1318,9 +1343,9 @@ class Parser implements TagConstants {
>>                     break;
>>                 }
>>                 char next = (char) reader.peekChar();
>> -                // Looking for \% or \$ or \#
>> -                if (next == '%' || ((next == '$' || next == '#') &&
>> -                        !pageInfo.isELIgnored())) {
>> +                // Looking for \$ or \# when EL is being used
>> +                if ((next == '$' || next == '#') &&
>> +                        !pageInfo.isELIgnored()) {
>>                     ch = reader.nextChar();
>>                 }
>>             }
>>
>> Modified:
>tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java
>> URL:
>http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java?rev=1214855&r1=1214854&r2=1214855&view=diff
>>
>==============================================================================
>> --- tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java
>(original)
>> +++ tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java Thu
>Dec 15 16:50:25 2011
>> @@ -290,6 +290,26 @@ public class TestParser extends TomcatBa
>>         assertEcho(res.toString(), "OK");
>>     }
>>
>> +    @Test
>> +    public void testBug52335() throws Exception {
>> +        Tomcat tomcat = getTomcatInstance();
>> +
>> +        File appDir =
>> +            new File("test/webapp-3.0");
>> +        // app dir is relative to server home
>> +        tomcat.addWebapp(null, "/test", appDir.getAbsolutePath());
>> +
>> +        tomcat.start();
>> +
>> +        ByteChunk res = getUrl("http://localhost:" + getPort() +
>> +                "/test/bug52335.jsp");
>> +
>> +        String result = res.toString();
>> +        // Beware of the differences between escaping in JSP
>attributes and
>> +        // in Java Strings
>> +        assertEcho(result, "00 - \\% \\\\% <\\%");
>> +    }
>> +
>>     /** Assertion for text printed by tags:echo */
>>     private static void assertEcho(String result, String expected) {
>>         assertTrue(result.indexOf("<p>" + expected + "</p>") >
0);
>>
>> Added: tomcat/trunk/test/webapp-3.0/bug52335.jsp
>> URL:
>http://svn.apache.org/viewvc/tomcat/trunk/test/webapp-3.0/bug52335.jsp?rev=1214855&view=auto
>>
>==============================================================================
>> --- tomcat/trunk/test/webapp-3.0/bug52335.jsp (added)
>> +++ tomcat/trunk/test/webapp-3.0/bug52335.jsp Thu Dec 15 16:50:25
>2011
>> @@ -0,0 +1,24 @@
>> +<%--
>> + Licensed to the Apache Software Foundation (ASF) under one or more
>> +  contributor license agreements.  See the NOTICE file distributed
>with
>> +  this work for additional information regarding copyright
>ownership.
>> +  The ASF licenses this file to You under the Apache License,
>Version 2.0
>> +  (the "License"); you may not use this file except in compliance
>with
>> +  the License.  You may obtain a copy of the License at
>> +
>> +      http://www.apache.org/licenses/LICENSE-2.0
>> +
>> +  Unless required by applicable law or agreed to in writing,
>software
>> +  distributed under the License is distributed on an "AS IS" BASIS,
>> +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>implied.
>> +  See the License for the specific language governing permissions
>and
>> +  limitations under the License.
>> +--%>
>> +<%@ page isELIgnored="true" %>
>> +<html>
>> +  <head><title>Bug 52335 test case</title></head>
>> +  <body>
>> +    <p>00 - \% \\% <\%</p>
>> +  </body>
>> +</html>
>>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>For additional commands, e-mail: dev-help@tomcat.apache.org

Yep, brain not fully engaged when I made that change. I'll try and look at this again tomorrow.
Feel free to revert my "fix" in the meantime.

Mark

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


Mime
View raw message