From f04003abbe60d863a7a02212d722283078104801 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Fri, 13 Mar 2009 13:53:53 +0000 Subject: [PATCH] CookieEncoder / CookieDecoder should not urlencode/urldecode attributes except for the value --- .../netty/handler/codec/http/Cookie.java | 4 +- .../handler/codec/http/CookieDecoder.java | 10 ++--- .../handler/codec/http/CookieEncoder.java | 10 ++--- .../handler/codec/http/DefaultCookie.java | 36 +++++++++++---- .../handler/codec/http/CookieDecoderTest.java | 44 +++++++++---------- .../handler/codec/http/CookieEncoderTest.java | 30 ++++++------- 6 files changed, 77 insertions(+), 57 deletions(-) diff --git a/src/main/java/org/jboss/netty/handler/codec/http/Cookie.java b/src/main/java/org/jboss/netty/handler/codec/http/Cookie.java index 4fb14f3747..411e3dfe16 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/Cookie.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/Cookie.java @@ -44,8 +44,8 @@ public interface Cookie extends Comparable { void setVersion(int version); boolean isSecure(); void setSecure(boolean secure); - String getCommentURL(); - void setCommentURL(String commentURL); + String getCommentUrl(); + void setCommentUrl(String commentUrl); boolean isDiscard(); void setDiscard(boolean discard); Set getPorts(); diff --git a/src/main/java/org/jboss/netty/handler/codec/http/CookieDecoder.java b/src/main/java/org/jboss/netty/handler/codec/http/CookieDecoder.java index 80d5f59c48..e37539366b 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/CookieDecoder.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/CookieDecoder.java @@ -92,17 +92,17 @@ public class CookieDecoder { name = val[0].trim(); value = val[1].trim(); if (CookieHeaderNames.COMMENT.equalsIgnoreCase(name)) { - comment = QueryStringDecoder.decodeComponent(value, charset); + comment = value; } else if (CookieHeaderNames.COMMENTURL.equalsIgnoreCase(name)) { value = trimSurroundingQuotes(value); - commentURL = QueryStringDecoder.decodeComponent(value, charset); + commentURL = value; } else if (CookieHeaderNames.DOMAIN.equalsIgnoreCase(name)) { - domain = QueryStringDecoder.decodeComponent(value, charset); + domain = value; } else if (CookieHeaderNames.PATH.equalsIgnoreCase(name)) { - path = QueryStringDecoder.decodeComponent(value, charset); + path = value; } else if (CookieHeaderNames.EXPIRES.equalsIgnoreCase(name)) { // FIXME: Expires attribute has different representation from Max-Age. @@ -138,7 +138,7 @@ public class CookieDecoder { theCookie.setComment(comment); } if (version > 1) { - theCookie.setCommentURL(commentURL); + theCookie.setCommentUrl(commentURL); if (ports != null) { theCookie.setPorts(ports); } diff --git a/src/main/java/org/jboss/netty/handler/codec/http/CookieEncoder.java b/src/main/java/org/jboss/netty/handler/codec/http/CookieEncoder.java index 391e12d3d2..325bc8e212 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/CookieEncoder.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/CookieEncoder.java @@ -86,11 +86,11 @@ public class CookieEncoder { add(sb, CookieHeaderNames.getMaxAgeString(encodingVersion), cookie.getMaxAge()); if (cookie.getPath() != null) { - add(sb, CookieHeaderNames.PATH, QueryStringEncoder.encodeComponent(cookie.getPath(), charset)); + add(sb, CookieHeaderNames.PATH, cookie.getPath()); } if (cookie.getDomain() != null) { - add(sb, CookieHeaderNames.DOMAIN, QueryStringEncoder.encodeComponent(cookie.getDomain(), charset)); + add(sb, CookieHeaderNames.DOMAIN, cookie.getDomain()); } if (cookie.isSecure()) { sb.append(CookieHeaderNames.SECURE); @@ -98,15 +98,15 @@ public class CookieEncoder { } if (encodingVersion >= 1) { if (cookie.getComment() != null) { - add(sb, CookieHeaderNames.COMMENT, QueryStringEncoder.encodeComponent(cookie.getComment(), charset)); + add(sb, CookieHeaderNames.COMMENT, cookie.getComment()); } add(sb, CookieHeaderNames.VERSION, 1); } if (encodingVersion == 2) { - if (cookie.getCommentURL() != null) { - addQuoted(sb, CookieHeaderNames.COMMENTURL, QueryStringEncoder.encodeComponent(cookie.getCommentURL(), charset)); + if (cookie.getCommentUrl() != null) { + addQuoted(sb, CookieHeaderNames.COMMENTURL, cookie.getCommentUrl()); } if(!cookie.getPorts().isEmpty()) { sb.append(CookieHeaderNames.PORT); diff --git a/src/main/java/org/jboss/netty/handler/codec/http/DefaultCookie.java b/src/main/java/org/jboss/netty/handler/codec/http/DefaultCookie.java index 00df04a5bf..7984855bbf 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/DefaultCookie.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/DefaultCookie.java @@ -39,7 +39,7 @@ public class DefaultCookie implements Cookie { private String domain; private String path; private String comment; - private String commentURL; + private String commentUrl; private boolean discard; private Set ports = Collections.emptySet(); private Set unmodifiablePorts = ports; @@ -97,7 +97,7 @@ public class DefaultCookie implements Cookie { } public void setDomain(String domain) { - this.domain = domain; + this.domain = validateValue("domain", domain); } public String getPath() { @@ -105,7 +105,7 @@ public class DefaultCookie implements Cookie { } public void setPath(String path) { - this.path = path; + this.path = validateValue("path", path); } public String getComment() { @@ -113,15 +113,15 @@ public class DefaultCookie implements Cookie { } public void setComment(String comment) { - this.comment = comment; + this.comment = validateValue("comment", comment); } - public String getCommentURL() { - return commentURL; + public String getCommentUrl() { + return commentUrl; } - public void setCommentURL(String commentURL) { - this.commentURL = commentURL; + public void setCommentUrl(String commentUrl) { + this.commentUrl = validateValue("commentUrl", commentUrl); } public boolean isDiscard() { @@ -251,4 +251,24 @@ public class DefaultCookie implements Cookie { } return buf.toString(); } + + private static String validateValue(String name, String value) { + if (value == null) { + return null; + } + value = value.trim(); + if (value.length() == 0) { + return null; + } + for (int i = 0; i < value.length(); i ++) { + char c = value.charAt(i); + switch (c) { + case '\r': case '\n': case '\f': case 0x0b: case ';': + throw new IllegalArgumentException( + value + " contains one of the following prohibited characters: " + + ";\\r\\n\\f\\v (" + value + ')'); + } + } + return value; + } } \ No newline at end of file diff --git a/src/test/java/org/jboss/netty/handler/codec/http/CookieDecoderTest.java b/src/test/java/org/jboss/netty/handler/codec/http/CookieDecoderTest.java index ae5478cc41..35e28828fd 100644 --- a/src/test/java/org/jboss/netty/handler/codec/http/CookieDecoderTest.java +++ b/src/test/java/org/jboss/netty/handler/codec/http/CookieDecoderTest.java @@ -33,7 +33,7 @@ import org.junit.Test; public class CookieDecoderTest { @Test public void testDecodingSingleCookieV0() { - String cookieString = "myCookie=myValue;expires=50;path=%2Fapathsomewhere;domain=%2Fadomainsomewhere;secure;"; + String cookieString = "myCookie=myValue;expires=50;path=/apathsomewhere;domain=.adomainsomewhere;secure;"; CookieDecoder cookieDecoder = new CookieDecoder(); Map cookieMap = cookieDecoder.decode(cookieString); assertEquals(1, cookieMap.size()); @@ -41,8 +41,8 @@ public class CookieDecoderTest { assertNotNull(cookie); assertEquals("myValue", cookie.getValue()); assertNull(cookie.getComment()); - assertNull(cookie.getCommentURL()); - assertEquals("/adomainsomewhere", cookie.getDomain()); + assertNull(cookie.getCommentUrl()); + assertEquals(".adomainsomewhere", cookie.getDomain()); assertFalse(cookie.isDiscard()); assertEquals(50, cookie.getMaxAge()); assertEquals("/apathsomewhere", cookie.getPath()); @@ -53,7 +53,7 @@ public class CookieDecoderTest { @Test public void testDecodingSingleCookieV0ExtraParamsIgnored() { - String cookieString = "myCookie=myValue;max-age=50;path=%2Fapathsomewhere;domain=%2Fadomainsomewhere;secure;comment=this%20is%20a%20comment;version=0;commentURL=http%2F%3Aaurl.com;port=80,8080;discard;"; + String cookieString = "myCookie=myValue;max-age=50;path=/apathsomewhere;domain=.adomainsomewhere;secure;comment=this is a comment;version=0;commentURL=http://aurl.com;port=80,8080;discard;"; CookieDecoder cookieDecoder = new CookieDecoder(); Map cookieMap = cookieDecoder.decode(cookieString); assertEquals(1, cookieMap.size()); @@ -61,8 +61,8 @@ public class CookieDecoderTest { assertNotNull(cookie); assertEquals("myValue", cookie.getValue()); assertNull(cookie.getComment()); - assertNull(cookie.getCommentURL()); - assertEquals("/adomainsomewhere", cookie.getDomain()); + assertNull(cookie.getCommentUrl()); + assertEquals(".adomainsomewhere", cookie.getDomain()); assertFalse(cookie.isDiscard()); assertEquals(50, cookie.getMaxAge()); assertEquals("/apathsomewhere", cookie.getPath()); @@ -72,7 +72,7 @@ public class CookieDecoderTest { } @Test public void testDecodingSingleCookieV1() { - String cookieString = "myCookie=myValue;max-age=50;path=%2Fapathsomewhere;domain=%2Fadomainsomewhere;secure;comment=this%20is%20a%20comment;version=1;"; + String cookieString = "myCookie=myValue;max-age=50;path=/apathsomewhere;domain=.adomainsomewhere;secure;comment=this is a comment;version=1;"; CookieDecoder cookieDecoder = new CookieDecoder(); Map cookieMap = cookieDecoder.decode(cookieString); assertEquals(1, cookieMap.size()); @@ -80,8 +80,8 @@ public class CookieDecoderTest { assertEquals("myValue", cookie.getValue()); assertNotNull(cookie); assertEquals("this is a comment", cookie.getComment()); - assertNull(cookie.getCommentURL()); - assertEquals("/adomainsomewhere", cookie.getDomain()); + assertNull(cookie.getCommentUrl()); + assertEquals(".adomainsomewhere", cookie.getDomain()); assertFalse(cookie.isDiscard()); assertEquals(50, cookie.getMaxAge()); assertEquals("/apathsomewhere", cookie.getPath()); @@ -92,7 +92,7 @@ public class CookieDecoderTest { @Test public void testDecodingSingleCookieV1ExtraParamsIgnored() { - String cookieString = "myCookie=myValue;max-age=50;path=%2Fapathsomewhere;domain=%2Fadomainsomewhere;secure;comment=this%20is%20a%20comment;version=1;commentURL=http%2F%3Aaurl.com;port=80,8080;discard;"; + String cookieString = "myCookie=myValue;max-age=50;path=/apathsomewhere;domain=.adomainsomewhere;secure;comment=this is a comment;version=1;commentURL=http://aurl.com;port=80,8080;discard;"; CookieDecoder cookieDecoder = new CookieDecoder(); Map cookieMap = cookieDecoder.decode(cookieString); assertEquals(1, cookieMap.size()); @@ -100,8 +100,8 @@ public class CookieDecoderTest { assertNotNull(cookie); assertEquals("myValue", cookie.getValue()); assertEquals("this is a comment", cookie.getComment()); - assertNull(cookie.getCommentURL()); - assertEquals("/adomainsomewhere", cookie.getDomain()); + assertNull(cookie.getCommentUrl()); + assertEquals(".adomainsomewhere", cookie.getDomain()); assertFalse(cookie.isDiscard()); assertEquals(50, cookie.getMaxAge()); assertEquals("/apathsomewhere", cookie.getPath()); @@ -111,7 +111,7 @@ public class CookieDecoderTest { } @Test public void testDecodingSingleCookieV2() { - String cookieString = "myCookie=myValue;max-age=50;path=%2Fapathsomewhere;domain=%2Fadomainsomewhere;secure;comment=this%20is%20a%20comment;version=2;commentURL=http%2F%3Aaurl.com;port=80,8080;discard;"; + String cookieString = "myCookie=myValue;max-age=50;path=/apathsomewhere;domain=.adomainsomewhere;secure;comment=this is a comment;version=2;commentURL=http://aurl.com;port=\"80,8080\";discard;"; CookieDecoder cookieDecoder = new CookieDecoder(); Map cookieMap = cookieDecoder.decode(cookieString); assertEquals(1, cookieMap.size()); @@ -119,8 +119,8 @@ public class CookieDecoderTest { assertNotNull(cookie); assertEquals("myValue", cookie.getValue()); assertEquals("this is a comment", cookie.getComment()); - assertEquals("http/:aurl.com", cookie.getCommentURL()); - assertEquals("/adomainsomewhere", cookie.getDomain()); + assertEquals("http://aurl.com", cookie.getCommentUrl()); + assertEquals(".adomainsomewhere", cookie.getDomain()); assertTrue(cookie.isDiscard()); assertEquals(50, cookie.getMaxAge()); assertEquals("/apathsomewhere", cookie.getPath()); @@ -134,8 +134,8 @@ public class CookieDecoderTest { @Test public void testDecodingMultipleCookies() { - String c1 = "myCookie=myValue;max-age=50;path=%2Fapathsomewhere;domain=%2Fadomainsomewhere;secure;comment=this%20is%20a%20comment;version=2;commentURL=http%2F%3Aaurl.com;port=80,8080;discard;"; - String c2 = "myCookie2=myValue2;max-age=0;path=%2Fanotherpathsomewhere;domain=%2Fanotherdomainsomewhere;comment=this%20is%20another%20comment;version=2;commentURL=http%2F%3Aanotherurl.com;"; + String c1 = "myCookie=myValue;max-age=50;path=/apathsomewhere;domain=.adomainsomewhere;secure;comment=this is a comment;version=2;commentURL=\"http://aurl.com\";port='80,8080';discard;"; + String c2 = "myCookie2=myValue2;max-age=0;path=/anotherpathsomewhere;domain=.anotherdomainsomewhere;comment=this is another comment;version=2;commentURL=http://anotherurl.com;"; String c3 = "myCookie3=myValue3;max-age=0;version=2;"; CookieDecoder decoder = new CookieDecoder(); @@ -145,8 +145,8 @@ public class CookieDecoderTest { assertNotNull(cookie); assertEquals("myValue", cookie.getValue()); assertEquals("this is a comment", cookie.getComment()); - assertEquals("http/:aurl.com", cookie.getCommentURL()); - assertEquals("/adomainsomewhere", cookie.getDomain()); + assertEquals("http://aurl.com", cookie.getCommentUrl()); + assertEquals(".adomainsomewhere", cookie.getDomain()); assertTrue(cookie.isDiscard()); assertEquals(50, cookie.getMaxAge()); assertEquals("/apathsomewhere", cookie.getPath()); @@ -159,8 +159,8 @@ public class CookieDecoderTest { assertNotNull(cookie); assertEquals("myValue2", cookie.getValue()); assertEquals("this is another comment", cookie.getComment()); - assertEquals("http/:anotherurl.com", cookie.getCommentURL()); - assertEquals("/anotherdomainsomewhere", cookie.getDomain()); + assertEquals("http://anotherurl.com", cookie.getCommentUrl()); + assertEquals(".anotherdomainsomewhere", cookie.getDomain()); assertFalse(cookie.isDiscard()); assertEquals(0, cookie.getMaxAge()); assertEquals("/anotherpathsomewhere", cookie.getPath()); @@ -171,7 +171,7 @@ public class CookieDecoderTest { assertNotNull(cookie); assertEquals("myValue3", cookie.getValue()); assertNull( cookie.getComment()); - assertNull(cookie.getCommentURL()); + assertNull(cookie.getCommentUrl()); assertNull(cookie.getDomain()); assertFalse(cookie.isDiscard()); assertEquals(0, cookie.getMaxAge()); diff --git a/src/test/java/org/jboss/netty/handler/codec/http/CookieEncoderTest.java b/src/test/java/org/jboss/netty/handler/codec/http/CookieEncoderTest.java index bc10811f07..262abb7059 100644 --- a/src/test/java/org/jboss/netty/handler/codec/http/CookieEncoderTest.java +++ b/src/test/java/org/jboss/netty/handler/codec/http/CookieEncoderTest.java @@ -32,13 +32,13 @@ import org.junit.Test; public class CookieEncoderTest { @Test public void testEncodingSingleCookieV0() { - String result = "myCookie=myValue;expires=50;path=%2Fapathsomewhere;domain=%2Fadomainsomewhere;secure;"; + String result = "myCookie=myValue;expires=50;path=/apathsomewhere;domain=.adomainsomewhere;secure;"; Cookie cookie = new DefaultCookie("myCookie", "myValue"); CookieEncoder encoder = new CookieEncoder(0); encoder.addCookie(cookie); cookie.setComment("this is a comment"); - cookie.setCommentURL("http/:aurl.com"); - cookie.setDomain("/adomainsomewhere"); + cookie.setCommentUrl("http://aurl.com"); + cookie.setDomain(".adomainsomewhere"); cookie.setDiscard(true); cookie.setMaxAge(50); cookie.setPath("/apathsomewhere"); @@ -49,13 +49,13 @@ public class CookieEncoderTest { } @Test public void testEncodingSingleCookieV1() { - String result = "myCookie=myValue;max-age=50;path=%2Fapathsomewhere;domain=%2Fadomainsomewhere;secure;comment=this%20is%20a%20comment;version=1;"; + String result = "myCookie=myValue;max-age=50;path=/apathsomewhere;domain=.adomainsomewhere;secure;comment=this is a comment;version=1;"; Cookie cookie = new DefaultCookie("myCookie", "myValue"); CookieEncoder encoder = new CookieEncoder(1); encoder.addCookie(cookie); cookie.setComment("this is a comment"); - cookie.setCommentURL("http/:aurl.com"); - cookie.setDomain("/adomainsomewhere"); + cookie.setCommentUrl("http://aurl.com"); + cookie.setDomain(".adomainsomewhere"); cookie.setDiscard(true); cookie.setMaxAge(50); cookie.setPath("/apathsomewhere"); @@ -66,13 +66,13 @@ public class CookieEncoderTest { } @Test public void testEncodingSingleCookieV2() { - String result = "myCookie=myValue;max-age=50;path=%2Fapathsomewhere;domain=%2Fadomainsomewhere;secure;comment=this%20is%20a%20comment;version=1;commentURL=\"http%2F%3Aaurl.com\";port=\"80,8080\";discard;"; + String result = "myCookie=myValue;max-age=50;path=/apathsomewhere;domain=.adomainsomewhere;secure;comment=this is a comment;version=1;commentURL=\"http://aurl.com\";port=\"80,8080\";discard;"; Cookie cookie = new DefaultCookie("myCookie", "myValue"); CookieEncoder encoder = new CookieEncoder(2); encoder.addCookie(cookie); cookie.setComment("this is a comment"); - cookie.setCommentURL("http/:aurl.com"); - cookie.setDomain("/adomainsomewhere"); + cookie.setCommentUrl("http://aurl.com"); + cookie.setDomain(".adomainsomewhere"); cookie.setDiscard(true); cookie.setMaxAge(50); cookie.setPath("/apathsomewhere"); @@ -84,14 +84,14 @@ public class CookieEncoderTest { @Test public void testEncodingMultipleCookies() { - String c1 = "myCookie=myValue;max-age=50;path=%2Fapathsomewhere;domain=%2Fadomainsomewhere;secure;comment=this%20is%20a%20comment;version=1;commentURL=\"http%2F%3Aaurl.com\";port=\"80,8080\";discard;"; - String c2 = "myCookie2=myValue2;max-age=0;path=%2Fanotherpathsomewhere;domain=%2Fanotherdomainsomewhere;comment=this%20is%20another%20comment;version=1;commentURL=\"http%2F%3Aanotherurl.com\";"; + String c1 = "myCookie=myValue;max-age=50;path=/apathsomewhere;domain=.adomainsomewhere;secure;comment=this is a comment;version=1;commentURL=\"http://aurl.com\";port=\"80,8080\";discard;"; + String c2 = "myCookie2=myValue2;max-age=0;path=/anotherpathsomewhere;domain=.anotherdomainsomewhere;comment=this is another comment;version=1;commentURL=\"http://anotherurl.com\";"; String c3 = "myCookie3=myValue3;max-age=0;version=1;"; CookieEncoder encoder = new CookieEncoder(2); Cookie cookie = new DefaultCookie("myCookie", "myValue"); cookie.setComment("this is a comment"); - cookie.setCommentURL("http/:aurl.com"); - cookie.setDomain("/adomainsomewhere"); + cookie.setCommentUrl("http://aurl.com"); + cookie.setDomain(".adomainsomewhere"); cookie.setDiscard(true); cookie.setMaxAge(50); cookie.setPath("/apathsomewhere"); @@ -100,8 +100,8 @@ public class CookieEncoderTest { encoder.addCookie(cookie); Cookie cookie2 = new DefaultCookie("myCookie2", "myValue2"); cookie2.setComment("this is another comment"); - cookie2.setCommentURL("http/:anotherurl.com"); - cookie2.setDomain("/anotherdomainsomewhere"); + cookie2.setCommentUrl("http://anotherurl.com"); + cookie2.setDomain(".anotherdomainsomewhere"); cookie2.setDiscard(false); cookie2.setPath("/anotherpathsomewhere"); cookie2.setSecure(false);