Skip to content

Commit

Permalink
Fix #3433 - Multiple cookies with same name are not supported (#4390)
Browse files Browse the repository at this point in the history
* Fix 3433 - Multiple cookies with same name are not supported

Signed-off-by: tvallin <thibault.vallin@oracle.com>
  • Loading branch information
tvallin committed Feb 18, 2020
1 parent 38d0819 commit e5af7d6
Show file tree
Hide file tree
Showing 6 changed files with 139 additions and 16 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2020 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand Down Expand Up @@ -70,10 +70,7 @@ public static Map<String, Cookie> parseCookies(String header) {
value = value.substring(1, value.length() - 1);
}
if (!name.startsWith("$")) {
if (cookie != null) {
cookies.put(cookie.name, cookie.getImmutableCookie());
}

checkSimilarCookieName(cookies, cookie);
cookie = new MutableCookie(name, value);
cookie.version = version;
} else if (name.startsWith("$Version")) {
Expand All @@ -84,10 +81,26 @@ public static Map<String, Cookie> parseCookies(String header) {
cookie.domain = value;
}
}
checkSimilarCookieName(cookies, cookie);
return cookies;
}

/**
* Check if a cookie with identical name had been parsed.
* If yes, the one with the longest string will be kept
* @param cookies : Map of cookies
* @param cookie : Cookie to be checked
*/
private static void checkSimilarCookieName(Map<String, Cookie> cookies, MutableCookie cookie) {
if (cookie != null) {
cookies.put(cookie.name, cookie.getImmutableCookie());
if (cookies.containsKey(cookie.name)){
if (cookie.value.length() > cookies.get(cookie.name).getValue().length()){
cookies.put(cookie.name, cookie.getImmutableCookie());
}
} else {
cookies.put(cookie.name, cookie.getImmutableCookie());
}
}
return cookies;
}

public static Cookie parseCookie(String header) {
Expand Down Expand Up @@ -148,8 +161,6 @@ public static NewCookie parseNewCookie(String header) {
cookie.secure = true;
} else if (param.startsWith("version")) {
cookie.version = Integer.parseInt(value);
} else if (param.startsWith("domain")) {
cookie.domain = value;
} else if (param.startsWith("httponly")) {
cookie.httpOnly = true;
} else if (param.startsWith("expires")) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2020 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand All @@ -17,6 +17,7 @@
package org.glassfish.jersey.message.internal;

import java.util.Collections;
import java.util.Comparator;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
Expand All @@ -29,6 +30,7 @@
import javax.ws.rs.core.AbstractMultivaluedMap;
import javax.ws.rs.core.Configuration;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.NewCookie;
import javax.ws.rs.ext.RuntimeDelegate;
import javax.ws.rs.ext.RuntimeDelegate.HeaderDelegate;

Expand Down Expand Up @@ -298,6 +300,33 @@ public static void checkHeaderChanges(final Map<String, String> headersSnapshot,
}
}

/**
* Compare two NewCookies having the same name. See documentation RFC.
*
* @param first NewCookie to be compared.
* @param second NewCookie to be compared.
* @return the preferred NewCookie according to rules :
* - the latest maxAge.
* - if equal, compare the expiry date
* - if equal, compare name length
*/
public static NewCookie getPreferredCookie(NewCookie first, NewCookie second) {

if (first == null) {
return second;
} else if (second == null) {
return first;
}

if (first.getMaxAge() != second.getMaxAge()){
return Comparator.comparing(NewCookie::getMaxAge).compare(first, second) > 0 ? first : second;
} else if (first.getExpiry() != null && second.getExpiry() != null && !first.getExpiry().equals(second.getExpiry())) {
return Comparator.comparing(NewCookie::getExpiry).compare(first, second) > 0 ? first : second;
} else {
return first.getPath().length() > second.getPath().length() ? first : second;
}
}

