You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2014/04/12 22:07:54 UTC

svn commit: r1586890 - in /tomcat/trunk: java/org/apache/jasper/compiler/ELParser.java test/org/apache/jasper/compiler/TestELParser.java test/org/apache/jasper/compiler/TestParser.java test/webapp/bug5nnnn/bug56334.jspx webapps/docs/changelog.xml

Author: markt
Date: Sat Apr 12 20:07:54 2014
New Revision: 1586890

URL: http://svn.apache.org/r1586890
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56334
Correct double backslash escaping in attributes

Added:
    tomcat/trunk/test/webapp/bug5nnnn/bug56334.jspx
Modified:
    tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java
    tomcat/trunk/test/org/apache/jasper/compiler/TestELParser.java
    tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java
    tomcat/trunk/webapps/docs/changelog.xml

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=1586890&r1=1586889&r2=1586890&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java Sat Apr 12 20:07:54 2014
@@ -209,7 +209,7 @@ public class ELParser {
                 prev = 0;
                 if (ch == '\\') {
                     buf.append('\\');
-                    prev = '\\';
+                    continue;
                 } else if (ch == '$'
                         || (!isDeferredSyntaxAllowedAsLiteral && ch == '#')) {
                     buf.append(ch);
@@ -468,18 +468,18 @@ public class ELParser {
 
         @Override
         public void visit(Function n) throws JasperException {
-            output.append(n.getOriginalText());
+            output.append(Generator.escape(n.getOriginalText()));
             output.append('(');
         }
 
         @Override
         public void visit(Text n) throws JasperException {
-            output.append(n.getText());
+            output.append(Generator.escape(n.getText()));
         }
 
         @Override
         public void visit(ELText n) throws JasperException {
-            output.append(n.getText());
+            output.append(Generator.escape(n.getText()));
         }
     }
 }

Modified: tomcat/trunk/test/org/apache/jasper/compiler/TestELParser.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/jasper/compiler/TestELParser.java?rev=1586890&r1=1586889&r2=1586890&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/jasper/compiler/TestELParser.java (original)
+++ tomcat/trunk/test/org/apache/jasper/compiler/TestELParser.java Sat Apr 12 20:07:54 2014
@@ -170,6 +170,18 @@ public class TestELParser {
     }
 
 
+    @Test
+    public void testEscape01() throws JasperException {
+        doTestParser("${'\\\\'}");
+    }
+
+
+    @Test
+    public void testEscape02() throws JasperException {
+        doTestParser("\\\\x${'\\\\'}");
+    }
+
+
     private void doTestParser(String input) throws JasperException {
         Nodes nodes = ELParser.parse(input, false);
 

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=1586890&r1=1586889&r2=1586890&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java (original)
+++ tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java Sat Apr 12 20:07:54 2014
@@ -380,6 +380,40 @@ public class TestParser extends TomcatBa
                 result.contains("[4: [data-test]: [window.alert('Hello 'World <&>'!')]]"));
     }
 
