On 26/10/2014 08:20, Mark Thomas wrote: > On 25/10/2014 01:59, Konstantin Kolinko wrote: >> 2014-10-25 3:27 GMT+04:00 <ma...@apache.org>: >>> Author: markt >>> Date: Fri Oct 24 23:27:40 2014 >>> New Revision: 1634161 >>> >>> URL: http://svn.apache.org/r1634161 >>> Log: >>> Follow up to r1634089. >>> Fix some additional test failures with the stricter escaping rules. >>> >>> Modified: >>> tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java >>> tomcat/trunk/java/org/apache/jasper/compiler/Parser.java >>> tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java >> >> 1) The patches covered Parser.parseTemplateText(). >> >> There is also Parser.parseXMLTemplateText() > > ACK. > >> 2) In Parser: >> >> reader.nextChar() can return -1 in case when hasMoreInput() is false. >> >> In that case nothing has been read and "reader.pushChar();" should not >> be called. >> >> There exists reader.peekChar() method in JspReader, but only for one >> character ahead. > > ACK. > >> 3) In Parser and JspReader that it uses: There is a caveat with >> reader.hasMoreInput(): it modifies the current reader state due to >> the call to popFile(). >> >> If popFile() call happened during the hasMoreInput() call, then >> reader.pushChar() won't be possible anymore. >> >> Is it possible to call pushChar() after end-of-data has been reached? >> - I do not see the answer from popFile() code. It needs testing. >> >> I looked where this pushing/popping comes from, but the only caller to >> JspReader.pushFile() is JspReader constructor. I think that actually >> there is always no more than a single file. > > In that case I'd be happy to ditch that code, the sooner the better. > Anything to make Jasper less complex would be a good thing. > > I'll so some svn archaeology and see if I can find a time when that > feature was used and what it was used for.
Tomcat 3.x, for including files: http://svn.apache.org/repos/asf/tomcat/archive/jasper/tags/other/tomcat_33_m4/jasper34/generator/org/apache/jasper34/generator/JspParseEventListener.java and http://svn.apache.org/repos/asf/tomcat/archive/jasper/tags/other/tomcat_33_m4/jasper34/generator/org/apache/jasper34/parser/JspReader.java Elements of it were kept when Jasper was re-written as Jasper2 for Tomcat 4 (and then renamed back to Jasper). As fas as I can tell the pushFile code was never used in Jasper 2 but it has been there since Tomcat 4. Japser(2) handles included files differently. I think it is safe to remove pushFile and all the associated code. Mark > > Mark > > >> >> There exists "JspReader.setSingleFile()" setter method that turns off >> multiple files mode. The setter is called in two places, but I have >> not yet figured why and why 'single mode' flag is not always true. >> >> Coverage report: [1] >> >> 158 boolean hasMoreInput() throws JasperException { >> 159 1930260 if (current.cursor >= current.stream.length) { >> 160 8958 if (singleFile) return false; >> 161 2040 while (popFile()) { >> 162 0 if (current.cursor < current.stream.length) return >> true; >> 163 } >> 164 2040 return false; >> 165 } >> 166 1921302 return true; >> 167 } >> >> The popFile() line has "Conditional coverage 50%". The popFile() >> method was called, but always returned false. >> >> [1] >> http://ci.apache.org/projects/tomcat/tomcat8/coverage/org.apache.jasper.compiler.JspReader.html >> >> Best regards, >> Konstantin Kolinko >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org >> For additional commands, e-mail: dev-h...@tomcat.apache.org >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
