On 23/12/2013 19:15, jboy...@apache.org wrote: > Author: jboynes > Date: Mon Dec 23 19:15:35 2013 > New Revision: 1553187 > > URL: http://svn.apache.org/r1553187 > Log: > fix #55917 by allowing 8-bit ISO-8859-1 characters in V0 cookie values
-1 (veto) The Tomcat community has (to date) always taken the view that cookie headers should be restricted (as per RFC2616 section 4.2) to "combinations of token, separators, and quoted-string". That does not permit 0x80 to 0xFF in tokens. Mark > Modified: > tomcat/trunk/java/org/apache/tomcat/util/http/Cookies.java > tomcat/trunk/test/org/apache/tomcat/util/http/TestCookies.java > tomcat/trunk/webapps/docs/changelog.xml > > Modified: tomcat/trunk/java/org/apache/tomcat/util/http/Cookies.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/Cookies.java?rev=1553187&r1=1553186&r2=1553187&view=diff > ============================================================================== > --- tomcat/trunk/java/org/apache/tomcat/util/http/Cookies.java (original) > +++ tomcat/trunk/java/org/apache/tomcat/util/http/Cookies.java Mon Dec 23 > 19:15:35 2013 > @@ -508,14 +508,7 @@ public final class Cookies { > private static final int getTokenEndPosition(byte bytes[], int off, int > end, > int version, boolean isName){ > int pos = off; > - while (pos < end && > - (!CookieSupport.isHttpSeparator((char)bytes[pos]) || > - version == 0 && > - CookieSupport.ALLOW_HTTP_SEPARATORS_IN_V0 && > - bytes[pos] != '=' && > - !CookieSupport.isV0Separator((char)bytes[pos]) || > - !isName && bytes[pos] == '=' && > - CookieSupport.ALLOW_EQUALS_IN_VALUE)) { > + while (pos < end && allowInToken(bytes[pos], version, isName)) { > pos++; > } > > @@ -525,6 +518,34 @@ public final class Cookies { > return pos; > } > > + private static boolean allowInToken(byte b, int version, boolean isName) > { > + // byte is signed so cast into a positive int for comparisons > + int octet = ((int)b) & 0xff; > + > + // disallow all controls > + if (octet < 0x20 && octet != 0x09 || octet >= 0x7f && octet < 0xa0) { > + throw new IllegalArgumentException( > + "Control character in cookie value or attribute."); > + } > + > + // values 0xa0-0xff are allowed in V0 values, otherwise disallow > + if (octet >= 0x80) { > + if (isName || version != 0) { > + throw new IllegalArgumentException( > + "Control character in cookie value or attribute."); > + } > + return true; > + } > + > + return !CookieSupport.isHttpSeparator((char) b) || > + version == 0 && > + CookieSupport.ALLOW_HTTP_SEPARATORS_IN_V0 && > + b != '=' && > + !CookieSupport.isV0Separator((char) b) || > + !isName && b == '=' && > + CookieSupport.ALLOW_EQUALS_IN_VALUE; > + } > + > /** > * Given a starting position after an initial quote character, this gets > * the position of the end quote. This escapes anything after a '\' char > > Modified: tomcat/trunk/test/org/apache/tomcat/util/http/TestCookies.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/http/TestCookies.java?rev=1553187&r1=1553186&r2=1553187&view=diff > ============================================================================== > --- tomcat/trunk/test/org/apache/tomcat/util/http/TestCookies.java (original) > +++ tomcat/trunk/test/org/apache/tomcat/util/http/TestCookies.java Mon Dec 23 > 19:15:35 2013 > @@ -17,9 +17,113 @@ > > package org.apache.tomcat.util.http; > > +import java.nio.charset.StandardCharsets; > + > +import javax.servlet.http.Cookie; > + > +import org.junit.Assert; > +import org.junit.Before; > +import org.junit.Ignore; > import org.junit.Test; > > public class TestCookies { > + private Cookies cookies; > + > + @Before > + public void init() { > + this.cookies = new Cookies(null); > + } > + > + @Test > + public void skipJsonInV0Value() { > + process("bad={\"v\":1,\"x\":2}; a=b"); > + expect(makeCookie("a", "b", 0)); > + } > + > + @Test(expected = IllegalArgumentException.class) > + public void disallow8bitInName() { > + process("f\u00f6o=bar"); > + } > + > + @Test(expected = IllegalArgumentException.class) > + public void disallowControlInName() { > + process("f\010o=bar"); > + } > + > + @Test(expected = IllegalArgumentException.class) > + public void disallow8BitControlInName() { > + process("f\210o=bar"); > + } > + > + @Test > + public void allow8BitInV0Value() { > + process("foo=b\u00e1r"); > + expect(makeCookie("foo", "b\u00e1r", 0)); > + } > + > + @Test(expected = IllegalArgumentException.class) > + public void disallow8bitInV1UnquotedValue() { > + process("$Version=1; foo=b\u00e1r"); > + } > + > + @Test > + public void allow8bitInV1QuotedValue() { > + process("$Version=1; foo=\"b\u00e1r\""); > + expect(makeCookie("foo", "b\u00e1r", 1)); > + } > + > + @Test(expected = IllegalArgumentException.class) > + public void disallowControlInV0Value() { > + process("foo=b\010r"); > + } > + > + @Test(expected = IllegalArgumentException.class) > + public void disallow8BitControlInV0Value() { > + process("foo=b\210r"); > + } > + > + @Test(expected = IllegalArgumentException.class) > + public void disallowControlInV1UnquotedValue() { > + process("$Version=1; foo=b\010r"); > + } > + > + @Ignore > + @Test(expected = IllegalArgumentException.class) > + public void disallowControlInV1QuotedValue() { > + process("$Version=1; foo=\"b\010r\""); > + } > + > + @Test(expected = IllegalArgumentException.class) > + public void disallow8BitControlInV1UnquotedValue() { > + process("$Version=1; foo=b\210r"); > + } > + > + @Ignore > + @Test > + public void allow8BitControlInV1QuotedValue() { > + process("$Version=1; foo=\"b\210r\""); > + expect(makeCookie("foo", "b\210r", 1)); > + } > + > + private void process(String header) { > + byte[] bytes = header.getBytes(StandardCharsets.ISO_8859_1); > + cookies.processCookieHeader(bytes, 0, bytes.length); > + } > + > + private void expect(Cookie... expected) { > + Assert.assertEquals(expected.length, cookies.getCookieCount()); > + for (int i = 0; i < expected.length; i++) { > + ServerCookie actual = cookies.getCookie(i); > + Assert.assertEquals(expected[i].getName(), > actual.getName().toString()); > + Assert.assertEquals(expected[i].getValue(), > actual.getValue().toString()); > + } > + } > + > + private static Cookie makeCookie(String name, String value, int version) > { > + Cookie cookie = new Cookie(name, value); > + cookie.setVersion(version); > + return cookie; > + } > > @Test > public void testCookies() throws Exception { > > Modified: tomcat/trunk/webapps/docs/changelog.xml > URL: > http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1553187&r1=1553186&r2=1553187&view=diff > ============================================================================== > --- tomcat/trunk/webapps/docs/changelog.xml (original) > +++ tomcat/trunk/webapps/docs/changelog.xml Mon Dec 23 19:15:35 2013 > @@ -225,6 +225,10 @@ > Change the default URIEncoding for all connectors from ISO-8859-1 to > UTF-8. (markt) > </update> > + <scode> > + <bug>55917</bug>: Allow ISO-8859-1 characters 0xA0-0xFF in V0 cookie > + values (jboynes). > + </scode> > </changelog> > </subsection> > <subsection name="Jasper"> > > > > --------------------------------------------------------------------- > 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
