andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Drop broken DefaultCookie name validation, close #4999

Browse Source 
Motivation:

DefaultCookie constructor performs a name validation that doesn’t match
RFC6265. Moreover, such validation is already performed in strict
encoders and decoders.

Modifications:

Drop DefaultCookie name validation, rely on encoders and decoders.

Result:

no more duplicate broken validation
This commit is contained in:
Stephane Landelle 2016-03-18 17:40:28 +01:00 committed by Norman Maurer
parent 48506f5b05
commit 881ff3cd98
3 changed files with 83 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
codec-http/src/main/java/io/netty/handler/codec/http/cookie/CookieUtil.java
View File

@ -22,9 +22,29 @@ import java.util.BitSet;
final class CookieUtil {
private static final BitSet VALID_COOKIE_NAME_OCTETS = validCookieNameOctets();
private static final BitSet VALID_COOKIE_VALUE_OCTETS = validCookieValueOctets();
private static final BitSet VALID_COOKIE_NAME_OCTETS = validCookieNameOctets();
private static final BitSet VALID_COOKIE_ATTRIBUTE_VALUE_OCTETS = validCookieAttributeValueOctets();
// token = 1*<any CHAR except CTLs or separators>
// separators = "(" | ")" | "<" | ">" | "@"
// | "," | ";" | ":" | "\" | <">
// | "/" | "[" | "]" | "?" | "="
// | "{" | "}" | SP | HT
private static BitSet validCookieNameOctets() {
BitSet bits = new BitSet();
for (int i = 32; i < 127; i++) {
bits.set(i);
}
int[] separators = new int[]
{ '(', ')', '<', '>', '@', ',', ';', ':', '\\', '"', '/', '[', ']', '?', '=', '{', '}', ' ', '\t' };
for (int separator : separators) {
bits.set(separator, false);
}
return bits;
}
// cookie-octet = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
// US-ASCII characters excluding CTLs, whitespace, DQUOTE, comma, semicolon, and backslash
@ -46,21 +66,13 @@ final class CookieUtil {
return bits;
}
// token = 1*<any CHAR except CTLs or separators>
// separators = "(" | ")" | "<" | ">" | "@"
// | "," | ";" | ":" | "\" | <">
// | "/" | "[" | "]" | "?" | "="
// | "{" | "}" | SP | HT
private static BitSet validCookieNameOctets() {
// path-value = <any CHAR except CTLs or ";">
private static BitSet validCookieAttributeValueOctets() {
BitSet bits = new BitSet();
for (int i = 32; i < 127; i++) {
bits.set(i);
}
int[] separators = new int[]
{ '(', ')', '<', '>', '@', ',', ';', ':', '\\', '"', '/', '[', ']', '?', '=', '{', '}', ' ', '\t' };
for (int separator : separators) {
bits.set(separator, false);
}
bits.set(';', false);
return bits;
}
@ -150,6 +162,21 @@ final class CookieUtil {
return cs;
}
static String validateAttributeValue(String name, String value) {
if (value == null) {
return null;
}
value = value.trim();
if (value.isEmpty()) {
return null;
}
int i = firstInvalidOctet(value, VALID_COOKIE_ATTRIBUTE_VALUE_OCTETS);
if (i != -1) {
throw new IllegalArgumentException(name + " contains the prohibited characters: " + value.charAt(i));
}
return value;
}
private CookieUtil() {
// Unused
}

61
codec-http/src/main/java/io/netty/handler/codec/http/cookie/DefaultCookie.java
View File

@ -15,7 +15,7 @@
*/
package io.netty.handler.codec.http.cookie;
import static io.netty.handler.codec.http.cookie.CookieUtil.stringBuilder;
import static io.netty.handler.codec.http.cookie.CookieUtil.*;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
/**
@ -40,28 +40,6 @@ public class DefaultCookie implements Cookie {
if (name.isEmpty()) {
throw new IllegalArgumentException("empty name");
}
for (int i = 0; i < name.length(); i ++) {
char c = name.charAt(i);
if (c > 127) {
throw new IllegalArgumentException(
"name contains non-ascii character: " + name);
}
// Check prohibited characters.
switch (c) {
case '\t': case '\n': case 0x0b: case '\f': case '\r':
case ' ': case ',': case ';': case '=':
throw new IllegalArgumentException(
"name contains one of the following prohibited characters: " +
"=,; \\t\\r\\n\\v\\f: " + name);
}
}
if (name.charAt(0) == '$') {
throw new IllegalArgumentException("name starting with '$' not allowed: " + name);
}
this.name = name;
setValue(value);
}
@ -98,7 +76,7 @@ public class DefaultCookie implements Cookie {
@Override
public void setDomain(String domain) {
this.domain = validateValue("domain", domain);
this.domain = validateAttributeValue("domain", domain);
}
@Override
@ -108,7 +86,7 @@ public class DefaultCookie implements Cookie {
@Override
public void setPath(String path) {
this.path = validateValue("path", path);
this.path = validateAttributeValue("path", path);
}
@Override
@ -218,6 +196,19 @@ public class DefaultCookie implements Cookie {
return 0;
}
/**
* Validate a cookie attribute value, throws a {@link IllegalArgumentException} otherwise.
* Only intended to be used by {@link io.netty.handler.codec.http.DefaultCookie}.
* @param name attribute name
* @param value attribute value
* @return the trimmed, validated attribute value
* @deprecated CookieUtil is package private, will be removed once old Cookie API is dropped
*/
@Deprecated
protected String validateValue(String name, String value) {
return validateAttributeValue(name, value);
}
@Override
public String toString() {
StringBuilder buf = stringBuilder()
@ -245,24 +236,4 @@ public class DefaultCookie implements Cookie {
}
return buf.toString();
}
protected String validateValue(String name, String value) {
if (value == null) {
return null;
}
value = value.trim();
if (value.isEmpty()) {
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(
name + " contains one of the following prohibited characters: " +
";\\r\\n\\f\\v (" + value + ')');
}
}
return value;
}
}

