On 26/10/2014 08:50, Mark Thomas wrote: > 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.
Should be fixed in r1634741 >>> 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. Should be fixed in 1634740 >>> 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. Done in multiple commits (so it is easier to follow the justification for why the code can be removed) r1634697-r1634724. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
