Jetty
  1. Jetty
  2. JETTY-154

Cookie parsing issue with Jetty 5

    Details

    • Type: Bug Bug
    • Status: Resolved Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.0.2, 6.1.0pre3
    • Component/s: HTTP
    • Labels:
      None
    • Number of attachments :
      0

      Description

      I am having an issue with Jetty parsing a cookie that is being sent to an application. This is the value of the cookie:

      %c2%a8%c3%acR%13%7b%e2%82%acX%c3%9a%c3%9b%3d%22%cb%9c%c3%ae3r%c3%b5%c3%8d%c5%bd'%c2%b8%e2%82%ac%1f%e2%84%a2P

      If you go through that mess, you will see there is a single quote ( ' ) in the string that is not encoded. This is causing Jetty to return the value of that cookie as that string plus the name and value of any cookies in the Cookie: header after this string. So, basically, the application does not see any cookies that show up after the single quote. I couldn't find anything in the RFC that forbids a single quote in the cookie value so, I was hoping someone here could tell me if this is an issue that should be fixed in Jetty or if this an issue with the application not encoding that properly.

        Activity

        Hide
        Greg Wilkins added a comment -

        quotes in cookies are a very problematic matter. the specs differ in their implementation and it also plays to the issue of version 0 vs version 1 cookies and
        how browsers handle them etc. etc.

        so ideally it would be best to avoid such special characters in your cookies.

        Having said that, I think Jetty could probably do better with parsing a single single quote - or at least encode it.

        Do you know if the %xx encoding in that string is being done by Jetty or by their application?

        Show
        Greg Wilkins added a comment - quotes in cookies are a very problematic matter. the specs differ in their implementation and it also plays to the issue of version 0 vs version 1 cookies and how browsers handle them etc. etc. so ideally it would be best to avoid such special characters in your cookies. Having said that, I think Jetty could probably do better with parsing a single single quote - or at least encode it. Do you know if the %xx encoding in that string is being done by Jetty or by their application?
        Hide
        Greg Wilkins added a comment -

        Try this patch:

        Index: src/org/mortbay/http/HttpFields.java
        ===================================================================
        RCS file: /cvsroot/jetty/Jetty/src/org/mortbay/http/HttpFields.java,v
        retrieving revision 1.76
        diff -r1.76 HttpFields.java
        1463c1463
        < URI.encodeString(buf,value,"\";, ");

        > URI.encodeString(buf,value,"\";, '");
        1465c1465
        < buf.append(QuotedStringTokenizer.quote(value,"\";, "));

        > buf.append(QuotedStringTokenizer.quote(value,"\";, '"));

        Show
        Greg Wilkins added a comment - Try this patch: Index: src/org/mortbay/http/HttpFields.java =================================================================== RCS file: /cvsroot/jetty/Jetty/src/org/mortbay/http/HttpFields.java,v retrieving revision 1.76 diff -r1.76 HttpFields.java 1463c1463 < URI.encodeString(buf,value,"\";, "); — > URI.encodeString(buf,value,"\";, '"); 1465c1465 < buf.append(QuotedStringTokenizer.quote(value,"\";, ")); — > buf.append(QuotedStringTokenizer.quote(value,"\";, '"));
        Hide
        Tony Thompson added a comment -

        The encoding is done by the application.

        The patch works for my test I put together. I will try to get it in for the customer in the next couple of days and let you know how it works "in production".

        Thanks for the quick turnaround.

        Show
        Tony Thompson added a comment - The encoding is done by the application. The patch works for my test I put together. I will try to get it in for the customer in the next couple of days and let you know how it works "in production". Thanks for the quick turnaround.
        Hide
        Tony Thompson added a comment -

        OK, of course my test was a little different than what I am doing in production. But, I think I have a test case that matches better what is happening in production. The cookie handling is being done in a proxy handler that doesn't go through HttpFields.addSetCookie() to get cookies in the response. It is copying headers directly into the HttpResponse (my test was using response.addCookie() in a JSP so of course it would work that way) so the value in the Set-Cookie header is literally what I posted in my original message. Jetty never gets a chance to encode things more.

        So, to simulate the case, if you have a servlet/JSP that sets the cookie header directly:

        String str = "cookie1=%c2%a8%c3%acR%13%7b%e2%82%acX%c3%9a%c3%9b%3d%22%cb%9c%c3%ae3r%c3%b5%c3%8d%c5%bd'%c2%b8%e2%82%ac%1f%e2%84%a2P";
        response.setHeader( "Set-Cookie", str );

        Any cookie sent back to Jetty in the Cookie: header after "cookie1" shows up in the value of cookie1. Sorry I didn't explain it better in my original post. Hopefully there is still a way to fix this.

        Show
        Tony Thompson added a comment - OK, of course my test was a little different than what I am doing in production. But, I think I have a test case that matches better what is happening in production. The cookie handling is being done in a proxy handler that doesn't go through HttpFields.addSetCookie() to get cookies in the response. It is copying headers directly into the HttpResponse (my test was using response.addCookie() in a JSP so of course it would work that way) so the value in the Set-Cookie header is literally what I posted in my original message. Jetty never gets a chance to encode things more. So, to simulate the case, if you have a servlet/JSP that sets the cookie header directly: String str = "cookie1=%c2%a8%c3%acR%13%7b%e2%82%acX%c3%9a%c3%9b%3d%22%cb%9c%c3%ae3r%c3%b5%c3%8d%c5%bd'%c2%b8%e2%82%ac%1f%e2%84%a2P"; response.setHeader( "Set-Cookie", str ); Any cookie sent back to Jetty in the Cookie: header after "cookie1" shows up in the value of cookie1. Sorry I didn't explain it better in my original post. Hopefully there is still a way to fix this.
        Hide
        Greg Wilkins added a comment -

        Sorry tony,

        but I don't think I will fix this one. As far as I can tell, quoting cookies is legal so I do not know when to ignore quotes and when not to ignore quotes.

        Also, there is a simple work around, as you get call request.getHeaders("Cookie") directly are parse the results as you need
        to. You could even do this in a filter so down stream code would still call getCookies to get the parsed results.

        sorry

        Show
        Greg Wilkins added a comment - Sorry tony, but I don't think I will fix this one. As far as I can tell, quoting cookies is legal so I do not know when to ignore quotes and when not to ignore quotes. Also, there is a simple work around, as you get call request.getHeaders("Cookie") directly are parse the results as you need to. You could even do this in a filter so down stream code would still call getCookies to get the parsed results. sorry
        Hide
        Tony Thompson added a comment -

        I guess this may be klunky but, wouldn't a quoted cookie start with =" and end with ";, if the cookie is really quoted (of course there could be whitespace in there but a parser can easily figure that out)? My issue is a single quote in the cookie value without the cookie value being quoted. I am not sure parsing the cookie header myself is a very good option. Seems the container should be able to figure it out for me.

        Show
        Tony Thompson added a comment - I guess this may be klunky but, wouldn't a quoted cookie start with =" and end with ";, if the cookie is really quoted (of course there could be whitespace in there but a parser can easily figure that out)? My issue is a single quote in the cookie value without the cookie value being quoted. I am not sure parsing the cookie header myself is a very good option. Seems the container should be able to figure it out for me.
        Hide
        Tony Thompson added a comment -

        For what it's worth, I tested this with Tomcat and they appear to handle it fine. I included the requests headers below to show that the JSESSIONID is in fact being sent after the test cookie. Here is the full JSP that I am using to test with:

        <%@ page import="java.io.,java.util.,java.net.*" session="true"%>

        <html>
        <head>
        </head>
        <body>
        <%
        boolean found = false;
        Cookie[] cks = request.getCookies();
        if( cks != null ) {
        for( int i=0; i<cks.length; i++ )

        { %>Cookie: <%= cks[i].getName() %><br/> Value: <%= cks[i].getValue() %><br/><br/><% if( "cookie1".equalsIgnoreCase( cks[i].getName() ) ) found = true; }

        }

        if( ! found )

        { String str = "cookie1=%c2%a8%c3%acR%13%7b%e2%82%acX%c3%9a%c3%9b%3d%22%cb%9c%c3%ae3r%c3%b5%c3%8d%c5%bd'%c2%b8%e2%82%ac%1f%e2%84%a2P;"; response.setHeader( "Set-Cookie", str ); }

        %>
        </body>
        </html>

        This is the raw request that is sent to the JSP (after the cookie is set):

        GET /init.jsp HTTP/1.1
        Accept: /
        Accept-Language: en-us
        Accept-Encoding: gzip, deflate
        User-Agent: Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322)
        Host: 192.168.1.27:8080
        Connection: Keep-Alive
        Cache-Control: no-cache
        Cookie: cookie1=%c2%a8%c3%acR%13%7b%e2%82%acX%c3%9a%c3%9b%3d%22%cb%9c%c3%ae3r%c3%b5%c3%8d%c5%bd'%c2%b8%e2%82%ac%1f%e2%84%a2P; JSESSIONID=BCEA3F599B082CE0BD67830232A5C26A

        For completeness (not that it matters other than to show it is handled by Tomcat), here is the response to the above request:

        HTTP/1.1 200 OK
        Server: Apache-Coyote/1.1
        Content-Type: text/html;charset=ISO-8859-1
        Content-Length: 278
        Date: Wed, 22 Nov 2006 21:34:01 GMT

        Here is the info displayed in the browser:

        Cookie: cookie1
        Value: %c2%a8%c3%acR%13%7b%e2%82%acX%c3%9a%c3%9b%3d%22%cb%9c%c3%ae3r%c3%b5%c3%8d%c5%bd'%c2%b8%e2%82%ac%1f%e2%84%a2P

        Cookie: JSESSIONID
        Value: BCEA3F599B082CE0BD67830232A5C26A

        Show
        Tony Thompson added a comment - For what it's worth, I tested this with Tomcat and they appear to handle it fine. I included the requests headers below to show that the JSESSIONID is in fact being sent after the test cookie. Here is the full JSP that I am using to test with: <%@ page import="java.io. ,java.util. ,java.net.*" session="true"%> <html> <head> </head> <body> <% boolean found = false; Cookie[] cks = request.getCookies(); if( cks != null ) { for( int i=0; i<cks.length; i++ ) { %>Cookie: <%= cks[i].getName() %><br/> Value: <%= cks[i].getValue() %><br/><br/><% if( "cookie1".equalsIgnoreCase( cks[i].getName() ) ) found = true; } } if( ! found ) { String str = "cookie1=%c2%a8%c3%acR%13%7b%e2%82%acX%c3%9a%c3%9b%3d%22%cb%9c%c3%ae3r%c3%b5%c3%8d%c5%bd'%c2%b8%e2%82%ac%1f%e2%84%a2P;"; response.setHeader( "Set-Cookie", str ); } %> </body> </html> This is the raw request that is sent to the JSP (after the cookie is set): GET /init.jsp HTTP/1.1 Accept: / Accept-Language: en-us Accept-Encoding: gzip, deflate User-Agent: Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322) Host: 192.168.1.27:8080 Connection: Keep-Alive Cache-Control: no-cache Cookie: cookie1=%c2%a8%c3%acR%13%7b%e2%82%acX%c3%9a%c3%9b%3d%22%cb%9c%c3%ae3r%c3%b5%c3%8d%c5%bd'%c2%b8%e2%82%ac%1f%e2%84%a2P; JSESSIONID=BCEA3F599B082CE0BD67830232A5C26A For completeness (not that it matters other than to show it is handled by Tomcat), here is the response to the above request: HTTP/1.1 200 OK Server: Apache-Coyote/1.1 Content-Type: text/html;charset=ISO-8859-1 Content-Length: 278 Date: Wed, 22 Nov 2006 21:34:01 GMT Here is the info displayed in the browser: Cookie: cookie1 Value: %c2%a8%c3%acR%13%7b%e2%82%acX%c3%9a%c3%9b%3d%22%cb%9c%c3%ae3r%c3%b5%c3%8d%c5%bd'%c2%b8%e2%82%ac%1f%e2%84%a2P Cookie: JSESSIONID Value: BCEA3F599B082CE0BD67830232A5C26A
        Hide
        Tony Thompson added a comment -

        Sorry, apparently posting some of that content didn't come through very well. Hopefully you get the idea. If not, I can send everything some other way, if needed.

        Show
        Tony Thompson added a comment - Sorry, apparently posting some of that content didn't come through very well. Hopefully you get the idea. If not, I can send everything some other way, if needed.
        Hide
        Greg Wilkins added a comment -

        I'll think about it some more

        Show
        Greg Wilkins added a comment - I'll think about it some more
        Hide
        Greg Wilkins added a comment -

        can you add your javascript as an attachment.... I think it is being miss quoted

        Show
        Greg Wilkins added a comment - can you add your javascript as an attachment.... I think it is being miss quoted
        Hide
        Greg Wilkins added a comment -

        Ah - it looks like browsers only pay attention to double quotes and not single quotes!
        So the solution may simple be to only dequote double quotes!

        Show
        Greg Wilkins added a comment - Ah - it looks like browsers only pay attention to double quotes and not single quotes! So the solution may simple be to only dequote double quotes!
        Hide
        Greg Wilkins added a comment -

        Tony,

        I have a fix in head of cvs for jetty 5.
        Can you test ASAP?

        Show
        Greg Wilkins added a comment - Tony, I have a fix in head of cvs for jetty 5. Can you test ASAP?
        Hide
        Greg Wilkins added a comment -

        Fixed in 5.1, 6.0 and 6.1
        single quotes are just ignored.

        Show
        Greg Wilkins added a comment - Fixed in 5.1, 6.0 and 6.1 single quotes are just ignored.

          People

          • Assignee:
            Greg Wilkins
            Reporter:
            Tony Thompson
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: