HttpConversionUtil does not account for COOKIE compression

Motivation:
The HTTP/2 RFC allows for COOKIE values to be split into individual header elements to get more benefit from compression (https://tools.ietf.org/html/rfc7540#section-8.1.2.5). HttpConversionUtil was not accounting for this behavior.

Modifications:
- Modify HttpConversionUtil to support compressing and decompressing the COOKIE values

Result:
HttpConversionUtil is compatible with https://tools.ietf.org/html/rfc7540#section-8.1.2.5)
Fixes https://github.com/netty/netty/issues/4457
This commit is contained in:
Scott Mitchell 2015-12-03 16:48:12 -08:00
parent 2fefb2f79c
commit 6257091d12
7 changed files with 192 additions and 22 deletions

View File

@ -14,17 +14,16 @@
*/
package io.netty.handler.codec.http2;
import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
import static io.netty.handler.codec.http2.Http2Exception.connectionError;
import static io.netty.util.AsciiString.CASE_SENSITIVE_HASHER;
import static io.netty.util.AsciiString.isUpperCase;
import io.netty.handler.codec.CharSequenceValueConverter;
import io.netty.handler.codec.DefaultHeaders;
import io.netty.util.AsciiString;
import io.netty.util.ByteProcessor;
import io.netty.util.internal.PlatformDependent;
import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
import static io.netty.handler.codec.http2.Http2Exception.connectionError;
import static io.netty.util.AsciiString.CASE_SENSITIVE_HASHER;
import static io.netty.util.AsciiString.isUpperCase;
public class DefaultHttp2Headers
extends DefaultHeaders<CharSequence, CharSequence, Http2Headers> implements Http2Headers {
private static final ByteProcessor HTTP2_NAME_VALIDATOR_PROCESSOR = new ByteProcessor() {
@ -113,6 +112,20 @@ public class DefaultHttp2Headers
return super.clear();
}
@Override
public boolean equals(Object o) {
if (!(o instanceof Http2Headers)) {
return false;
}
return equals((Http2Headers) o, CASE_SENSITIVE_HASHER);
}
@Override
public int hashCode() {
return hashCode(CASE_SENSITIVE_HASHER);
}
@Override
public Http2Headers method(CharSequence value) {
set(PseudoHeaderName.METHOD.value(), value);

View File

@ -14,6 +14,18 @@
*/
package io.netty.handler.codec.http2;
import static io.netty.handler.codec.http.HttpScheme.HTTP;
import static io.netty.handler.codec.http.HttpScheme.HTTPS;
import static io.netty.handler.codec.http.HttpUtil.isAsteriskForm;
import static io.netty.handler.codec.http.HttpUtil.isOriginForm;
import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
import static io.netty.handler.codec.http2.Http2Exception.connectionError;
import static io.netty.handler.codec.http2.Http2Exception.streamError;
import static io.netty.util.AsciiString.EMPTY_STRING;
import static io.netty.util.ByteProcessor.FIND_SEMI_COLON;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
import static io.netty.util.internal.StringUtil.isNullOrEmpty;
import static io.netty.util.internal.StringUtil.length;
import io.netty.handler.codec.http.DefaultFullHttpRequest;
import io.netty.handler.codec.http.DefaultFullHttpResponse;
import io.netty.handler.codec.http.FullHttpMessage;
@ -35,18 +47,6 @@ import java.net.URI;
import java.util.Iterator;
import java.util.Map.Entry;
import static io.netty.handler.codec.http.HttpScheme.HTTP;
import static io.netty.handler.codec.http.HttpScheme.HTTPS;
import static io.netty.handler.codec.http.HttpUtil.isAsteriskForm;
import static io.netty.handler.codec.http.HttpUtil.isOriginForm;
import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
import static io.netty.handler.codec.http2.Http2Exception.connectionError;
import static io.netty.handler.codec.http2.Http2Exception.streamError;
import static io.netty.util.AsciiString.EMPTY_STRING;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
import static io.netty.util.internal.StringUtil.isNullOrEmpty;
import static io.netty.util.internal.StringUtil.length;
/**
* Provides utility methods and constants for the HTTP/2 to HTTP conversion
*/
@ -330,9 +330,33 @@ public final class HttpConversionUtil {
final AsciiString aName = AsciiString.of(entry.getKey()).toLowerCase();
if (!HTTP_TO_HTTP2_HEADER_BLACKLIST.contains(aName)) {
// https://tools.ietf.org/html/rfc7540#section-8.1.2.2 makes a special exception for TE
if (!aName.contentEqualsIgnoreCase(HttpHeaderNames.TE) ||
AsciiString.contentEqualsIgnoreCase(entry.getValue(), HttpHeaderValues.TRAILERS)) {
out.add(aName, AsciiString.of(entry.getValue()));
if (aName.contentEqualsIgnoreCase(HttpHeaderNames.TE) &&
!AsciiString.contentEqualsIgnoreCase(entry.getValue(), HttpHeaderValues.TRAILERS)) {
throw new IllegalArgumentException("Invalid value for " + HttpHeaderNames.TE + ": " +
entry.getValue());
}
if (aName.contentEqualsIgnoreCase(HttpHeaderNames.COOKIE)) {
AsciiString value = AsciiString.of(entry.getValue());
// split up cookies to allow for better compression
// https://tools.ietf.org/html/rfc7540#section-8.1.2.5
int index = value.forEachByte(FIND_SEMI_COLON);
if (index != -1) {
int start = 0;
do {
out.add(HttpHeaderNames.COOKIE, value.subSequence(start, index, false));
// skip 2 characters "; " (see https://tools.ietf.org/html/rfc6265#section-4.2.1)
start = index + 2;
} while (start < value.length() &&
(index = value.forEachByte(start, value.length() - start, FIND_SEMI_COLON)) != -1);
if (start >= value.length()) {
throw new IllegalArgumentException("cookie value is of unexpected format: " + value);
}
out.add(HttpHeaderNames.COOKIE, value.subSequence(start, value.length(), false));
} else {
out.add(HttpHeaderNames.COOKIE, value);
}
} else {
out.add(aName, entry.getValue());
}
}
}
@ -449,7 +473,15 @@ public final class HttpConversionUtil {
throw streamError(streamId, PROTOCOL_ERROR,
"Invalid HTTP/2 header '%s' encountered in translation to HTTP/1.x", name);
}
output.add(AsciiString.of(name), AsciiString.of(value));
if (HttpHeaderNames.COOKIE.equals(name)) {
// combine the cookie values into 1 header entry.
// https://tools.ietf.org/html/rfc7540#section-8.1.2.5
String existingCookie = output.get(HttpHeaderNames.COOKIE);
output.set(HttpHeaderNames.COOKIE,
(existingCookie != null) ? (existingCookie + "; " + value) : value);
} else {
output.add(name, value);
}
}
}
}

View File

@ -141,6 +141,27 @@ public class HttpToHttp2ConnectionHandlerTest {
verifyHeadersOnly(http2Headers, writePromise, clientChannel.writeAndFlush(request, writePromise));
}
@Test
public void testMultipleCookieEntriesAreCombined() throws Exception {
bootstrapEnv(2, 1, 0);
final FullHttpRequest request = new DefaultFullHttpRequest(HTTP_1_1, GET,
"http://my-user_name@www.example.org:5555/example");
final HttpHeaders httpHeaders = request.headers();
httpHeaders.setInt(HttpConversionUtil.ExtensionHeaderNames.STREAM_ID.text(), 5);
httpHeaders.set(HttpHeaderNames.HOST, "my-user_name@www.example.org:5555");
httpHeaders.set(HttpConversionUtil.ExtensionHeaderNames.SCHEME.text(), "http");
httpHeaders.set(HttpHeaderNames.COOKIE, "a=b; c=d; e=f");
final Http2Headers http2Headers =
new DefaultHttp2Headers().method(new AsciiString("GET")).path(new AsciiString("/example"))
.authority(new AsciiString("www.example.org:5555")).scheme(new AsciiString("http"))
.add(HttpHeaderNames.COOKIE, "a=b")
.add(HttpHeaderNames.COOKIE, "c=d")
.add(HttpHeaderNames.COOKIE, "e=f");
ChannelPromise writePromise = newPromise();
verifyHeadersOnly(http2Headers, writePromise, clientChannel.writeAndFlush(request, writePromise));
}
@Test
public void testOriginFormRequestTargetHandled() throws Exception {
bootstrapEnv(2, 1, 0);

View File

@ -158,6 +158,75 @@ public class InboundHttp2ToHttpAdapterTest {
}
}
@Test
public void clientRequestSingleHeaderCookieSplitIntoMultipleEntries() throws Exception {
boostrapEnv(1, 1, 1);
final FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET,
"/some/path/resource2", true);
try {
HttpHeaders httpHeaders = request.headers();
httpHeaders.set(HttpConversionUtil.ExtensionHeaderNames.SCHEME.text(), "https");
httpHeaders.set(HttpHeaderNames.HOST, "example.org");
httpHeaders.setInt(HttpConversionUtil.ExtensionHeaderNames.STREAM_ID.text(), 3);
httpHeaders.setInt(HttpHeaderNames.CONTENT_LENGTH, 0);
httpHeaders.set(HttpHeaderNames.COOKIE, "a=b; c=d; e=f");
final Http2Headers http2Headers = new DefaultHttp2Headers().method(new AsciiString("GET")).
scheme(new AsciiString("https")).authority(new AsciiString("example.org"))
.path(new AsciiString("/some/path/resource2"))
.add(HttpHeaderNames.COOKIE, "a=b")
.add(HttpHeaderNames.COOKIE, "c=d")
.add(HttpHeaderNames.COOKIE, "e=f");
runInChannel(clientChannel, new Http2Runnable() {
@Override
public void run() {
frameWriter.writeHeaders(ctxClient(), 3, http2Headers, 0, true, newPromiseClient());
ctxClient().flush();
}
});
awaitRequests();
ArgumentCaptor<FullHttpMessage> requestCaptor = ArgumentCaptor.forClass(FullHttpMessage.class);
verify(serverListener).messageReceived(requestCaptor.capture());
capturedRequests = requestCaptor.getAllValues();
assertEquals(request, capturedRequests.get(0));
} finally {
request.release();
}
}
@Test
public void clientRequestSingleHeaderCookieSplitIntoMultipleEntries2() throws Exception {
boostrapEnv(1, 1, 1);
final FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET,
"/some/path/resource2", true);
try {
HttpHeaders httpHeaders = request.headers();
httpHeaders.set(HttpConversionUtil.ExtensionHeaderNames.SCHEME.text(), "https");
httpHeaders.set(HttpHeaderNames.HOST, "example.org");
httpHeaders.setInt(HttpConversionUtil.ExtensionHeaderNames.STREAM_ID.text(), 3);
httpHeaders.setInt(HttpHeaderNames.CONTENT_LENGTH, 0);
httpHeaders.set(HttpHeaderNames.COOKIE, "a=b; c=d; e=f");
final Http2Headers http2Headers = new DefaultHttp2Headers().method(new AsciiString("GET")).
scheme(new AsciiString("https")).authority(new AsciiString("example.org"))
.path(new AsciiString("/some/path/resource2"))
.add(HttpHeaderNames.COOKIE, "a=b; c=d")
.add(HttpHeaderNames.COOKIE, "e=f");
runInChannel(clientChannel, new Http2Runnable() {
@Override
public void run() {
frameWriter.writeHeaders(ctxClient(), 3, http2Headers, 0, true, newPromiseClient());
ctxClient().flush();
}
});
awaitRequests();
ArgumentCaptor<FullHttpMessage> requestCaptor = ArgumentCaptor.forClass(FullHttpMessage.class);
verify(serverListener).messageReceived(requestCaptor.capture());
capturedRequests = requestCaptor.getAllValues();
assertEquals(request, capturedRequests.get(0));
} finally {
request.release();
}
}
@Test
public void clientRequestSingleHeaderNonAsciiShouldThrow() throws Exception {
boostrapEnv(1, 1, 1);

View File

@ -276,7 +276,7 @@ public final class AsciiString implements CharSequence, Comparable<CharSequence>
}
private int forEachByte0(int index, int length, ByteProcessor visitor) throws Exception {
final int len = offset + length;
final int len = offset + index + length;
for (int i = offset + index; i < len; ++i) {
if (!visitor.process(value[i])) {
return i - offset;

View File

@ -120,6 +120,16 @@ public interface ByteProcessor {
}
};
/**
* Aborts on a {@code CR (';')}.
*/
ByteProcessor FIND_SEMI_COLON = new ByteProcessor() {
@Override
public boolean process(byte value) throws Exception {
return value != ';';
}
};
/**
* @return {@code true} if the processor wants to continue the loop and handle the next byte in the buffer.
* {@code false} if the processor wants to stop handling bytes and abort the loop.

View File

@ -17,6 +17,7 @@ package io.netty.util;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import io.netty.util.ByteProcessor.IndexOfProcessor;
import java.util.Random;
import java.util.concurrent.atomic.AtomicReference;
@ -102,6 +103,18 @@ public class AsciiStringMemoryTest {
assertEquals(bAsciiString.length(), bCount.get().intValue());
}
@Test
public void forEachWithIndexEndTest() throws Exception {
assertNotEquals(-1, aAsciiString.forEachByte(aAsciiString.length() - 1,
1, new IndexOfProcessor(aAsciiString.byteAt(aAsciiString.length() - 1))));
}
@Test
public void forEachWithIndexBeginTest() throws Exception {
assertNotEquals(-1, aAsciiString.forEachByte(0,
1, new IndexOfProcessor(aAsciiString.byteAt(0))));
}
@Test
public void forEachDescTest() throws Exception {
final AtomicReference<Integer> aCount = new AtomicReference<Integer>(0);
@ -128,6 +141,18 @@ public class AsciiStringMemoryTest {
assertEquals(bAsciiString.length(), bCount.get().intValue());
}
@Test
public void forEachDescWithIndexEndTest() throws Exception {
assertNotEquals(-1, bAsciiString.forEachByteDesc(bAsciiString.length() - 1,
1, new IndexOfProcessor(bAsciiString.byteAt(bAsciiString.length() - 1))));
}
@Test
public void forEachDescWithIndexBeginTest() throws Exception {
assertNotEquals(-1, bAsciiString.forEachByteDesc(0,
1, new IndexOfProcessor(bAsciiString.byteAt(0))));
}
@Test
public void subSequenceTest() {
final int start = 12;