Set-Cookie headers should not be combined (#8611)

Motivation:

According to the HTTP spec set-cookie headers should not be combined
because they are not using the list syntax.

Modifications:

Do not combine set-cookie headers.

Result:

Set-Cookie headers won't be combined anymore
This commit is contained in:
Julien Hoarau 2018-12-01 20:47:18 +11:00 committed by Norman Maurer
parent a0c3081d82
commit d05666ae2d
2 changed files with 58 additions and 3 deletions

View File

@ -26,6 +26,7 @@ import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import static io.netty.handler.codec.http.HttpHeaderNames.SET_COOKIE;
import static io.netty.util.AsciiString.CASE_INSENSITIVE_HASHER; import static io.netty.util.AsciiString.CASE_INSENSITIVE_HASHER;
import static io.netty.util.internal.StringUtil.COMMA; import static io.netty.util.internal.StringUtil.COMMA;
import static io.netty.util.internal.StringUtil.unescapeCsvFields; import static io.netty.util.internal.StringUtil.unescapeCsvFields;
@ -87,7 +88,7 @@ public class CombinedHttpHeaders extends DefaultHttpHeaders {
@Override @Override
public Iterator<CharSequence> valueIterator(CharSequence name) { public Iterator<CharSequence> valueIterator(CharSequence name) {
Iterator<CharSequence> itr = super.valueIterator(name); Iterator<CharSequence> itr = super.valueIterator(name);
if (!itr.hasNext()) { if (!itr.hasNext() || cannotBeCombined(name)) {
return itr; return itr;
} }
Iterator<CharSequence> unescapedItr = unescapeCsvFields(itr.next()).iterator(); Iterator<CharSequence> unescapedItr = unescapeCsvFields(itr.next()).iterator();
@ -100,7 +101,7 @@ public class CombinedHttpHeaders extends DefaultHttpHeaders {
@Override @Override
public List<CharSequence> getAll(CharSequence name) { public List<CharSequence> getAll(CharSequence name) {
List<CharSequence> values = super.getAll(name); List<CharSequence> values = super.getAll(name);
if (values.isEmpty()) { if (values.isEmpty() || cannotBeCombined(name)) {
return values; return values;
} }
if (values.size() != 1) { if (values.size() != 1) {
@ -213,9 +214,13 @@ public class CombinedHttpHeaders extends DefaultHttpHeaders {
return this; return this;
} }
private static boolean cannotBeCombined(CharSequence name) {
return SET_COOKIE.contentEqualsIgnoreCase(name);
}
private CombinedHttpHeadersImpl addEscapedValue(CharSequence name, CharSequence escapedValue) { private CombinedHttpHeadersImpl addEscapedValue(CharSequence name, CharSequence escapedValue) {
CharSequence currentValue = super.get(name); CharSequence currentValue = super.get(name);
if (currentValue == null) { if (currentValue == null || cannotBeCombined(name)) {
super.add(name, escapedValue); super.add(name, escapedValue);
} else { } else {
super.set(name, commaSeparateEscapedValues(currentValue, escapedValue)); super.set(name, commaSeparateEscapedValues(currentValue, escapedValue));

View File

@ -22,9 +22,12 @@ import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.Iterator; import java.util.Iterator;
import static io.netty.handler.codec.http.HttpHeaderNames.SET_COOKIE;
import static io.netty.util.AsciiString.contentEquals; import static io.netty.util.AsciiString.contentEquals;
import static org.hamcrest.Matchers.hasSize;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
public class CombinedHttpHeadersTest { public class CombinedHttpHeadersTest {
@ -66,6 +69,28 @@ public class CombinedHttpHeadersTest {
assertEquals("a,b,c", headers.get(HEADER_NAME).toString()); assertEquals("a,b,c", headers.get(HEADER_NAME).toString());
} }
@Test
public void dontCombineSetCookieHeaders() {
final CombinedHttpHeaders headers = newCombinedHttpHeaders();
headers.add(SET_COOKIE, "a");
final CombinedHttpHeaders otherHeaders = newCombinedHttpHeaders();
otherHeaders.add(SET_COOKIE, "b");
otherHeaders.add(SET_COOKIE, "c");
headers.add(otherHeaders);
assertThat(headers.getAll(SET_COOKIE), hasSize(3));
}
@Test
public void dontCombineSetCookieHeadersRegardlessOfCase() {
final CombinedHttpHeaders headers = newCombinedHttpHeaders();
headers.add("Set-Cookie", "a");
final CombinedHttpHeaders otherHeaders = newCombinedHttpHeaders();
otherHeaders.add("set-cookie", "b");
otherHeaders.add("SET-COOKIE", "c");
headers.add(otherHeaders);
assertThat(headers.getAll(SET_COOKIE), hasSize(3));
}
@Test @Test
public void setCombinedHeadersWhenNotEmpty() { public void setCombinedHeadersWhenNotEmpty() {
final CombinedHttpHeaders headers = newCombinedHttpHeaders(); final CombinedHttpHeaders headers = newCombinedHttpHeaders();
@ -274,6 +299,15 @@ public class CombinedHttpHeadersTest {
assertEquals(Arrays.asList("a,b,c"), headers.getAll(HEADER_NAME)); assertEquals(Arrays.asList("a,b,c"), headers.getAll(HEADER_NAME));
} }
@Test
public void getAllDontCombineSetCookie() {
final CombinedHttpHeaders headers = newCombinedHttpHeaders();
headers.add(SET_COOKIE, "a");
headers.add(SET_COOKIE, "b");
assertThat(headers.getAll(SET_COOKIE), hasSize(2));
assertEquals(Arrays.asList("a", "b"), headers.getAll(SET_COOKIE));
}
@Test @Test
public void owsTrimming() { public void owsTrimming() {
final CombinedHttpHeaders headers = newCombinedHttpHeaders(); final CombinedHttpHeaders headers = newCombinedHttpHeaders();
@ -314,6 +348,22 @@ public class CombinedHttpHeadersTest {
assertValueIterator(headers.valueCharSequenceIterator(HEADER_NAME)); assertValueIterator(headers.valueCharSequenceIterator(HEADER_NAME));
} }
@Test
public void nonCombinableHeaderIterator() {
final CombinedHttpHeaders headers = newCombinedHttpHeaders();
headers.add(SET_COOKIE, "c");
headers.add(SET_COOKIE, "b");
headers.add(SET_COOKIE, "a");
final Iterator<String> strItr = headers.valueStringIterator(SET_COOKIE);
assertTrue(strItr.hasNext());
assertEquals("a", strItr.next());
assertTrue(strItr.hasNext());
assertEquals("b", strItr.next());
assertTrue(strItr.hasNext());
assertEquals("c", strItr.next());
}
private static void assertValueIterator(Iterator<? extends CharSequence> strItr) { private static void assertValueIterator(Iterator<? extends CharSequence> strItr) {
assertTrue(strItr.hasNext()); assertTrue(strItr.hasNext());
assertEquals("a", strItr.next()); assertEquals("a", strItr.next());