Skip to content

Commit 60b500b

Browse files
huntcjroper
huntc
authored and
jroper
committed
Factored out the cookie data codec and provided tests
1 parent 9d8cd3a commit 60b500b

File tree

3 files changed

+221
-45
lines changed

3 files changed

+221
-45
lines changed

‎framework/src/play/mvc/CookieDataCodec.java

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package play.mvc;
2+
3+
import java.io.UnsupportedEncodingException;
4+
import java.net.URLDecoder;
5+
import java.net.URLEncoder;
6+
import java.util.Map;
7+
8+
/**
9+
* Provides operations around the encoding and decoding of Cookie data.
10+
*/
11+
public class CookieDataCodec {
12+
/**
13+
* @param map the map to decode data into.
14+
* @param data the data to decode.
15+
* @throws UnsupportedEncodingException
16+
*/
17+
public static void decode(Map<String, String> map, String data) throws UnsupportedEncodingException {
18+
String[] keyValues = data.split("&");
19+
for (String keyValue : keyValues) {
20+
String[] splitted = keyValue.split("=", 2);
21+
if (splitted.length == 2) {
22+
map.put(URLDecoder.decode(splitted[0], "utf-8"), URLDecoder.decode(splitted[1], "utf-8"));
23+
}
24+
}
25+
}
26+
27+
/**
28+
* @param map the data to encode.
29+
* @return the encoded data.
30+
* @throws UnsupportedEncodingException
31+
*/
32+
public static String encode(Map<String, String> map) throws UnsupportedEncodingException {
33+
StringBuilder data = new StringBuilder();
34+
String separator = "";
35+
for (Map.Entry<String, String> entry : map.entrySet()) {
36+
if (entry.getValue() != null) {
37+
data.append(separator)
38+
.append(URLEncoder.encode(entry.getKey(), "utf-8"))
39+
.append("=")
40+
.append(URLEncoder.encode(entry.getValue(), "utf-8"));
41+
separator = "&";
42+
}
43+
}
44+
return data.toString();
45+
}
46+
47+
/**
48+
* Constant time for same length String comparison, to prevent timing attacks
49+
*/
50+
public static boolean safeEquals(String a, String b) {
51+
if (a.length() != b.length()) {
52+
return false;
53+
} else {
54+
char equal = 0;
55+
for (int i = 0; i < a.length(); i++) {
56+
equal |= a.charAt(i) ^ b.charAt(i);
57+
}
58+
return equal == 0;
59+
}
60+
}
61+
}

‎framework/src/play/mvc/Scope.java