+    @Test
+    public void testBug56334() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        File appDir = new File("test/webapp");
+        // app dir is relative to server home
+        StandardContext ctxt = (StandardContext) tomcat.addWebapp(null,
+                "/test", appDir.getAbsolutePath());
+
+        // This test needs the JSTL libraries
+        File lib = new File("webapps/examples/WEB-INF/lib");
+        ctxt.setResources(new StandardRoot(ctxt));
+        ctxt.getResources().createWebResourceSet(
+                WebResourceRoot.ResourceSetType.POST, "/WEB-INF/lib",
+                lib.getAbsolutePath(), null, "/");
+
+        tomcat.start();
+
+        ByteChunk res = getUrl("http://localhost:" + getPort() +
+                "/test/bug5nnnn/bug56334.jspx");
+
+        String result = res.toString();
+
+        System.out.println(result);
+
+        // NOTE: The expected values must themselves be \ escaped below
+        Assert.assertTrue(result, result.contains("\\?resize01"));
+        Assert.assertTrue(result, result.contains("<set data-value=\"\\\\?resize02a\"/>"));
+        Assert.assertTrue(result, result.contains("<set data-value=\"\\\\x\\\\?resize02b\"/>"));
+        Assert.assertTrue(result, result.contains("<set data-value=\"\\?resize03a\"/>"));
+        Assert.assertTrue(result, result.contains("<set data-value=\"\\x\\?resize03b\"/>"));
+        Assert.assertTrue(result, result.contains("<\\?resize04/>"));
+    }
+
     /** 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/bug5nnnn/bug56334.jspx
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/webapp/bug5nnnn/bug56334.jspx?rev=1586890&view=auto
==============================================================================
--- tomcat/trunk/test/webapp/bug5nnnn/bug56334.jspx (added)
+++ tomcat/trunk/test/webapp/bug5nnnn/bug56334.jspx Sat Apr 12 20:07:54 2014
@@ -0,0 +1,26 @@
+<jsp:root version="2.3"
+    xmlns="http://www.w3.org/1999/xhtml"
+    xmlns:jsp="http://java.sun.com/JSP/Page"
+    xmlns:c="http://java.sun.com/jsp/jstl/core">
+
+    <jsp:directive.page contentType="text/plain; charset=ISO-8859-1"/>
+
+    <!-- Test 1: Use \\ in EL in tag attribute -->
+    <c:set var="asd" value="${'\\?resize01'}" />
+    <c:out value="${asd}"/>
+
+    <!-- Test 2a: Use \\\\ in template text -->
+    <set data-value="${'\\\\?resize02a'}" />
+
+    <!-- Test 2b: Use \\\\ in template text -->
+    <set data-value="\\\\x${'\\\\?resize02b'}" />
+
+    <!-- Test 3a: Use \\ in template text -->
+    <set data-value="${'\\?resize03a'}" />
+
+    <!-- Test 3b: Use \\ in template text -->
+    <set data-value="\\x${'\\?resize03b'}" />
+
+    <!-- Test 4: Use \\ in jsp:element -->
+    <jsp:element name="${'\\?resize04'}"></jsp:element>
+</jsp:root>
\ No newline at end of file

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1586890&r1=1586889&r2=1586890&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Sat Apr 12 20:07:54 2014
@@ -115,6 +115,14 @@
       </fix>
     </changelog>
   </subsection>
+  <subsection name="Jasper">
+    <changelog>
+      <fix>
+        <bug>56334</bug>: Fix a regression in the handling of back-slash
+        escaping introduced by the fix for <bug>55735</bug>. (markt)
+      </fix>
+    </changelog>
+  </subsection>
   <subsection name="Cluster">
     <changelog>
       <scode>



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


Re: svn commit: r1586890 - in /tomcat/trunk: java/org/apache/jasper/compiler/ELParser.java test/org/apache/jasper/compiler/TestELParser.java test/org/apache/jasper/compiler/TestParser.java test/webapp/bug5nnnn/bug56334.jspx webapps/docs/changelog.xml

Posted by Mark Thomas <ma...@apache.org>.
On 13/04/2014 22:30, Konstantin Kolinko wrote:
> 2014-04-13 0:07 GMT+04:00  <ma...@apache.org>:
>> Author: markt
>> Date: Sat Apr 12 20:07:54 2014
>> New Revision: 1586890
>>
>> URL: http://svn.apache.org/r1586890
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56334
>> Correct double backslash escaping in attributes
>>
>> Added:
>>     tomcat/trunk/test/webapp/bug5nnnn/bug56334.jspx
>> Modified:
>>     tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java
>>     tomcat/trunk/test/org/apache/jasper/compiler/TestELParser.java
>>     tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java
>>     tomcat/trunk/webapps/docs/changelog.xml
>>
>> 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=1586890&r1=1586889&r2=1586890&view=diff
>> ==============================================================================
>> --- tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java (original)
>> +++ tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java Sat Apr 12 20:07:54 2014
>> @@ -209,7 +209,7 @@ public class ELParser {
>>                  prev = 0;
>>                  if (ch == '\\') {
>>                      buf.append('\\');
>> -                    prev = '\\';
>> +                    continue;
>>                  } else if (ch == '$'
>>                          || (!isDeferredSyntaxAllowedAsLiteral && ch == '#')) {
>>                      buf.append(ch);
> 
> I think it needs 'continue;' here in this branch as well.
> (So that  "if (ch == '\\' || ch == '$' " block below does not happen
> and does not set prev=ch. )
> 
> I wonder what a test case it will be.

I'll take a look.


>> @@ -468,18 +468,18 @@ public class ELParser {
>>
>>          @Override
>>          public void visit(Function n) throws JasperException {
>> -            output.append(n.getOriginalText());
>> +            output.append(Generator.escape(n.getOriginalText()));
> 
> The above method is escaping for Java strings.
> E.g. it escapes LF -> '\' + 'n', but that is a wrong escaping for this use case.

Yes. I'd like to blame the jet lag for that one but I think I just
wasn't thinking straight.

Mark


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


Re: svn commit: r1586890 - in /tomcat/trunk: java/org/apache/jasper/compiler/ELParser.java test/org/apache/jasper/compiler/TestELParser.java test/org/apache/jasper/compiler/TestParser.java test/webapp/bug5nnnn/bug56334.jspx webapps/docs/changelog.xml

Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-04-13 0:07 GMT+04:00  <ma...@apache.org>:
> Author: markt
> Date: Sat Apr 12 20:07:54 2014
> New Revision: 1586890
>
> URL: http://svn.apache.org/r1586890
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56334
> Correct double backslash escaping in attributes
>
> Added:
>     tomcat/trunk/test/webapp/bug5nnnn/bug56334.jspx
> Modified:
>     tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java
>     tomcat/trunk/test/org/apache/jasper/compiler/TestELParser.java
>     tomcat/trunk/test/org/apache/jasper/compiler/TestParser.java
>     tomcat/trunk/webapps/docs/changelog.xml
>
> 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=1586890&r1=1586889&r2=1586890&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java (original)
> +++ tomcat/trunk/java/org/apache/jasper/compiler/ELParser.java Sat Apr 12 20:07:54 2014
> @@ -209,7 +209,7 @@ public class ELParser {
>                  prev = 0;
>                  if (ch == '\\') {
>                      buf.append('\\');
> -                    prev = '\\';
> +                    continue;
>                  } else if (ch == '$'
>                          || (!isDeferredSyntaxAllowedAsLiteral && ch == '#')) {
>                      buf.append(ch);

I think it needs 'continue;' here in this branch as well.
(So that  "if (ch == '\\' || ch == '$' " block below does not happen
and does not set prev=ch. )

I wonder what a test case it will be.

> @@ -468,18 +468,18 @@ public class ELParser {
>
>          @Override
>          public void visit(Function n) throws JasperException {
> -            output.append(n.getOriginalText());
> +            output.append(Generator.escape(n.getOriginalText()));

The above method is escaping for Java strings.
E.g. it escapes LF -> '\' + 'n', but that is a wrong escaping for this use case.

>              output.append('(');
>          }
>
>          @Override
>          public void visit(Text n) throws JasperException {
> -            output.append(n.getText());
> +            output.append(Generator.escape(n.getText()));
>          }
>
>          @Override
>          public void visit(ELText n) throws JasperException {
> -            output.append(n.getText());
> +            output.append(Generator.escape(n.getText()));
>          }
>      }
>  }
>

Best regards,
Konstantin Kolinko

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