From 011099b84c8c29ed108f1ac64fc6c70f89aae76d Mon Sep 17 00:00:00 2001 From: Justin Guerra Date: Tue, 27 Feb 2024 15:25:04 -0700 Subject: [PATCH] Switch to io.netty.handler.codec.http.cookie.Cookie in order to support 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 <409207+gavinbunney@users.noreply.github.com> --------- Co-authored-by: Gavin Bunney <409207+gavinbunney@users.noreply.github.com> --- gradle.properties | 2 +- .../netflix/zuul/message/http/Cookies.java | 17 +++++------- .../message/http/HttpRequestMessageImpl.java | 7 +++-- .../message/http/HttpResponseMessage.java | 2 +- .../message/http/HttpResponseMessageImpl.java | 25 +++++++---------- .../netty/server/push/PushAuthHandler.java | 10 +++---- .../http/HttpRequestMessageImplTest.java | 27 +++++++++++++++++++ .../sample/push/SamplePushAuthHandler.java | 6 ++--- 8 files changed, 56 insertions(+), 40 deletions(-) diff --git a/gradle.properties b/gradle.properties index 8b9ae3d2d5..b727b80246 100644 --- 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 index aa58edfd99..d1496c0a6f 100644 --- 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> map = new HashMap<>(); - private List all = new ArrayList<>(); + private final Map> map = new HashMap<>(); + private final List all = new ArrayList<>(); public void add(Cookie cookie) { - List 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 get(String name) { public Cookie getFirst(String name) { List 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 index 0da8022db8..4cc24192dc 100644 --- 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 decoded = CookieDecoder.decode(aCookieHeader, false); + List 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 index 98ee1791ac..3d53fd06a0 100644 --- 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 index 10d3249ea5..be8a282d25 100644 --- 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 index a5e4348af7..9f7592ded6 100644 --- 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 decoded = CookieDecoder.decode(cookieStr, false); - decoded.forEach(cookie -> cookies.add(cookie)); + List 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 index 33c06b3741..e35dfec51b 100644 --- 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 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 index 1d2f523bb5..125dadd380 100644 --- 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); }