Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Backported session fix to 1.1.x #615

Merged
merged 2 commits into from Aug 6, 2013
Merged

Backported session fix to 1.1.x #615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d976401
Improved session and flash cookie encoding
jroper Aug 6, 2013
92c439e
Factored out the cookie data codec and provided tests
huntc Aug 6, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
61 changes: 61 additions & 0 deletions framework/src/play/mvc/CookieDataCodec.java
View file
Open in desktop
@@ -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;
}
}
}
49 changes: 12 additions & 37 deletions framework/src/play/mvc/Scope.java
View file
Open in desktop
@@ -1,12 +1,11 @@
package play.mvc;

import java.io.UnsupportedEncodingException;
import java.lang.annotation.Annotation;
import java.net.URLDecoder;
import java.net.URLEncoder;
import java.util.HashMap;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import play.Logger;
import play.Play;
Expand Down Expand Up @@ -37,18 +36,13 @@ public static class Flash {

Map<String, String> data = new HashMap<String, String>();
Map<String, String> out = new HashMap<String, String>();
static Pattern flashParser = Pattern.compile("\u0000([^:]*):([^\u0000]*)\u0000");

static Flash restore() {
try {
Flash flash = new Flash();
Http.Cookie cookie = Http.Request.current().cookies.get(COOKIE_PREFIX + "_FLASH");
if (cookie != null) {
String flashData = URLDecoder.decode(cookie.value, "utf-8");
Matcher matcher = flashParser.matcher(flashData);
while (matcher.find()) {
flash.data.put(matcher.group(1), matcher.group(2));
}
CookieDataCodec.decode(flash.data, cookie.value);
}
return flash;
} catch (Exception e) {
Expand All @@ -58,15 +52,7 @@ static Flash restore() {

void save() {
try {
StringBuilder flash = new StringBuilder();
for (String key : out.keySet()) {
flash.append("\u0000");
flash.append(key);
flash.append(":");
flash.append(out.get(key));
flash.append("\u0000");
}
String flashData = URLEncoder.encode(flash.toString(), "utf-8");
String flashData = CookieDataCodec.encode(data);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to encode the out hash instead of the data one here. Will fix it during the merge.

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 @@ -153,23 +139,20 @@ public String toString() {
*/
public static class Session {

static Pattern sessionParser = Pattern.compile("\u0000([^:]*):([^\u0000]*)\u0000");

static Session restore() {
try {
Session session = new Session();
Http.Cookie cookie = Http.Request.current().cookies.get(COOKIE_PREFIX + "_SESSION");
if (cookie != null && Play.started) {
String value = cookie.value;
String sign = value.substring(0, value.indexOf("-"));
String data = value.substring(value.indexOf("-") + 1);
if (sign.equals(Crypto.sign(data, Play.secretKey.getBytes()))) {
String sessionData = URLDecoder.decode(data, "utf-8");
Matcher matcher = sessionParser.matcher(sessionData);
while (matcher.find()) {
session.put(matcher.group(1), matcher.group(2));
}
}
int firstDashIndex = value.indexOf("-");
if(firstDashIndex > -1) {
String sign = value.substring(0, firstDashIndex);
String data = value.substring(firstDashIndex + 1);
if (CookieDataCodec.safeEquals(sign, Crypto.sign(data, Play.secretKey.getBytes()))) {
CookieDataCodec.decode(session.data, data);
}
}
if (COOKIE_EXPIRE != null) {
// Verify that the session contains a timestamp, and that it's not expired
if (!session.contains("___TS")) {
Expand Down Expand Up @@ -213,15 +196,7 @@ public String getAuthenticityToken() {

void save() {
try {
StringBuilder session = new StringBuilder();
for (String key: data.keySet()) {
session.append("\u0000");
session.append(key);
session.append(":");
session.append(data.get(key));
session.append("\u0000");
}
String sessionData = URLEncoder.encode(session.toString(), "utf-8");
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
Open in desktop
@@ -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());
}
}