Trim optional white space in CombinedHttpHeaders values

Motivation:

The updated HTTP/1.x RFC allows for header values to be CSV and separated by OWS [1]. CombinedHttpHeaders should remove this OWS on insertion.

[1] https://tools.ietf.org/html/rfc7230#section-7

Modification:

CombinedHttpHeaders doesn't account for the OWS and returns it back to the user as part of the value.

Result:

Fixes #6452
This commit is contained in:
David Dossot 2017-02-24 12:45:23 -08:00 committed by Norman Maurer
parent 9e6e1a3e7b
commit 9c1a191696
5 changed files with 210 additions and 28 deletions

View File

@ -16,9 +16,9 @@
package io.netty.handler.codec.http;
import io.netty.handler.codec.DefaultHeaders;
import io.netty.handler.codec.Headers;
import io.netty.handler.codec.ValueConverter;
import io.netty.util.HashingStrategy;
import io.netty.handler.codec.Headers;
import io.netty.util.internal.StringUtil;
import java.util.Collection;
@ -39,6 +39,11 @@ public class CombinedHttpHeaders extends DefaultHttpHeaders {
super(new CombinedHttpHeadersImpl(CASE_INSENSITIVE_HASHER, valueConverter(validate), nameValidator(validate)));
}
@Override
public boolean containsValue(CharSequence name, CharSequence value, boolean ignoreCase) {
return super.containsValue(name, StringUtil.trimOws(value), ignoreCase);
}
private static final class CombinedHttpHeadersImpl
extends DefaultHeaders<CharSequence, CharSequence, CombinedHttpHeadersImpl> {
/**
@ -53,7 +58,7 @@ public class CombinedHttpHeaders extends DefaultHttpHeaders {
objectEscaper = new CsvValueEscaper<Object>() {
@Override
public CharSequence escape(Object value) {
return StringUtil.escapeCsv(valueConverter().convertObject(value));
return StringUtil.escapeCsv(valueConverter().convertObject(value), true);
}
};
}
@ -65,7 +70,7 @@ public class CombinedHttpHeaders extends DefaultHttpHeaders {
charSequenceEscaper = new CsvValueEscaper<CharSequence>() {
@Override
public CharSequence escape(CharSequence value) {
return StringUtil.escapeCsv(value);
return StringUtil.escapeCsv(value, true);
}
};
}
@ -136,7 +141,7 @@ public class CombinedHttpHeaders extends DefaultHttpHeaders {
@Override
public CombinedHttpHeadersImpl add(CharSequence name, CharSequence value) {
return addEscapedValue(name, StringUtil.escapeCsv(value));
return addEscapedValue(name, charSequenceEscaper().escape(value));
}
@Override
@ -149,6 +154,11 @@ public class CombinedHttpHeaders extends DefaultHttpHeaders {
return addEscapedValue(name, commaSeparate(charSequenceEscaper(), values));
}
@Override
public CombinedHttpHeadersImpl addObject(CharSequence name, Object value) {
return addEscapedValue(name, commaSeparate(objectEscaper(), value));
}
@Override
public CombinedHttpHeadersImpl addObject(CharSequence name, Iterable<?> values) {
return addEscapedValue(name, commaSeparate(objectEscaper(), values));

View File

@ -1573,7 +1573,7 @@ public abstract class HttpHeaders implements Iterable<Map.Entry<String, String>>
/**
* Returns {@code true} if a header with the {@code name} and {@code value} exists, {@code false} otherwise.
* This also handles multiple values that are seperated with a {@code ,}.
* This also handles multiple values that are separated with a {@code ,}.
* <p>
* If {@code ignoreCase} is {@code true} then a case insensitive compare is done on the value.
* @param name the name of the header to find

View File

@ -16,6 +16,7 @@
package io.netty.handler.codec.http;
import io.netty.handler.codec.http.HttpHeadersTestUtils.HeaderValue;
import io.netty.util.internal.StringUtil;
import org.junit.Test;
import java.util.Arrays;
@ -23,6 +24,8 @@ import java.util.Collections;
import static io.netty.util.AsciiString.contentEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
public class CombinedHttpHeadersTest {
@ -271,4 +274,32 @@ public class CombinedHttpHeadersTest {
headers.set(HEADER_NAME, "\"a,b,c\"");
assertEquals(Arrays.asList("a,b,c"), headers.getAll(HEADER_NAME));
}
@Test
public void owsTrimming() {
final CombinedHttpHeaders headers = newCombinedHttpHeaders();
headers.set(HEADER_NAME, Arrays.asList("\ta", " ", " b ", "\t \t"));
headers.add(HEADER_NAME, " c, d \t");
assertEquals(Arrays.asList("a", "", "b", "", "c, d"), headers.getAll(HEADER_NAME));
assertEquals("a,,b,,\"c, d\"", headers.get(HEADER_NAME));
assertTrue(headers.containsValue(HEADER_NAME, "a", true));
assertTrue(headers.containsValue(HEADER_NAME, " a ", true));
assertTrue(headers.containsValue(HEADER_NAME, "a", true));
assertFalse(headers.containsValue(HEADER_NAME, "a,b", true));
assertFalse(headers.containsValue(HEADER_NAME, " c, d ", true));
assertFalse(headers.containsValue(HEADER_NAME, "c, d", true));
assertTrue(headers.containsValue(HEADER_NAME, " c ", true));
assertTrue(headers.containsValue(HEADER_NAME, "d", true));
assertTrue(headers.containsValue(HEADER_NAME, "\t", true));
assertTrue(headers.containsValue(HEADER_NAME, "", true));
assertFalse(headers.containsValue(HEADER_NAME, "e", true));
HttpHeaders copiedHeaders = newCombinedHttpHeaders().add(headers);
assertEquals(Arrays.asList("a", "", "b", "", "c, d"), copiedHeaders.getAll(HEADER_NAME));
}
}

View File

@ -34,6 +34,7 @@ public final class StringUtil {
public static final char LINE_FEED = '\n';
public static final char CARRIAGE_RETURN = '\r';
public static final char TAB = '\t';
public static final char SPACE = 0x20;
private static final String[] BYTE2HEX_PAD = new String[256];
private static final String[] BYTE2HEX_NOPAD = new String[256];
@ -48,16 +49,16 @@ public final class StringUtil {
static {
// Generate the lookup table that converts a byte into a 2-digit hexadecimal integer.
int i;
for (i = 0; i < 10; i ++) {
for (i = 0; i < 10; i++) {
BYTE2HEX_PAD[i] = "0" + i;
BYTE2HEX_NOPAD[i] = String.valueOf(i);
}
for (; i < 16; i ++) {
for (; i < 16; i++) {
char c = (char) ('a' + i - 10);
BYTE2HEX_PAD[i] = "0" + c;
BYTE2HEX_NOPAD[i] = String.valueOf(c);
}
for (; i < BYTE2HEX_PAD.length; i ++) {
for (; i < BYTE2HEX_PAD.length; i++) {
String str = Integer.toHexString(i);
BYTE2HEX_PAD[i] = str;
BYTE2HEX_NOPAD[i] = str;
@ -84,8 +85,8 @@ public final class StringUtil {
/**
* Checks if two strings have the same suffix of specified length
*
* @param s string
* @param p string
* @param s string
* @param p string
* @param len length of the common suffix
* @return true if both s and p are not null and both have the same suffix. Otherwise - false
*/
@ -138,7 +139,7 @@ public final class StringUtil {
*/
public static <T extends Appendable> T toHexStringPadded(T dst, byte[] src, int offset, int length) {
final int end = offset + length;
for (int i = offset; i < end; i ++) {
for (int i = offset; i < end; i++) {
byteToHexStringPadded(dst, src[i]);
}
return dst;
@ -198,13 +199,13 @@ public final class StringUtil {
int i;
// Skip preceding zeroes.
for (i = offset; i < endMinusOne; i ++) {
for (i = offset; i < endMinusOne; i++) {
if (src[i] != 0) {
break;
}
}
byteToHexString(dst, src[i ++]);
byteToHexString(dst, src[i++]);
int remaining = end - i;
toHexStringPadded(dst, src, i, remaining);
@ -244,22 +245,51 @@ public final class StringUtil {
* @return {@link CharSequence} the escaped value if necessary, or the value unchanged
*/
public static CharSequence escapeCsv(CharSequence value) {
return escapeCsv(value, false);
}
/**
* Escapes the specified value, if necessary according to
* <a href="https://tools.ietf.org/html/rfc4180#section-2">RFC-4180</a>.
*
* @param value The value which will be escaped according to
* <a href="https://tools.ietf.org/html/rfc4180#section-2">RFC-4180</a>
* @param trimWhiteSpace The value will first be trimmed of its optional white-space characters,
* according to <a href="https://tools.ietf.org/html/rfc7230#section-7">RFC-7230</a>
* @return {@link CharSequence} the escaped value if necessary, or the value unchanged
*/
public static CharSequence escapeCsv(CharSequence value, boolean trimWhiteSpace) {
int length = checkNotNull(value, "value").length();
if (length == 0) {
return value;
}
int start = 0;
int last = length - 1;
boolean quoted = isDoubleQuote(value.charAt(0)) && isDoubleQuote(value.charAt(last)) && length != 1;
boolean trimmed = false;
if (trimWhiteSpace) {
start = indexOfFirstNonOwsChar(value, length);
if (start == length) {
return EMPTY_STRING;
}
last = indexOfLastNonOwsChar(value, start, length);
trimmed = start > 0 || last < length - 1;
if (trimmed) {
length = last - start + 1;
}
}
StringBuilder result = new StringBuilder(length + CSV_NUMBER_ESCAPE_CHARACTERS);
boolean quoted = isDoubleQuote(value.charAt(start)) && isDoubleQuote(value.charAt(last)) && length != 1;
boolean foundSpecialCharacter = false;
boolean escapedDoubleQuote = false;
StringBuilder escaped = new StringBuilder(length + CSV_NUMBER_ESCAPE_CHARACTERS).append(DOUBLE_QUOTE);
for (int i = 0; i < length; i++) {
for (int i = start; i <= last; i++) {
char current = value.charAt(i);
switch (current) {
case DOUBLE_QUOTE:
if (i == 0 || i == last) {
if (i == start || i == last) {
if (!quoted) {
escaped.append(DOUBLE_QUOTE);
result.append(DOUBLE_QUOTE);
} else {
continue;
}
@ -267,7 +297,7 @@ public final class StringUtil {
boolean isNextCharDoubleQuote = isDoubleQuote(value.charAt(i + 1));
if (!isDoubleQuote(value.charAt(i - 1)) &&
(!isNextCharDoubleQuote || i + 1 == last)) {
escaped.append(DOUBLE_QUOTE);
result.append(DOUBLE_QUOTE);
escapedDoubleQuote = true;
}
break;
@ -277,10 +307,20 @@ public final class StringUtil {
case COMMA:
foundSpecialCharacter = true;
}
escaped.append(current);
result.append(current);
}
return escapedDoubleQuote || foundSpecialCharacter && !quoted ?
escaped.append(DOUBLE_QUOTE) : value;
if (escapedDoubleQuote || foundSpecialCharacter && !quoted) {
return quote(result);
}
if (trimmed) {
return quoted ? quote(result) : result;
}
return value;
}
private static StringBuilder quote(StringBuilder builder) {
return builder.insert(0, DOUBLE_QUOTE).append(DOUBLE_QUOTE);
}
/**
@ -390,7 +430,7 @@ public final class StringUtil {
return unescaped;
}
/**s
/**
* Validate if {@code value} is a valid csv field without double-quotes.
*
* @throws IllegalArgumentException if {@code value} needs to be encoded with double-quotes.
@ -430,7 +470,8 @@ public final class StringUtil {
/**
* Find the index of the first non-white space character in {@code s} starting at {@code offset}.
* @param seq The string to search.
*
* @param seq The string to search.
* @param offset The offset to start searching at.
* @return the index of the first non-white space character or &lt;{@code 0} if none was found.
*/
@ -446,6 +487,7 @@ public final class StringUtil {
/**
* Determine if {@code c} lies within the range of values defined for
* <a href="http://unicode.org/glossary/#surrogate_code_point">Surrogate Code Point</a>.
*
* @param c the character to check.
* @return {@code true} if {@code c} lies within the range of values defined for
* <a href="http://unicode.org/glossary/#surrogate_code_point">Surrogate Code Point</a>. {@code false} otherwise.
@ -469,4 +511,47 @@ public final class StringUtil {
int len = s.length();
return len > 0 && s.charAt(len - 1) == c;
}
/**
* Trim optional white-space characters from the specified value,
* according to <a href="https://tools.ietf.org/html/rfc7230#section-7">RFC-7230</a>.
*
* @param value the value to trim
* @return {@link CharSequence} the trimmed value if necessary, or the value unchanged
*/
public static CharSequence trimOws(CharSequence value) {
final int length = value.length();
if (length == 0) {
return value;
}
int start = indexOfFirstNonOwsChar(value, length);
int end = indexOfLastNonOwsChar(value, start, length);
return start == 0 && end == length - 1 ? value : value.subSequence(start, end + 1);
}
/**
* @return {@code length} if no OWS is found.
*/
private static int indexOfFirstNonOwsChar(CharSequence value, int length) {
int i = 0;
while (i < length && isOws(value.charAt(i))) {
i++;
}
return i;
}
/**
* @return {@code start} if no OWS is found.
*/
private static int indexOfLastNonOwsChar(CharSequence value, int start, int length) {
int i = length - 1;
while (i > start && isOws(value.charAt(i))) {
i--;
}
return i;
}
private static boolean isOws(char c) {
return c == SPACE || c == TAB;
}
}

View File

@ -19,9 +19,22 @@ import org.junit.Test;
import java.util.Arrays;
import static io.netty.util.internal.StringUtil.*;
import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;
import static io.netty.util.internal.StringUtil.NEWLINE;
import static io.netty.util.internal.StringUtil.commonSuffixOfLength;
import static io.netty.util.internal.StringUtil.simpleClassName;
import static io.netty.util.internal.StringUtil.substringAfter;
import static io.netty.util.internal.StringUtil.toHexString;
import static io.netty.util.internal.StringUtil.toHexStringPadded;
import static io.netty.util.internal.StringUtil.unescapeCsv;
import static io.netty.util.internal.StringUtil.unescapeCsvFields;
import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
public class StringUtilTest {
@ -331,13 +344,39 @@ public class StringUtilTest {
}
private static void escapeCsv(CharSequence value, CharSequence expected) {
escapeCsv(value, expected, false);
}
private static void escapeCsvWithTrimming(CharSequence value, CharSequence expected) {
escapeCsv(value, expected, true);
}
private static void escapeCsv(CharSequence value, CharSequence expected, boolean trimOws) {
CharSequence escapedValue = value;
for (int i = 0; i < 10; ++i) {
escapedValue = StringUtil.escapeCsv(escapedValue);
escapedValue = StringUtil.escapeCsv(escapedValue, trimOws);
assertEquals(expected, escapedValue.toString());
}
}
@Test
public void escapeCsvWithTrimming() {
assertSame("", StringUtil.escapeCsv("", true));
assertSame("ab", StringUtil.escapeCsv("ab", true));
escapeCsvWithTrimming("", "");
escapeCsvWithTrimming(" \t ", "");
escapeCsvWithTrimming("ab", "ab");
escapeCsvWithTrimming("a b", "a b");
escapeCsvWithTrimming(" \ta \tb", "a \tb");
escapeCsvWithTrimming("a \tb \t", "a \tb");
escapeCsvWithTrimming("\t a \tb \t", "a \tb");
escapeCsvWithTrimming("\"\t a b \"", "\"\t a b \"");
escapeCsvWithTrimming(" \"\t a b \"\t", "\"\t a b \"");
escapeCsvWithTrimming(" testing\t\n ", "\"testing\t\n\"");
escapeCsvWithTrimming("\ttest,ing ", "\"test,ing\"");
}
@Test
public void testUnescapeCsv() {
assertEquals("", unescapeCsv(""));
@ -465,4 +504,21 @@ public class StringUtilTest {
assertFalse(StringUtil.endsWith("-", 'u'));
assertFalse(StringUtil.endsWith("u-", 'u'));
}
@Test
public void trimOws() {
assertSame("", StringUtil.trimOws(""));
assertEquals("", StringUtil.trimOws(" \t "));
assertSame("a", StringUtil.trimOws("a"));
assertEquals("a", StringUtil.trimOws(" a"));
assertEquals("a", StringUtil.trimOws("a "));
assertEquals("a", StringUtil.trimOws(" a "));
assertSame("abc", StringUtil.trimOws("abc"));
assertEquals("abc", StringUtil.trimOws("\tabc"));
assertEquals("abc", StringUtil.trimOws("abc\t"));
assertEquals("abc", StringUtil.trimOws("\tabc\t"));
assertSame("a\t b", StringUtil.trimOws("a\t b"));
assertEquals("", StringUtil.trimOws("\t ").toString());
assertEquals("a b", StringUtil.trimOws("\ta b \t").toString());
}
}