/**
* Convert a message header value, represented as a general object, to it's
* string representation. If the supplied header value is {@code null},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2020 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand Down Expand Up @@ -587,7 +587,12 @@ public Map<String, NewCookie> getResponseCookies() {
for (String cookie : cookies) {
if (cookie != null) {
NewCookie newCookie = HttpHeaderReader.readNewCookie(cookie);
result.put(newCookie.getName(), newCookie);
String cookieName = newCookie.getName();
if (result.containsKey(cookieName)) {
result.put(cookieName, HeaderUtils.getPreferredCookie(result.get(cookieName), newCookie));
} else {
result.put(cookieName, newCookie);
}
}
}
return result;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012, 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2012, 2020 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand Down Expand Up @@ -456,7 +456,12 @@ public Map<String, NewCookie> getResponseCookies() {
for (String cookie : HeaderUtils.asStringList(cookies, configuration)) {
if (cookie != null) {
NewCookie newCookie = HttpHeaderReader.readNewCookie(cookie);
result.put(newCookie.getName(), newCookie);
String cookieName = newCookie.getName();
if (result.containsKey(cookieName)) {
result.put(cookieName, HeaderUtils.getPreferredCookie(result.get(cookieName), newCookie));
} else {
result.put(cookieName, newCookie);
}
}
}
return result;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 2020 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand All @@ -18,12 +18,15 @@

import java.net.URI;
import java.util.Arrays;
import java.util.Calendar;
import java.util.Collections;
import java.util.Date;
import java.util.LinkedList;
import java.util.List;

import javax.ws.rs.core.AbstractMultivaluedMap;
import javax.ws.rs.core.MultivaluedMap;
import javax.ws.rs.core.NewCookie;
import javax.ws.rs.ext.RuntimeDelegate;

import org.glassfish.jersey.message.internal.HeaderUtils;
Expand Down Expand Up @@ -180,4 +183,48 @@ public void testAsHeaderString() throws Exception {
final String result = HeaderUtils.asHeaderString(values, null);
assertEquals("value,[null]," + uri.toASCIIString(), result);
}

@Test
public void testgetPreferredCookie(){

Calendar calendar = Calendar.getInstance();
calendar.set(2000, Calendar.JANUARY, 1);
Date earlyDate = calendar.getTime();
calendar.set(2000, Calendar.JANUARY, 2);
Date laterDate = calendar.getTime();

NewCookie earlyCookie = new NewCookie("fred", "valuestring", "pathstring", "domainstring",
0, "commentstring", 0, earlyDate, false, false);
NewCookie laterCookie = new NewCookie("fred", "valuestring", "pathstring", "domainstring",
0, "commentstring", 0, laterDate, false, false);

assertEquals(laterCookie, HeaderUtils.getPreferredCookie(earlyCookie, laterCookie));

NewCookie one = new NewCookie("fred", "valuestring", "pathstring", "domainstring",
0, "commentstring", 100, null, false, false);
NewCookie second = new NewCookie("fred", "valuestring", "pathstring", "domainstring",
0, "commentstring", 10, null, false, false);

assertEquals(one, HeaderUtils.getPreferredCookie(one, second));

NewCookie longPathNewCookie = new NewCookie("fred", "valuestring", "longestpathstring",
"domainstring", 0, "commentstring", 0, null,
false, false);
NewCookie shortPathNewCookie = new NewCookie("fred", "valuestring", "shortestpath",
"domainstring", 0, "commentstring", 0, null,
false, false);

assertEquals(longPathNewCookie, HeaderUtils.getPreferredCookie(longPathNewCookie, shortPathNewCookie));

NewCookie identicalNewCookie = new NewCookie("fred", "valuestring", "pathstring",
"domainstring", 0, "commentstring", 0, null,
false, false);
NewCookie identicalNewCookie1 = new NewCookie("fred", "valuestring", "pathstring",
"domainstring", 0, "commentstring", 0, null,
false, false);

assertEquals(identicalNewCookie, HeaderUtils.getPreferredCookie(identicalNewCookie, identicalNewCookie1));

}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2014, 2018 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2014, 2020 Oracle and/or its affiliates. All rights reserved.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0, which is available at
Expand Down Expand Up @@ -148,6 +148,32 @@ public void testCreateCookies() {
assertTrue(".sun.com".equals(c.getDomain()));
}

@Test
public void testMultipleCookiesWithSameName(){

String cookieHeader = "kobe=longeststring; kobe=shortstring";
Map<String, Cookie> cookies = HttpHeaderReader.readCookies(cookieHeader);
assertEquals(cookies.size(), 1);
Cookie c = cookies.get("kobe");
assertEquals(c.getVersion(), 0);
assertEquals("kobe", c.getName());
assertEquals("longeststring", c.getValue());

cookieHeader = "bryant=longeststring; bryant=shortstring; fred=shortstring ;fred=longeststring;$Path=/path";
cookies = HttpHeaderReader.readCookies(cookieHeader);
assertEquals(cookies.size(), 2);
c = cookies.get("bryant");
assertEquals(c.getVersion(), 0);
assertEquals("bryant", c.getName());
assertEquals("longeststring", c.getValue());
c = cookies.get("fred");
assertEquals(c.getVersion(), 0);
assertEquals("fred", c.getName());
assertEquals("longeststring", c.getValue());
assertEquals("/path", c.getPath());

}

@Test
public void testNewCookieToString() {
NewCookie cookie = new NewCookie("fred", "flintstone");
Expand Down

0 comments on commit e5af7d6

Please sign in to comment.