Try another approach

line · Jun 8, 2023 · a2a2f48 · a2a2f48
1 parent b81f037
commit a2a2f48
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 49 deletions.
diff --git a/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java b/core/src/main/java/com/linecorp/armeria/internal/common/ArmeriaHttpUtil.java
@@ -720,7 +720,7 @@ public static void toArmeria(io.netty.handler.codec.http.HttpHeaders inHeaders,
         }
     }
 
-    private static boolean maybeWebSocketUpgrade(AsciiString header, CharSequence value) {
+    public static boolean maybeWebSocketUpgrade(AsciiString header, CharSequence value) {
         if (HttpHeaderNames.CONNECTION.contentEqualsIgnoreCase(header) &&
             HttpHeaderValues.UPGRADE.contentEqualsIgnoreCase(value)) {
             return true;

diff --git a/core/src/main/java/com/linecorp/armeria/server/ArmeriaHttpHeaders.java b/core/src/main/java/com/linecorp/armeria/server/ArmeriaHttpHeaders.java
@@ -16,25 +16,16 @@
 
 package com.linecorp.armeria.server;
 
-import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.COOKIE_SEPARATOR;
-import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.COOKIE_SPLITTER;
-import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.HTTP_TO_HTTP2_HEADER_DISALLOWED_LIST;
 import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.convertHeaderValue;
-import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.splitByCommaAndAdd;
-import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.toHttp2HeadersFilterTE;
-import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.toLowercaseMap;
 
 import java.util.AbstractMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map.Entry;
 import java.util.Set;
-import java.util.StringJoiner;
 import java.util.stream.Collectors;
 
-import com.linecorp.armeria.common.HttpHeaderNames;
 import com.linecorp.armeria.common.RequestHeadersBuilder;
-import com.linecorp.armeria.internal.common.ArmeriaHttpUtil.CaseInsensitiveMap;
 
 import io.netty.handler.codec.http.HttpHeaders;
 import io.netty.util.AsciiString;
@@ -45,7 +36,7 @@
 public final class ArmeriaHttpHeaders extends HttpHeaders {
 
     private final RequestHeadersBuilder builder;
-    private final CaseInsensitiveMap connectionDisallowedList;
+//    private final CaseInsensitiveMap connectionDisallowedList;
 
     /**
      * Creates a new instance.
@@ -54,7 +45,7 @@ public final class ArmeriaHttpHeaders extends HttpHeaders {
      */
     public ArmeriaHttpHeaders(RequestHeadersBuilder builder) {
         this.builder = builder;
-        connectionDisallowedList = toLowercaseMap(valueCharSequenceIterator(HttpHeaderNames.CONNECTION), 8);
+//        connectionDisallowedList = toLowercaseMap(valueCharSequenceIterator(HttpHeaderNames.CONNECTION), 8);
     }
 
     /**
@@ -66,7 +57,7 @@ public ArmeriaHttpHeaders(RequestHeadersBuilder builder) {
     public ArmeriaHttpHeaders(RequestHeadersBuilder builder, HttpHeaders headers) {
         this.builder = builder;
         headers.forEach(e -> this.add(e.getKey(), e.getValue()));
-        connectionDisallowedList = toLowercaseMap(valueCharSequenceIterator(HttpHeaderNames.CONNECTION), 8);
+//        connectionDisallowedList = toLowercaseMap(valueCharSequenceIterator(HttpHeaderNames.CONNECTION), 8);
     }
 
     @Override
@@ -157,41 +148,9 @@ public Set<String> names() {
     @Override
     public HttpHeaders add(String name, Object value) {
         final AsciiString asciiName = AsciiString.of(name);
-
-        if (asciiName.equals(HttpHeaderNames.CONNECTION)) {
-            final AsciiString lowerCased = AsciiString.of(value.toString().toLowerCase());
-            splitByCommaAndAdd(connectionDisallowedList, lowerCased);
-        }
-
-        if (HTTP_TO_HTTP2_HEADER_DISALLOWED_LIST.contains(asciiName) ||
-            connectionDisallowedList.contains(asciiName)) {
-            return this;
-        }
-
         final CharSequence charSequenceValue = (CharSequence) value;
-        if (asciiName.equals(HttpHeaderNames.TE)) {
-            toHttp2HeadersFilterTE(
-                    new AbstractMap.SimpleEntry<>(name, charSequenceValue),
-                    builder
-            );
-            return this;
-        }
-
-        if (asciiName.equals(HttpHeaderNames.COOKIE)) {
-            final StringJoiner cookieJoiner = new StringJoiner(COOKIE_SEPARATOR);
-
-            final String existingCookies = builder.get(HttpHeaderNames.COOKIE);
-            if (existingCookies != null) {
-                COOKIE_SPLITTER.split(existingCookies).forEach(cookieJoiner::add);
-            }
-            COOKIE_SPLITTER.split(charSequenceValue).forEach(cookieJoiner::add);
-
-            if (cookieJoiner.length() != 0) {
-                builder.set(HttpHeaderNames.COOKIE, cookieJoiner.toString());
-            }
-        } else {
-            builder.add(asciiName, convertHeaderValue(asciiName, charSequenceValue));
-        }
+
+        builder.add(asciiName, convertHeaderValue(asciiName, charSequenceValue));
 
         return this;
     }

diff --git a/core/src/main/java/com/linecorp/armeria/server/Http1RequestDecoder.java b/core/src/main/java/com/linecorp/armeria/server/Http1RequestDecoder.java
@@ -16,11 +16,22 @@
 
 package com.linecorp.armeria.server;
 
+import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.COOKIE_SEPARATOR;
+import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.COOKIE_SPLITTER;
+import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.HTTP_TO_HTTP2_HEADER_DISALLOWED_LIST;
+import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.convertHeaderValue;
+import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.maybeWebSocketUpgrade;
+import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.toHttp2HeadersFilterTE;
+import static com.linecorp.armeria.internal.common.ArmeriaHttpUtil.toLowercaseMap;
 import static com.linecorp.armeria.internal.common.websocket.WebSocketUtil.isHttp1WebSocketUpgradeRequest;
 import static com.linecorp.armeria.server.ServiceRouteUtil.newRoutingContext;
 
 import java.net.InetSocketAddress;
 import java.net.URISyntaxException;
+import java.util.AbstractMap;
+import java.util.Iterator;
+import java.util.Map.Entry;
+import java.util.StringJoiner;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -42,6 +53,7 @@
 import com.linecorp.armeria.common.annotation.Nullable;
 import com.linecorp.armeria.common.util.SystemInfo;
 import com.linecorp.armeria.internal.common.ArmeriaHttpUtil;
+import com.linecorp.armeria.internal.common.ArmeriaHttpUtil.CaseInsensitiveMap;
 import com.linecorp.armeria.internal.common.InboundTrafficController;
 import com.linecorp.armeria.internal.common.InitiateConnectionShutdown;
 import com.linecorp.armeria.internal.common.KeepAliveHandler;
@@ -172,6 +184,9 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
                         return;
                     }
 
+                    final boolean keepAlive = HttpUtil.isKeepAlive(nettyReq);
+                    final boolean transferEncodingChunked = !HttpUtil.isTransferEncodingChunked(nettyReq);
+
                     // Convert the Netty HttpHeaders into Armeria RequestHeaders.
                     final RequestHeaders headers;
                     assert nettyReq instanceof ArmeriaDefaultHttpRequest;
@@ -190,7 +205,52 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
                                     defaultHostname + ':' + port);
                     }
 
+                    HttpHeaders incomingHeaders = nettyReq.headers();
+                    final Iterator<Entry<CharSequence, CharSequence>> iter = incomingHeaders
+                            .iteratorCharSequence();
+                    final CaseInsensitiveMap connectionDisallowedList =
+                            toLowercaseMap(incomingHeaders.valueCharSequenceIterator(
+                                    com.linecorp.armeria.common.HttpHeaderNames.CONNECTION), 8);
+                    StringJoiner cookieJoiner = null;
+                    while (iter.hasNext()) {
+                        final Entry<CharSequence, CharSequence> entry = iter.next();
+                        final AsciiString asciiName = com.linecorp.armeria.common.HttpHeaderNames.of(entry.getKey()).toLowerCase();
+                        final CharSequence value = entry.getValue();
+
+                        if (HTTP_TO_HTTP2_HEADER_DISALLOWED_LIST.contains(asciiName) ||
+                            connectionDisallowedList.contains(asciiName)) {
+                            if (!maybeWebSocketUpgrade(asciiName, value)) {
+                                builder.remove(asciiName);
+                            }
+                        }
+
+                        final CharSequence charSequenceValue = (CharSequence) value;
+                        if (asciiName.equals(com.linecorp.armeria.common.HttpHeaderNames.TE)) {
+                            toHttp2HeadersFilterTE(
+                                    new AbstractMap.SimpleEntry<>(asciiName, charSequenceValue),
+                                    builder
+                            );
+                        }
+
+                        if (asciiName.equals(com.linecorp.armeria.common.HttpHeaderNames.COOKIE)) {
+                            cookieJoiner = new StringJoiner(COOKIE_SEPARATOR);
+
+                            final String existingCookies = builder.get(com.linecorp.armeria.common.HttpHeaderNames.COOKIE);
+                            if (existingCookies != null) {
+                                COOKIE_SPLITTER.split(existingCookies).forEach(cookieJoiner::add);
+                            }
+                            COOKIE_SPLITTER.split(charSequenceValue).forEach(cookieJoiner::add);
+
+                            if (cookieJoiner.length() != 0) {
+                                builder.set(com.linecorp.armeria.common.HttpHeaderNames.COOKIE, cookieJoiner.toString());
+                            }
+                        } else {
+                            builder.add(asciiName, convertHeaderValue(asciiName, charSequenceValue));
+                        }
+                    }
+
                     headers = builder.build();
+
                     // Do not accept unsupported methods.
                     final HttpMethod method = headers.method();
                     switch (method) {
@@ -276,8 +336,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
                         }
                     }
 
-                    final boolean keepAlive = HttpUtil.isKeepAlive(nettyReq);
-                    final boolean endOfStream = contentEmpty && !HttpUtil.isTransferEncodingChunked(nettyReq);
+                    final boolean endOfStream = contentEmpty && transferEncodingChunked;
                     this.req = req = DecodedHttpRequest.of(endOfStream, eventLoop, id, 1, headers,
                                                            keepAlive, inboundTrafficController, routingCtx);