Re: svn commit: r1634161 - in /tomcat/trunk: java/org/apache/jasper/compiler/ELParser.java java/org/apache/jasper/compiler/Parser.java test/org/apache/jasper/compiler/TestParser.java
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 : 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
Re: svn commit: r1634161 - in /tomcat/trunk: java/org/apache/jasper/compiler/ELParser.java java/org/apache/jasper/compiler/Parser.java test/org/apache/jasper/compiler/TestParser.java
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 :
>>> 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 1921302return 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
Re: svn commit: r1634161 - in /tomcat/trunk: java/org/apache/jasper/compiler/ELParser.java java/org/apache/jasper/compiler/Parser.java test/org/apache/jasper/compiler/TestParser.java
On 25/10/2014 01:59, Konstantin Kolinko wrote:
> 2014-10-25 3:27 GMT+04:00 :
>> 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.
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 1921302return 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
Re: svn commit: r1634161 - in /tomcat/trunk: java/org/apache/jasper/compiler/ELParser.java java/org/apache/jasper/compiler/Parser.java test/org/apache/jasper/compiler/TestParser.java
2014-10-25 3:27 GMT+04:00 :
> 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()
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.
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.
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 1921302return 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
svn commit: r1634161 - in /tomcat/trunk: java/org/apache/jasper/compiler/ELParser.java java/org/apache/jasper/compiler/Parser.java test/org/apache/jasper/compiler/TestParser.java
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
Modified: tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java?rev=1634161&r1=1634160&r2=1634161&view=diff
==
--- tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java Fri Oct 24
23:27:40 2014
@@ -244,13 +244,15 @@ public class ELParser {
for (int i = 0; i < len; i++) {
char ch = input.charAt(i);
if (ch =='$' || (!isDeferredSyntaxAllowedAsLiteral && ch == '#')) {
-if (output == null) {
-output = new StringBuilder(len + 20);
+if (i + 1 < len && input.charAt(i + 1) == '{') {
+if (output == null) {
+output = new StringBuilder(len + 20);
+}
+output.append(input.substring(lastAppend, i));
+lastAppend = i + 1;
+output.append('\\');
+output.append(ch);
}
-output.append(input.substring(lastAppend, i));
-lastAppend = i + 1;
-output.append('\\');
-output.append(ch);
}
}
if (output == null) {
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=1634161&r1=1634160&r2=1634161&view=diff
==
--- tomcat/trunk/java/org/apache/jasper/compiler/Parser.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/Parser.java Fri Oct 24
23:27:40 2014
@@ -1283,16 +1283,9 @@ class Parser implements TagConstants {
return;
CharArrayWriter ttext = new CharArrayWriter();
-// Output the first character
-int ch = reader.nextChar();
-if (ch == '\\') {
-reader.pushChar();
-} else {
-ttext.write(ch);
-}
while (reader.hasMoreInput()) {
-ch = reader.nextChar();
+int ch = reader.nextChar();
if (ch == '<') {
// Check for "<\%"
if (reader.nextChar() == '\\') {
@@ -1302,13 +1295,21 @@ class Parser implements TagConstants {
} else {
reader.pushChar();
reader.pushChar();
-reader.pushChar();
-break;
+if (ttext.size() == 0) {
+ttext.append('<');
+} else {
+reader.pushChar();
+break;
+}
}
} else {
reader.pushChar();
-reader.pushChar();
-break;
+if (ttext.size() == 0) {
+ttext.append('<');
+} else {
+reader.pushChar();
+break;
+}
}
} else if (ch == '\\' && !pageInfo.isELIgnored()) {
int next = reader.nextChar();
@@ -1325,7 +1326,8 @@ class Parser implements TagConstants {
ttext.append('\\');
reader.pushChar();
}
-} else if ((ch == '$' || ch == '#') && !pageInfo.isELIgnored()) {
+} else if ((ch == '$' || ch == '#' &&
!pageInfo.isDeferredSyntaxAllowedAsLiteral()) &&
+!pageInfo.isELIgnored()) {
if (reader.nextChar() == '{') {
reader.pushChar();
reader.pushChar();
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=1634161&r1=1634160&r2=1634161&view=diff
==
--- tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java (original)
+++ tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java Fri Oct 24
23:27:40 2014
@@ -412,9 +412,9 @@ public class TestParser extends TomcatBa
Assert.assertTrue(result, result.contains(""));
Assert.assertTrue(result, result.contains("<04a\\?resize04a/>"));
Assert.
