From 750bd9f7b7dc3f3fc4ad29722059946c2908dde2 Mon Sep 17 00:00:00 2001 From: Kevin Oliver Date: Wed, 19 Jun 2019 02:03:49 -0700 Subject: [PATCH] codec-http2: Lazily translate cookies for HTTP/1 (#9251) Motivation: For HTTP/2 messages with multiple cookies HttpConversionUtil.addHttp2ToHttpHeaders spends a good portion of time creating throwaway StringBuilders. Modification: Handle cookies lazily by using a ThreadLocal StringBuilder and then converting it to the H1 header at the end. Result: Less allocations. --- .../codec/http2/HttpConversionUtil.java | 61 +++++++++++-------- .../codec/http2/HttpConversionUtilTest.java | 16 +++++ 2 files changed, 53 insertions(+), 24 deletions(-) diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpConversionUtil.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpConversionUtil.java index e350f30c62..d986bbe5be 100644 --- a/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpConversionUtil.java +++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpConversionUtil.java @@ -33,6 +33,7 @@ import io.netty.handler.codec.http.HttpResponseStatus; import io.netty.handler.codec.http.HttpUtil; import io.netty.handler.codec.http.HttpVersion; import io.netty.util.AsciiString; +import io.netty.util.internal.InternalThreadLocalMap; import io.netty.util.internal.UnstableApi; import java.net.URI; @@ -360,9 +361,7 @@ public final class HttpConversionUtil { HttpVersion httpVersion, boolean isTrailer, boolean isRequest) throws Http2Exception { Http2ToHttpHeaderTranslator translator = new Http2ToHttpHeaderTranslator(streamId, outputHeaders, isRequest); try { - for (Entry entry : inputHeaders) { - translator.translate(entry); - } + translator.translateHeaders(inputHeaders); } catch (Http2Exception ex) { throw ex; } catch (Throwable t) { @@ -620,29 +619,43 @@ public final class HttpConversionUtil { translations = request ? REQUEST_HEADER_TRANSLATIONS : RESPONSE_HEADER_TRANSLATIONS; } - public void translate(Entry entry) throws Http2Exception { - final CharSequence name = entry.getKey(); - final CharSequence value = entry.getValue(); - AsciiString translatedName = translations.get(name); - if (translatedName != null) { - output.add(translatedName, AsciiString.of(value)); - } else if (!Http2Headers.PseudoHeaderName.isPseudoHeader(name)) { - // https://tools.ietf.org/html/rfc7540#section-8.1.2.3 - // All headers that start with ':' are only valid in HTTP/2 context - if (name.length() == 0 || name.charAt(0) == ':') { - throw streamError(streamId, PROTOCOL_ERROR, - "Invalid HTTP/2 header '%s' encountered in translation to HTTP/1.x", name); - } - if (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(COOKIE); - output.set(COOKIE, - (existingCookie != null) ? (existingCookie + "; " + value) : value); - } else { - output.add(name, value); + public void translateHeaders(Iterable> inputHeaders) throws Http2Exception { + // lazily created as needed + StringBuilder cookies = null; + + for (Entry entry : inputHeaders) { + final CharSequence name = entry.getKey(); + final CharSequence value = entry.getValue(); + AsciiString translatedName = translations.get(name); + if (translatedName != null) { + output.add(translatedName, AsciiString.of(value)); + } else if (!Http2Headers.PseudoHeaderName.isPseudoHeader(name)) { + // https://tools.ietf.org/html/rfc7540#section-8.1.2.3 + // All headers that start with ':' are only valid in HTTP/2 context + if (name.length() == 0 || name.charAt(0) == ':') { + throw streamError(streamId, PROTOCOL_ERROR, + "Invalid HTTP/2 header '%s' encountered in translation to HTTP/1.x", name); + } + if (COOKIE.equals(name)) { + // combine the cookie values into 1 header entry. + // https://tools.ietf.org/html/rfc7540#section-8.1.2.5 + if (cookies == null) { + cookies = InternalThreadLocalMap.get().stringBuilder(); + } else if (cookies.length() > 0) { + cookies.append("; "); + } + cookies.append(value); + } else { + output.add(name, value); + } } } + if (cookies != null) { + output.add(COOKIE, cookies.toString()); + } + } + + private void translateHeader(Entry entry) throws Http2Exception { } } } diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/HttpConversionUtilTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/HttpConversionUtilTest.java index 200dd3975c..4da9fd51a4 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/HttpConversionUtilTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/HttpConversionUtilTest.java @@ -17,10 +17,12 @@ package io.netty.handler.codec.http2; import io.netty.handler.codec.http.DefaultHttpHeaders; import io.netty.handler.codec.http.HttpHeaders; +import io.netty.handler.codec.http.HttpVersion; import io.netty.util.AsciiString; import org.junit.Test; import static io.netty.handler.codec.http.HttpHeaderNames.CONNECTION; +import static io.netty.handler.codec.http.HttpHeaderNames.COOKIE; import static io.netty.handler.codec.http.HttpHeaderNames.TE; import static io.netty.handler.codec.http.HttpHeaderValues.GZIP; import static io.netty.handler.codec.http.HttpHeaderValues.TRAILERS; @@ -143,4 +145,18 @@ public class HttpConversionUtilTest { assertEquals(1, out.size()); assertSame("world", out.get("hello")); } + + @Test + public void addHttp2ToHttpHeadersCombinesCookies() throws Http2Exception { + Http2Headers inHeaders = new DefaultHttp2Headers(); + inHeaders.add("yes", "no"); + inHeaders.add(COOKIE, "foo=bar"); + inHeaders.add(COOKIE, "bax=baz"); + + HttpHeaders outHeaders = new DefaultHttpHeaders(); + + HttpConversionUtil.addHttp2ToHttpHeaders(5, inHeaders, outHeaders, HttpVersion.HTTP_1_1, false, false); + assertEquals("no", outHeaders.get("yes")); + assertEquals("foo=bar; bax=baz", outHeaders.get(COOKIE.toString())); + } }