Konstantin Kolinko <knst.koli...@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 <ma...@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-unsubscr...@tomcat.apache.org >For additional commands, e-mail: dev-h...@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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
