You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2017/03/16 14:28:16 UTC

[Bug 60876] New: Rfc6265CookieProcessor: syntax of Set-Cookie header deviates from spec

https://bz.apache.org/bugzilla/show_bug.cgi?id=60876

            Bug ID: 60876
           Summary: Rfc6265CookieProcessor: syntax of Set-Cookie header
                    deviates from spec
           Product: Tomcat 8
           Version: 8.5.11
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Catalina
          Assignee: dev@tomcat.apache.org
          Reporter: jim.griswold@gmail.com
  Target Milestone: ----

Summary:

After a recent upgrade from Tomcat 8.0.28 to 8.5.11, I've noticed a syntax
change in the Set-Cookie header generated by Tomcat on HTTP responses where a
cookie is set with additional attributes such as "Path". From my investigation
thus far, the syntax change appears to be due to implementation details of
Rfc6265CookieProcessor, which was changed to be the default cookie processor in
between the Tomcat versions I tested. The syntax change does not appear to be
compliant with the syntax in RFC-6265, and this is change is breaking at least
one fairly common HTTP client implementation (Apache CXF).

Example:

When I set a cookie named "cookie_name" with value "value" and path "/", the
header that's generated by Tomcat 8.0.28 looks like:

    Set-Cookie: cookie_name=value; Path=/

With 8.5.11, it looks like:

    Set-Cookie: cookie_name=value;path=/

Note the missing space after the semicolon and the change from "Path" to
"path".

Impact:

This small change appears to break at least the Apache CXF WebClient's
implementation of parsing the Set-Cookie header, which is how I noticed the
issue.

In my testing, the CXF client discards the fact that a cookie called
"cookie_name" has been set. I tried upgrading the CXF client to see if a newer
version would be more tolerant, but the issue persisted.

Possible root cause / suggested solution:

After some digging around, I saw that the new Rfc6265CookieProcessor was
changed to be the default cookie processor. When I followed instructions [1] to
change back to the old processor, the original behavior was restored and my
tests passed again.

The RFC 6265 specifies [1] that there must be a space ("SP") between the
semicolon and "Path", and that it should be "Path" with the first letter
uppercased. Taking a look at the Rfc6265CookieProcessor source (the
generateHeader method, specifically), I see the following:

    153: header.append(";path=");

This appears to be a broader issue that impacts elements other than "path",
e.g.

    123: header.append(";Max-Age=");  // should be: "; Max-Age="
    131: header.append (";Expires="); // should be: "; Expires="
    146: header.append(";domain=");   // should be: "; Domain="
    158: header.append(";Secure");    // should be: "; Secure"
    162: header.append(";HttpOnly");  // should be: "; HttpOnly"

While I think clients should tolerate the small change, I think it's best for
the cookie processor to adhere to the RFC more strictly in this case in order
to avoid unexpected issues.

Thank you very much for all of your great work on this project!

[1] https://tools.ietf.org/html/rfc6265#section-4.1.1
[2]
http://stackoverflow.com/questions/38696081/how-to-change-cookie-processor-to-legacycookieprocessor-in-tomcat-8

Java version:

java version "1.8.0_91"
Java(TM) SE Runtime Environment (build 1.8.0_91-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.91-b14, mixed mode)

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60876] Rfc6265CookieProcessor: syntax of Set-Cookie header deviates from spec

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60876

Viveka Singh <vi...@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |viveka.singh@yahoo.com

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60876] Rfc6265CookieProcessor: syntax of Set-Cookie header deviates from spec

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60876

Jim Griswold <ji...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jim.griswold@gmail.com

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60876] Rfc6265CookieProcessor: syntax of Set-Cookie header deviates from spec

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60876

--- Comment #1 from Jim Griswold <ji...@gmail.com> ---
Created attachment 34836
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34836&action=edit
Patch with proposed changes

Adding patch.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[Bug 60876] Rfc6265CookieProcessor: syntax of Set-Cookie header deviates from spec

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60876

Mark Thomas <ma...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #2 from Mark Thomas <ma...@apache.org> ---
Thanks for the report, the analysis and the patch. I hope it is the first of
many.

Fixed in:
- trunk for 9.0.0.M19 onwards
- 8.5.x for 8.5.13 onwards
- 8.0.x for 8.0.43 onwards

I also fixed the test cases that started failing once this issue was fixed.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org