Skip to content

Commit

Permalink
Factored out the cookie data codec and provided tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@huntc @guillaumebort
huntc authored and guillaumebort committed Aug 7, 2013
1 parent 2c651c5 commit 329240e
Show file tree
Hide file tree
Showing 3 changed files with 221 additions and 45 deletions.
61 changes: 61 additions & 0 deletions framework/src/play/mvc/CookieDataCodec.java
View file
@@ -0,0 +1,61 @@
package play.mvc;

import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.net.URLEncoder;
import java.util.Map;

/**
* Provides operations around the encoding and decoding of Cookie data.
*/
public class CookieDataCodec {
/**
* @param map the map to decode data into.
* @param data the data to decode.
* @throws UnsupportedEncodingException
*/
public static void decode(Map<String, String> map, String data) throws UnsupportedEncodingException {
String[] keyValues = data.split("&");
for (String keyValue : keyValues) {
String[] splitted = keyValue.split("=", 2);
if (splitted.length == 2) {
map.put(URLDecoder.decode(splitted[0], "utf-8"), URLDecoder.decode(splitted[1], "utf-8"));
}
}
}

/**
* @param map the data to encode.
* @return the encoded data.
* @throws UnsupportedEncodingException
*/
public static String encode(Map<String, String> map) throws UnsupportedEncodingException {
StringBuilder data = new StringBuilder();
String separator = "";
for (Map.Entry<String, String> entry : map.entrySet()) {
if (entry.getValue() != null) {
data.append(separator)
.append(URLEncoder.encode(entry.getKey(), "utf-8"))
.append("=")
.append(URLEncoder.encode(entry.getValue(), "utf-8"));
separator = "&";
}
}
return data.toString();
}

/**
* Constant time for same length String comparison, to prevent timing attacks
*/
public static boolean safeEquals(String a, String b) {
if (a.length() != b.length()) {
return false;
} else {
char equal = 0;
for (int i = 0; i < a.length(); i++) {
equal |= a.charAt(i) ^ b.charAt(i);
}
return equal == 0;
}
}
}
50 changes: 5 additions & 45 deletions framework/src/play/mvc/Scope.java
View file
Expand Up @@ -33,46 +33,6 @@ public class Scope {
public static final boolean SESSION_HTTPONLY = Play.configuration.getProperty("application.session.httpOnly", "false").toLowerCase().equals("true");
public static final boolean SESSION_SEND_ONLY_IF_CHANGED = Play.configuration.getProperty("application.session.sendOnlyIfChanged", "false").toLowerCase().equals("true");

private static void parseCookieData(Map<String, String> map, String data) throws UnsupportedEncodingException {
String[] keyValues = data.split("&");
for (String keyValue: keyValues) {
String[] splitted = keyValue.split("=", 2);
if (splitted.length == 2) {
map.put(URLDecoder.decode(splitted[0], "utf-8"), URLDecoder.decode(splitted[1], "utf-8"));
}
}
}

private static String formatCookieData(Map<String, String> map) throws UnsupportedEncodingException {
StringBuilder data = new StringBuilder();
String separator = "";
for (Map.Entry<String, String> entry: map.entrySet()) {
if (entry.getValue() != null) {
data.append(separator)
.append(URLEncoder.encode(entry.getKey(), "utf-8"))
.append("=")
.append(URLEncoder.encode(entry.getValue(), "utf-8"));
separator = "&";
}
}
return data.toString();
}

/**
* Constant time for same length String comparison, to prevent timing attacks
*/
private static boolean safeEquals(String a, String b) {
if (a.length() != b.length()) {
return false;
} else {
char equal = 0;
for (int i = 0; i < a.length(); i++) {
equal |= a.charAt(i) ^ b.charAt(i);
}
return equal == 0;
}
}

/**
* Flash scope
*/
Expand All @@ -86,7 +46,7 @@ static Flash restore() {
Flash flash = new Flash();
Http.Cookie cookie = Http.Request.current().cookies.get(COOKIE_PREFIX + "_FLASH");
if (cookie != null) {
parseCookieData(flash.data, cookie.value);
CookieDataCodec.decode(flash.data, cookie.value);
}
return flash;
} catch (Exception e) {
Expand All @@ -106,7 +66,7 @@ void save() {
return;
}
try {
String flashData = formatCookieData(data);
String flashData = CookieDataCodec.encode(data);
Http.Response.current().setCookie(COOKIE_PREFIX + "_FLASH", flashData, null, "/", null, COOKIE_SECURE);
} catch (Exception e) {
throw new UnexpectedException("Flash serializationProblem", e);
Expand Down Expand Up @@ -210,8 +170,8 @@ static Session restore() {
if(firstDashIndex > -1) {
String sign = value.substring(0, firstDashIndex);
String data = value.substring(firstDashIndex + 1);
if (safeEquals(sign, Crypto.sign(data, Play.secretKey.getBytes()))) {
parseCookieData(session.data, data);
if (CookieDataCodec.safeEquals(sign, Crypto.sign(data, Play.secretKey.getBytes()))) {
CookieDataCodec.decode(session.data, data);
}
}
if (COOKIE_EXPIRE != null) {
Expand Down Expand Up @@ -289,7 +249,7 @@ void save() {
return;
}
try {
String sessionData = formatCookieData(data);
String sessionData = CookieDataCodec.encode(data);
String sign = Crypto.sign(sessionData, Play.secretKey.getBytes());
if (COOKIE_EXPIRE == null) {
Http.Response.current().setCookie(COOKIE_PREFIX + "_SESSION", sign + "-" + sessionData, null, "/", null, COOKIE_SECURE, SESSION_HTTPONLY);
Expand Down
155 changes: 155 additions & 0 deletions framework/test-src/play/mvc/CookieDataCodecTest.java
View file
@@ -0,0 +1,155 @@
package play.mvc;

import org.junit.Test;

import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
import java.util.HashMap;
import java.util.Map;

import static org.fest.assertions.Assertions.assertThat;
import static play.mvc.CookieDataCodec.decode;
import static play.mvc.CookieDataCodec.encode;

public class CookieDataCodecTest {

@Test
public void flash_cookies_should_bake_in_a_header_and_value() throws UnsupportedEncodingException {
final Map<String, String> inMap = new HashMap<String, String>(1);
inMap.put("a", "b");
final String data = encode(inMap);
final Map<String, String> outMap = new HashMap<String, String>(1);
decode(outMap, data);
assertThat(outMap.size()).isEqualTo(1);
assertThat(outMap.get("a")).isEqualTo("b");
}

@Test
public void bake_in_multiple_headers_and_values() throws UnsupportedEncodingException {
final Map<String, String> inMap = new HashMap<String, String>(2);
inMap.put("a", "b");
inMap.put("c", "d");
final String data = encode(inMap);
final Map<String, String> outMap = new HashMap<String, String>(1);
decode(outMap, data);
assertThat(outMap.size()).isEqualTo(2);
assertThat(outMap.get("a")).isEqualTo("b");
assertThat(outMap.get("c")).isEqualTo("d");
}

@Test
public void bake_in_a_header_an_empty_value() throws UnsupportedEncodingException {
final Map<String, String> inMap = new HashMap<String, String>(1);
inMap.put("a", "");
final String data = encode(inMap);
final Map<String, String> outMap = new HashMap<String, String>(1);
decode(outMap, data);
assertThat(outMap.size()).isEqualTo(1);
assertThat(outMap.get("a")).isEqualTo("");
}

@Test
public void bake_in_a_header_a_Unicode_value() throws UnsupportedEncodingException {
final Map<String, String> inMap = new HashMap<String, String>(1);
inMap.put("a", "\u0000");
final String data = encode(inMap);
final Map<String, String> outMap = new HashMap<String, String>(1);
decode(outMap, data);
assertThat(outMap.size()).isEqualTo(1);
assertThat(outMap.get("a")).isEqualTo("\u0000");
}

@Test
public void bake_in_an_empty_map() throws UnsupportedEncodingException {
final Map<String, String> inMap = new HashMap<String, String>(0);
final String data = encode(inMap);
final Map<String, String> outMap = new HashMap<String, String>(1);
decode(outMap, data);
assertThat(outMap.isEmpty());
}

@Test
public void encode_values_such_that_no_extra_keys_can_be_created() throws UnsupportedEncodingException {
final Map<String, String> inMap = new HashMap<String, String>(1);
inMap.put("a", "b&c=d");
final String data = encode(inMap);
final Map<String, String> outMap = new HashMap<String, String>(1);
decode(outMap, data);
assertThat(outMap.size()).isEqualTo(1);
assertThat(outMap.get("a")).isEqualTo("b&c=d");
}

@Test
public void specifically_exclude_control_chars() throws UnsupportedEncodingException {
for (int i = 0; i < 32; ++i) {
final Map<String, String> inMap = new HashMap<String, String>(1);
final String s = Character.toChars(i).toString();
inMap.put("a", s);
final String data = encode(inMap);
assertThat(data).doesNotContain(s);
final Map<String, String> outMap = new HashMap<String, String>(1);
decode(outMap, data);
assertThat(outMap.size()).isEqualTo(1);
assertThat(outMap.get("a")).isEqualTo(s);
}
}

@Test
public void specifically_exclude_special_cookie_chars() throws UnsupportedEncodingException {
final Map<String, String> inMap = new HashMap<String, String>(1);
inMap.put("a", " \",;\\");
final String data = encode(inMap);
assertThat(data).doesNotContain(" ");
assertThat(data).doesNotContain("\"");
assertThat(data).doesNotContain(",");
assertThat(data).doesNotContain(";");
assertThat(data).doesNotContain("\\");
final Map<String, String> outMap = new HashMap<String, String>(1);
decode(outMap, data);
assertThat(outMap.size()).isEqualTo(1);
assertThat(outMap.get("a")).isEqualTo(" \",;\\");
}

private String oldEncoder(final Map<String, String> out) throws UnsupportedEncodingException {
StringBuilder flash = new StringBuilder();
for (String key : out.keySet()) {
if (out.get(key) == null) continue;
flash.append("\u0000");
flash.append(key);
flash.append(":");
flash.append(out.get(key));
flash.append("\u0000");
}
return URLEncoder.encode(flash.toString(), "utf-8");

}

@Test
public void decode_values_of_the_previously_supported_format() throws UnsupportedEncodingException {
final Map<String, String> inMap = new HashMap<String, String>(2);
inMap.put("a", "b");
inMap.put("c", "d");
final String data = oldEncoder(inMap);
final Map<String, String> outMap = new HashMap<String, String>(0);
decode(outMap, data);
assertThat(outMap.isEmpty());
}

@Test
public void decode_values_of_the_previously_supported_format_with_the_new_delimiters_in_them() throws UnsupportedEncodingException {
final Map<String, String> inMap = new HashMap<String, String>(1);
inMap.put("a", "b&=");
final String data = oldEncoder(inMap);
final Map<String, String> outMap = new HashMap<String, String>(0);
decode(outMap, data);
assertThat(outMap.isEmpty());
}

@Test
public void decode_values_with_gibberish_in_them() throws UnsupportedEncodingException {
final String data = "asfjdlkasjdflk";
final Map<String, String> outMap = new HashMap<String, String>(1);
decode(outMap, data);
assertThat(outMap.isEmpty());
}
}

0 comments on commit 329240e

Please sign in to comment.