Switch to io.netty.handler.codec.http.cookie.Cookie in order to suppo…

…rt cookies with duplicate keys (#1749) * Switch to newer netty cookie parser * more deprecated usages * bump minor version * Update gradle.properties Co-authored-by: Gavin Bunney <[email protected]> --------- Co-authored-by: Gavin Bunney <[email protected]>
Netflix · Feb 27, 2024 · 956328a · 956328a
1 parent aad0fea
commit 956328a
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 40 deletions.
diff --git a/gradle.properties b/gradle.properties
@@ -4,5 +4,5 @@ versions_netty=4.1.107.Final
 versions_netty_io_uring=0.0.25.Final
 versions_brotli4j=1.16.0
 release.scope=patch
-release.version=2.4.1-SNAPSHOT
+release.version=2.5.0-SNAPSHOT
 org.gradle.jvmargs=-Xms1g -Xmx2g
diff --git a/zuul-core/src/main/java/com/netflix/zuul/message/http/Cookies.java b/zuul-core/src/main/java/com/netflix/zuul/message/http/Cookies.java
@@ -16,7 +16,7 @@
 
 package com.netflix.zuul.message.http;
 
-import io.netty.handler.codec.http.Cookie;
+import io.netty.handler.codec.http.cookie.Cookie;
 
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -29,16 +29,11 @@
  * Time: 12:04 AM
  */
 public class Cookies {
-    private Map<String, List<Cookie>> map = new HashMap<>();
-    private List<Cookie> all = new ArrayList<>();
+    private final Map<String, List<Cookie>> map = new HashMap<>();
+    private final List<Cookie> all = new ArrayList<>();
 
     public void add(Cookie cookie) {
-        List<Cookie> existing = map.get(cookie.getName());
-        if (existing == null) {
-            existing = new ArrayList<>();
-            map.put(cookie.getName(), existing);
-        }
-        existing.add(cookie);
+        map.computeIfAbsent(cookie.name(), k -> new ArrayList<>(1)).add(cookie);
         all.add(cookie);
     }
 
@@ -52,7 +47,7 @@ public List<Cookie> get(String name) {
 
     public Cookie getFirst(String name) {
         List<Cookie> found = map.get(name);
-        if (found == null || found.size() == 0) {
+        if (found == null || found.isEmpty()) {
             return null;
         }
         return found.get(0);
@@ -62,7 +57,7 @@ public String getFirstValue(String name) {
         Cookie c = getFirst(name);
         String value;
         if (c != null) {
-            value = c.getValue();
+            value = c.value();
         } else {
             value = null;
         }

diff --git a/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpRequestMessageImpl.java b/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpRequestMessageImpl.java
@@ -27,9 +27,9 @@
 import com.netflix.zuul.message.ZuulMessage;
 import com.netflix.zuul.message.ZuulMessageImpl;
 import com.netflix.zuul.util.HttpUtils;
-import io.netty.handler.codec.http.Cookie;
-import io.netty.handler.codec.http.CookieDecoder;
 import io.netty.handler.codec.http.HttpContent;
+import io.netty.handler.codec.http.cookie.Cookie;
+import io.netty.handler.codec.http.cookie.ServerCookieDecoder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -41,7 +41,6 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
-import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -389,7 +388,7 @@ public Cookies reParseCookies() {
                     aCookieHeader = cleanCookieHeader(aCookieHeader);
                 }
 
-                Set<Cookie> decoded = CookieDecoder.decode(aCookieHeader, false);
+                List<Cookie> decoded = ServerCookieDecoder.LAX.decodeAll(aCookieHeader);
                 for (Cookie cookie : decoded) {
                     cookies.add(cookie);
                 }

diff --git a/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpResponseMessage.java b/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpResponseMessage.java
@@ -16,7 +16,7 @@
 
 package com.netflix.zuul.message.http;
 
-import io.netty.handler.codec.http.Cookie;
+import io.netty.handler.codec.http.cookie.Cookie;
 
 /**
  * User: Mike Smith

diff --git a/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpResponseMessageImpl.java b/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpResponseMessageImpl.java
@@ -23,10 +23,10 @@
 import com.netflix.zuul.message.Headers;
 import com.netflix.zuul.message.ZuulMessage;
 import com.netflix.zuul.message.ZuulMessageImpl;
-import io.netty.handler.codec.http.Cookie;
-import io.netty.handler.codec.http.CookieDecoder;
 import io.netty.handler.codec.http.HttpContent;
-import io.netty.handler.codec.http.ServerCookieEncoder;
+import io.netty.handler.codec.http.cookie.ClientCookieDecoder;
+import io.netty.handler.codec.http.cookie.Cookie;
+import io.netty.handler.codec.http.cookie.ServerCookieEncoder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -179,24 +179,19 @@ public int getMaxBodySize() {
     @Override
     public Cookies parseSetCookieHeader(String setCookieValue) {
         Cookies cookies = new Cookies();
-        for (Cookie cookie : CookieDecoder.decode(setCookieValue)) {
-            cookies.add(cookie);
-        }
+        cookies.add(ClientCookieDecoder.STRICT.decode(setCookieValue));
         return cookies;
     }
 
     @Override
     public boolean hasSetCookieWithName(String cookieName) {
-        boolean has = false;
         for (String setCookieValue : getHeaders().getAll(HttpHeaderNames.SET_COOKIE)) {
-            for (Cookie cookie : CookieDecoder.decode(setCookieValue)) {
-                if (cookie.getName().equalsIgnoreCase(cookieName)) {
-                    has = true;
-                    break;
-                }
+            Cookie cookie = ClientCookieDecoder.STRICT.decode(setCookieValue);
+            if (cookie.name().equalsIgnoreCase(cookieName)) {
+                return true;
             }
         }
-        return has;
+        return false;
     }
 
     @Override
@@ -230,12 +225,12 @@ public boolean removeExistingSetCookie(String cookieName) {
 
     @Override
     public void addSetCookie(Cookie cookie) {
-        getHeaders().add(HttpHeaderNames.SET_COOKIE, ServerCookieEncoder.encode(cookie));
+        getHeaders().add(HttpHeaderNames.SET_COOKIE, ServerCookieEncoder.LAX.encode(cookie));
     }
 
     @Override
     public void setSetCookie(Cookie cookie) {
-        getHeaders().set(HttpHeaderNames.SET_COOKIE, ServerCookieEncoder.encode(cookie));
+        getHeaders().set(HttpHeaderNames.SET_COOKIE, ServerCookieEncoder.LAX.encode(cookie));
     }
 
     @Override

diff --git a/zuul-core/src/main/java/com/netflix/zuul/netty/server/push/PushAuthHandler.java b/zuul-core/src/main/java/com/netflix/zuul/netty/server/push/PushAuthHandler.java
@@ -22,8 +22,6 @@
 import io.netty.channel.ChannelHandler;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.SimpleChannelInboundHandler;
-import io.netty.handler.codec.http.Cookie;
-import io.netty.handler.codec.http.CookieDecoder;
 import io.netty.handler.codec.http.DefaultFullHttpResponse;
 import io.netty.handler.codec.http.FullHttpRequest;
 import io.netty.handler.codec.http.FullHttpResponse;
@@ -33,10 +31,12 @@
 import io.netty.handler.codec.http.HttpResponseStatus;
 import io.netty.handler.codec.http.HttpUtil;
 import io.netty.handler.codec.http.HttpVersion;
+import io.netty.handler.codec.http.cookie.Cookie;
+import io.netty.handler.codec.http.cookie.ServerCookieDecoder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.Set;
+import java.util.List;
 
 /**
  * Author: Susheel Aroskar
@@ -114,8 +114,8 @@ protected final Cookies parseCookies(FullHttpRequest req) {
         final Cookies cookies = new Cookies();
         final String cookieStr = req.headers().get(HttpHeaderNames.COOKIE);
         if (!Strings.isNullOrEmpty(cookieStr)) {
-            final Set<Cookie> decoded = CookieDecoder.decode(cookieStr, false);
-            decoded.forEach(cookie -> cookies.add(cookie));
+            List<Cookie> decoded = ServerCookieDecoder.LAX.decodeAll(cookieStr);
+            decoded.forEach(cookies::add);
         }
         return cookies;
     }

diff --git a/zuul-core/src/test/java/com/netflix/zuul/message/http/HttpRequestMessageImplTest.java b/zuul-core/src/test/java/com/netflix/zuul/message/http/HttpRequestMessageImplTest.java
@@ -22,6 +22,7 @@
 import com.netflix.zuul.context.SessionContext;
 import com.netflix.zuul.message.Headers;
 import io.netty.channel.local.LocalAddress;
+import io.netty.handler.codec.http.cookie.Cookie;
 import org.apache.commons.configuration.AbstractConfiguration;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
@@ -31,6 +32,7 @@
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.net.URISyntaxException;
+import java.util.List;
 import java.util.Optional;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -791,4 +793,29 @@ void shouldPreferClientDestPortWhenInitialized() {
 
         assertEquals(message.getClientDestinationPort(), Optional.of(443));
     }
+
+    @Test
+    public void duplicateCookieNames() {
+        Headers headers = new Headers();
+        headers.add("cookie", "k=v1;k=v2");
+        HttpRequestMessageImpl message = new HttpRequestMessageImpl(
+                new SessionContext(),
+                "HTTP/1.1",
+                "POST",
+                "/some/where",
+                new HttpQueryParams(),
+                headers,
+                "192.168.0.2",
+                "https",
+                7002,
+                "localhost",
+                new InetSocketAddress("api.netflix.com", 443),
+                true);
+        Cookies cookies = message.parseCookies();
+        assertEquals(2, cookies.getAll().size());
+        List<Cookie> kCookies = cookies.get("k");
+        assertEquals(2, kCookies.size());
+        assertEquals("v1", kCookies.get(0).value());
+        assertEquals("v2", kCookies.get(1).value());
+    }
 }
diff --git a/zuul-sample/src/main/java/com/netflix/zuul/sample/push/SamplePushAuthHandler.java b/zuul-sample/src/main/java/com/netflix/zuul/sample/push/SamplePushAuthHandler.java
@@ -21,9 +21,9 @@
 import com.netflix.zuul.netty.server.push.PushUserAuth;
 import io.netty.channel.ChannelHandler;
 import io.netty.channel.ChannelHandlerContext;
-import io.netty.handler.codec.http.Cookie;
 import io.netty.handler.codec.http.FullHttpRequest;
 import io.netty.handler.codec.http.HttpResponseStatus;
+import io.netty.handler.codec.http.cookie.Cookie;
 
 /**
  * Takes cookie value of the cookie "userAuthCookie" as a customerId WITHOUT ANY actual validation.
@@ -52,8 +52,8 @@ protected boolean isDelayedAuth(FullHttpRequest req, ChannelHandlerContext ctx)
     protected PushUserAuth doAuth(FullHttpRequest req) {
         final Cookies cookies = parseCookies(req);
         for (final Cookie c : cookies.getAll()) {
-            if (c.getName().equals("userAuthCookie")) {
-                final String customerId = c.getValue();
+            if (c.name().equals("userAuthCookie")) {
+                final String customerId = c.value();
                 if (!Strings.isNullOrEmpty(customerId)) {
                     return new SamplePushUserAuth(customerId);
                 }