https://bz.apache.org/bugzilla/show_bug.cgi?id=64509
Bug ID: 64509 Summary: Rfc6265CookieProcessor mishandles commas in $Version=1 cookie header Product: Tomcat 9 Version: 9.0.33 Hardware: PC OS: Linux Status: NEW Severity: normal Priority: P2 Component: Util Assignee: dev@tomcat.apache.org Reporter: bill-apa...@carpenter.org Target Milestone: ----- Created attachment 37299 --> https://bz.apache.org/bugzilla/attachment.cgi?id=37299&action=edit test code (Tested in both 9.0.33 and 9.0.36, but 9.0.36 wasn't available in the drop-down when I opened this bug report.) Rfc6265CookieProcessor tries to accept a comma as a cookie pair separator for $Version=1 cookie headers, but it gets it wrong. It also behave differently from the legacy parser. For this Cookie: header value $Version=1;first=1,second=2;third=3;case=justCOMMA the new parser loses cookies "first" and "second" (logs an invalid cookie warning). The legacy parser does not. Here is a small test program (also attached) that parses headers with both processors. The test values include both "$Version=1" cookie headers and the same with no version attribute. I believe the RFC-6265 parsing is not trying to honor the comma, which is fair enough. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> package aside; import org.apache.tomcat.util.http.LegacyCookieProcessor; import org.apache.tomcat.util.http.MimeHeaders; import org.apache.tomcat.util.http.Rfc6265CookieProcessor; import org.apache.tomcat.util.http.ServerCookie; import org.apache.tomcat.util.http.ServerCookies; public class TC9CookieParsingTest { private static final String[] cookieHeaderValues = { "$Version=1;first=1;second=2;third=3;case=justSEMI", "first=1;second=2;third=3;case=justSEMI", "$Version=1;first=1,second=2;third=3;case=justCOMMA", "first=1,second=2;third=3;case=justCOMMA", "$Version=1;first=1,;second=2;third=3;case=COMMAthenSEMI", "first=1,;second=2;third=3;case=COMMAthenSEMI", "$Version=1;first=1;,second=2;third=3;case=SEMIthenCOMMA", "first=1;,second=2;third=3;case=SEMIthenCOMMA", }; public static void main(String[] args) { for (final String cookieHeaderValue: TC9CookieParsingTest.cookieHeaderValues) { TC9CookieParsingTest.parseWithRfc6265Parser(cookieHeaderValue); TC9CookieParsingTest.parseWithLegacyParser(cookieHeaderValue); } } private static void parseWithRfc6265Parser(final String cookieHeaderValue) { final MimeHeaders headers = new MimeHeaders(); headers.addValue("Cookie").setString(cookieHeaderValue); final ServerCookies serverCookies = new ServerCookies(10); final Rfc6265CookieProcessor processor = new Rfc6265CookieProcessor(); processor.parseCookieHeader(headers, serverCookies); final int howMany = serverCookies.getCookieCount(); System.out.println("\n====================\nRfc6265CookieProcessor 'Cookie: " + cookieHeaderValue + "'"); for (int ii=0; ii<howMany; ++ii) { final ServerCookie sc = serverCookies.getCookie(ii); System.out.println(ii + " cookie: '" + sc.getName() + "' = '" + sc.getValue() + "'"); } } private static void parseWithLegacyParser(final String cookieHeaderValue) { final MimeHeaders headers = new MimeHeaders(); headers.addValue("Cookie").setString(cookieHeaderValue); final ServerCookies serverCookies = new ServerCookies(10); final LegacyCookieProcessor processor = new LegacyCookieProcessor(); processor.parseCookieHeader(headers, serverCookies); final int howMany = serverCookies.getCookieCount(); System.out.println("--------------------\nLegacyCookieProcessor 'Cookie: " + cookieHeaderValue + "'"); for (int ii=0; ii<howMany; ++ii) { final ServerCookie sc = serverCookies.getCookie(ii); System.out.println(ii + " cookie: '" + sc.getName() + "' = '" + sc.getValue() + "'"); } } } >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Here is the output from a run of the test program: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ==================== Rfc6265CookieProcessor 'Cookie: $Version=1;first=1;second=2;third=3;case=justSEMI' 0 cookie: 'first' = '1' 1 cookie: 'second' = '2' 2 cookie: 'third' = '3' 3 cookie: 'case' = 'justSEMI' -------------------- LegacyCookieProcessor 'Cookie: $Version=1;first=1;second=2;third=3;case=justSEMI' 0 cookie: 'first' = '1' 1 cookie: 'second' = '2' 2 cookie: 'third' = '3' 3 cookie: 'case' = 'justSEMI' ==================== Rfc6265CookieProcessor 'Cookie: first=1;second=2;third=3;case=justSEMI' 0 cookie: 'first' = '1' 1 cookie: 'second' = '2' 2 cookie: 'third' = '3' 3 cookie: 'case' = 'justSEMI' -------------------- LegacyCookieProcessor 'Cookie: first=1;second=2;third=3;case=justSEMI' 0 cookie: 'first' = '1' 1 cookie: 'second' = '2' 2 cookie: 'third' = '3' 3 cookie: 'case' = 'justSEMI' ==================== Rfc6265CookieProcessor 'Cookie: $Version=1;first=1,second=2;third=3;case=justCOMMA' 0 cookie: 'third' = '3' 1 cookie: 'case' = 'justCOMMA' -------------------- LegacyCookieProcessor 'Cookie: $Version=1;first=1,second=2;third=3;case=justCOMMA' 0 cookie: 'first' = '1' 1 cookie: 'second' = '2' 2 cookie: 'third' = '3' 3 cookie: 'case' = 'justCOMMA' ==================== Rfc6265CookieProcessor 'Cookie: first=1,second=2;third=3;case=justCOMMA' 0 cookie: 'third' = '3' 1 cookie: 'case' = 'justCOMMA' -------------------- LegacyCookieProcessor 'Cookie: first=1,second=2;third=3;case=justCOMMA' 0 cookie: 'first' = '1' 1 cookie: 'second' = '2' 2 cookie: 'third' = '3' 3 cookie: 'case' = 'justCOMMA' ==================== Rfc6265CookieProcessor 'Cookie: $Version=1;first=1,;second=2;third=3;case=COMMAthenSEMI' 0 cookie: 'first' = '1' 1 cookie: 'second' = '2' 2 cookie: 'third' = '3' 3 cookie: 'case' = 'COMMAthenSEMI' -------------------- LegacyCookieProcessor 'Cookie: $Version=1;first=1,;second=2;third=3;case=COMMAthenSEMI' 0 cookie: 'first' = '1' 1 cookie: 'second' = '2' 2 cookie: 'third' = '3' 3 cookie: 'case' = 'COMMAthenSEMI' ==================== Rfc6265CookieProcessor 'Cookie: first=1,;second=2;third=3;case=COMMAthenSEMI' 0 cookie: 'second' = '2' 1 cookie: 'third' = '3' 2 cookie: 'case' = 'COMMAthenSEMI' -------------------- LegacyCookieProcessor 'Cookie: first=1,;second=2;third=3;case=COMMAthenSEMI' 0 cookie: 'first' = '1' 1 cookie: 'second' = '2' 2 cookie: 'third' = '3' 3 cookie: 'case' = 'COMMAthenSEMI' ==================== Rfc6265CookieProcessor 'Cookie: $Version=1;first=1;,second=2;third=3;case=SEMIthenCOMMA' 0 cookie: 'first' = '1' 1 cookie: 'third' = '3' 2 cookie: 'case' = 'SEMIthenCOMMA' -------------------- LegacyCookieProcessor 'Cookie: $Version=1;first=1;,second=2;third=3;case=SEMIthenCOMMA' 0 cookie: 'first' = '1' 1 cookie: 'second' = '2' 2 cookie: 'third' = '3' 3 cookie: 'case' = 'SEMIthenCOMMA' ==================== Rfc6265CookieProcessor 'Cookie: first=1;,second=2;third=3;case=SEMIthenCOMMA' 0 cookie: 'first' = '1' 1 cookie: 'third' = '3' 2 cookie: 'case' = 'SEMIthenCOMMA' -------------------- LegacyCookieProcessor 'Cookie: first=1;,second=2;third=3;case=SEMIthenCOMMA' 0 cookie: 'first' = '1' 1 cookie: 'second' = '2' 2 cookie: 'third' = '3' 3 cookie: 'case' = 'SEMIthenCOMMA' >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I've had a quick look at the parser code. I suspect the problem lines in org.apache.tomcat.util.http.parser.Cookie, near line 274: skipResult = skipByte(bb, COMMA_BYTE); if (skipResult == SkipResult.FOUND) { parseAttributes = false; } skipResult = skipByte(bb, SEMICOLON_BYTE); if (skipResult == SkipResult.EOF) { parseAttributes = false; moreToProcess = false; } else if (skipResult == SkipResult.NOT_FOUND) { skipInvalidCookie(bb); continue; } Even though we've just found the COMMA_BYTE, it's still a failure if we don't then fint the SEMICOLON_BYTE. I suspect that wanted to be an "else if", but since the parser is kind of complicated I hesitate to claim that's a definitive fix. It might break some other edge case. -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org