31
codec-http/src/test/java/io/netty/handler/codec/http/cookie/ServerCookieEncoderTest.java
View File

@ -79,8 +79,34 @@ public class ServerCookieEncoderTest {
}
@Test
public void illegalCharInCookieValueMakesStrictEncoderThrowsException() {
public void illegalCharInCookieNameMakesStrictEncoderThrowsException() {
Set<Character> illegalChars = new HashSet<Character>();
// CTLs
for (int i = 0x00; i <= 0x1F; i++) {
illegalChars.add((char) i);
}
illegalChars.add((char) 0x7F);
// separators
for (char c : new char[] { '(', ')', '<', '>', '@', ',', ';', ':', '\\', '"', '/', '[', ']',
'?', '=', '{', '}', ' ', '\t' }) {
illegalChars.add(c);
}
int exceptions = 0;
for (char c : illegalChars) {
try {
ServerCookieEncoder.STRICT.encode(new DefaultCookie("foo" + c + "bar", "value"));
} catch (IllegalArgumentException e) {
exceptions++;
}
}
assertEquals(illegalChars.size(), exceptions);
}
@Test
public void illegalCharInCookieValueMakesStrictEncoderThrowsException() {
Set<Character> illegalChars = new HashSet<Character>();
// CTLs
for (int i = 0x00; i <= 0x1F; i++) {
@ -95,9 +121,8 @@ public class ServerCookieEncoderTest {
int exceptions = 0;
for (char c : illegalChars) {
Cookie cookie = new DefaultCookie("name", "value" + c);
try {
ServerCookieEncoder.STRICT.encode(cookie);
ServerCookieEncoder.STRICT.encode(new DefaultCookie("name", "value" + c));
} catch (IllegalArgumentException e) {
exceptions++;
}