Lines changed: 5 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -33,46 +33,6 @@ public class Scope {
3333
public static final boolean SESSION_HTTPONLY = Play.configuration.getProperty("application.session.httpOnly", "false").toLowerCase().equals("true");
3434
public static final boolean SESSION_SEND_ONLY_IF_CHANGED = Play.configuration.getProperty("application.session.sendOnlyIfChanged", "false").toLowerCase().equals("true");
3535

36-
private static void parseCookieData(Map<String, String> map, String data) throws UnsupportedEncodingException {
37-
String[] keyValues = data.split("&");
38-
for (String keyValue: keyValues) {
39-
String[] splitted = keyValue.split("=", 2);
40-
if (splitted.length == 2) {
41-
map.put(URLDecoder.decode(splitted[0], "utf-8"), URLDecoder.decode(splitted[1], "utf-8"));
42-
}
43-
}
44-
}
45-
46-
private static String formatCookieData(Map<String, String> map) throws UnsupportedEncodingException {
47-
StringBuilder data = new StringBuilder();
48-
String separator = "";
49-
for (Map.Entry<String, String> entry: map.entrySet()) {
50-
if (entry.getValue() != null) {
51-
data.append(separator)
52-
.append(URLEncoder.encode(entry.getKey(), "utf-8"))
53-
.append("=")
54-
.append(URLEncoder.encode(entry.getValue(), "utf-8"));
55-
separator = "&";
56-
}
57-
}
58-
return data.toString();
59-
}
60-
61-
/**
62-
* Constant time for same length String comparison, to prevent timing attacks
63-
*/
64-
private static boolean safeEquals(String a, String b) {
65-
if (a.length() != b.length()) {
66-
return false;
67-
} else {
68-
char equal = 0;
69-
for (int i = 0; i < a.length(); i++) {
70-
equal |= a.charAt(i) ^ b.charAt(i);
71-
}
72-
return equal == 0;
73-
}
74-
}
75-
7636
/**
7737
* Flash scope
7838
*/
@@ -86,7 +46,7 @@ static Flash restore() {
8646
Flash flash = new Flash();
8747
Http.Cookie cookie = Http.Request.current().cookies.get(COOKIE_PREFIX + "_FLASH");
8848
if (cookie != null) {
89-
parseCookieData(flash.data, cookie.value);
49+
CookieDataCodec.decode(flash.data, cookie.value);
9050
}
9151
return flash;
9252
} catch (Exception e) {
@@ -106,7 +66,7 @@ void save() {
10666
return;
10767
}
10868
try {
109-
String flashData = formatCookieData(data);
69+
String flashData = CookieDataCodec.encode(data);
11070
Http.Response.current().setCookie(COOKIE_PREFIX + "_FLASH", flashData, null, "/", null, COOKIE_SECURE);
11171
} catch (Exception e) {
11272
throw new UnexpectedException("Flash serializationProblem", e);
@@ -210,8 +170,8 @@ static Session restore() {
210170
if(firstDashIndex > -1) {
211171
String sign = value.substring(0, firstDashIndex);
212172
String data = value.substring(firstDashIndex + 1);
213-
if (safeEquals(sign, Crypto.sign(data, Play.secretKey.getBytes()))) {
214-
parseCookieData(session.data, data);
173+
if (CookieDataCodec.safeEquals(sign, Crypto.sign(data, Play.secretKey.getBytes()))) {
174+
CookieDataCodec.decode(session.data, data);
215175
}
216176
}
217177
if (COOKIE_EXPIRE != null) {
@@ -289,7 +249,7 @@ void save() {
289249
return;
290250
}
291251
try {
292-
String sessionData = formatCookieData(data);
252+
String sessionData = CookieDataCodec.encode(data);
293253
String sign = Crypto.sign(sessionData, Play.secretKey.getBytes());
294254
if (COOKIE_EXPIRE == null) {
295255
Http.Response.current().setCookie(COOKIE_PREFIX + "_SESSION", sign + "-" + sessionData, null, "/", null, COOKIE_SECURE, SESSION_HTTPONLY);

‎framework/test-src/play/mvc/CookieDataCodecTest.java

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
package play.mvc;
2+
3+
import org.junit.Test;
4+
5+
import java.io.UnsupportedEncodingException;
6+
import java.net.URLEncoder;
7+
import java.util.HashMap;
8+
import java.util.Map;
9+
10+
import static org.fest.assertions.Assertions.assertThat;
11+
import static play.mvc.CookieDataCodec.decode;
12+
import static play.mvc.CookieDataCodec.encode;
13+
14+
public class CookieDataCodecTest {
15+
16+
@Test
17+
public void flash_cookies_should_bake_in_a_header_and_value() throws UnsupportedEncodingException {
18+
final Map<String, String> inMap = new HashMap<String, String>(1);
19+
inMap.put("a", "b");
20+
final String data = encode(inMap);
21+
final Map<String, String> outMap = new HashMap<String, String>(1);
22+
decode(outMap, data);
23+
assertThat(outMap.size()).isEqualTo(1);
24+
assertThat(outMap.get("a")).isEqualTo("b");
25+
}
26+
27+
@Test
28+
public void bake_in_multiple_headers_and_values() throws UnsupportedEncodingException {
29+
final Map<String, String> inMap = new HashMap<String, String>(2);
30+
inMap.put("a", "b");
31+
inMap.put("c", "d");
32+
final String data = encode(inMap);
33+
final Map<String, String> outMap = new HashMap<String, String>(1);
34+
decode(outMap, data);
35+
assertThat(outMap.size()).isEqualTo(2);
36+
assertThat(outMap.get("a")).isEqualTo("b");
37+
assertThat(outMap.get("c")).isEqualTo("d");
38+
}
39+
40+
@Test
41+
public void bake_in_a_header_an_empty_value() throws UnsupportedEncodingException {
42+
final Map<String, String> inMap = new HashMap<String, String>(1);
43+
inMap.put("a", "");
44+
final String data = encode(inMap);
45+
final Map<String, String> outMap = new HashMap<String, String>(1);
46+
decode(outMap, data);
47+
assertThat(outMap.size()).isEqualTo(1);
48+
assertThat(outMap.get("a")).isEqualTo("");
49+
}
50+
51+
@Test
52+
public void bake_in_a_header_a_Unicode_value() throws UnsupportedEncodingException {
53+
final Map<String, String> inMap = new HashMap<String, String>(1);
54+
inMap.put("a", "\u0000");
55+
final String data = encode(inMap);
56+
final Map<String, String> outMap = new HashMap<String, String>(1);
57+
decode(outMap, data);
58+
assertThat(outMap.size()).isEqualTo(1);
59+
assertThat(outMap.get("a")).isEqualTo("\u0000");
60+
}
61+
62+
@Test
63+
public void bake_in_an_empty_map() throws UnsupportedEncodingException {
64+
final Map<String, String> inMap = new HashMap<String, String>(0);
65+
final String data = encode(inMap);
66+
final Map<String, String> outMap = new HashMap<String, String>(1);
67+
decode(outMap, data);
68+
assertThat(outMap.isEmpty());
69+
}
70+
71+
@Test
72+
public void encode_values_such_that_no_extra_keys_can_be_created() throws UnsupportedEncodingException {
73+
final Map<String, String> inMap = new HashMap<String, String>(1);
74+
inMap.put("a", "b&c=d");
75+
final String data = encode(inMap);
76+
final Map<String, String> outMap = new HashMap<String, String>(1);
77+
decode(outMap, data);
78+
assertThat(outMap.size()).isEqualTo(1);
79+
assertThat(outMap.get("a")).isEqualTo("b&c=d");
80+
}
81+
82+
@Test
83+
public void specifically_exclude_control_chars() throws UnsupportedEncodingException {
84+
for (int i = 0; i < 32; ++i) {
85+
final Map<String, String> inMap = new HashMap<String, String>(1);
86+
final String s = Character.toChars(i).toString();
87+
inMap.put("a", s);
88+
final String data = encode(inMap);
89+
assertThat(data).doesNotContain(s);
90+
final Map<String, String> outMap = new HashMap<String, String>(1);
91+
decode(outMap, data);
92+
assertThat(outMap.size()).isEqualTo(1);
93+
assertThat(outMap.get("a")).isEqualTo(s);
94+
}
95+
}
96+
97+
@Test
98+
public void specifically_exclude_special_cookie_chars() throws UnsupportedEncodingException {
99+
final Map<String, String> inMap = new HashMap<String, String>(1);
100+
inMap.put("a", " \",;\\");
101+
final String data = encode(inMap);
102+
assertThat(data).doesNotContain(" ");
103+
assertThat(data).doesNotContain("\"");
104+
assertThat(data).doesNotContain(",");
105+
assertThat(data).doesNotContain(";");
106+
assertThat(data).doesNotContain("\\");
107+
final Map<String, String> outMap = new HashMap<String, String>(1);
108+
decode(outMap, data);
109+
assertThat(outMap.size()).isEqualTo(1);
110+
assertThat(outMap.get("a")).isEqualTo(" \",;\\");
111+
}
112+
113+
private String oldEncoder(final Map<String, String> out) throws UnsupportedEncodingException {
114+
StringBuilder flash = new StringBuilder();
115+
for (String key : out.keySet()) {
116+
if (out.get(key) == null) continue;
117+
flash.append("\u0000");
118+
flash.append(key);
119+
flash.append(":");
120+
flash.append(out.get(key));
121+
flash.append("\u0000");
122+
}
123+
return URLEncoder.encode(flash.toString(), "utf-8");
124+
125+
}
126+
127+
@Test
128+
public void decode_values_of_the_previously_supported_format() throws UnsupportedEncodingException {
129+
final Map<String, String> inMap = new HashMap<String, String>(2);
130+
inMap.put("a", "b");
131+
inMap.put("c", "d");
132+
final String data = oldEncoder(inMap);
133+
final Map<String, String> outMap = new HashMap<String, String>(0);
134+
decode(outMap, data);
135+
assertThat(outMap.isEmpty());
136+
}
137+
138+
@Test
139+
public void decode_values_of_the_previously_supported_format_with_the_new_delimiters_in_them() throws UnsupportedEncodingException {
140+
final Map<String, String> inMap = new HashMap<String, String>(1);
141+
inMap.put("a", "b&=");
142+
final String data = oldEncoder(inMap);
143+
final Map<String, String> outMap = new HashMap<String, String>(0);
144+
decode(outMap, data);
145+
assertThat(outMap.isEmpty());
146+
}
147+
148+
@Test
149+
public void decode_values_with_gibberish_in_them() throws UnsupportedEncodingException {
150+
final String data = "asfjdlkasjdflk";
151+
final Map<String, String> outMap = new HashMap<String, String>(1);
152+
decode(outMap, data);
153+
assertThat(outMap.isEmpty());
154+
}
155+
}

0 commit comments

Comments
 (0)