tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <ma...@apache.org>
Subject Re: svn commit: r1553187 - in /tomcat/trunk: java/org/apache/tomcat/util/http/Cookies.java test/org/apache/tomcat/util/http/TestCookies.java webapps/docs/changelog.xml
Date Tue, 24 Dec 2013 10:29:01 GMT
On 23/12/2013 19:15, jboynes@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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 


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


Mime
View raw message