The addCookie method always replaces existing SetCookie fields #10797

Fix #10797 by make addCookie always replace existing SetCookie fields. Deprecated replaceCookie method.
jetty · Oct 27, 2023 · 09ce5da · 09ce5da
1 parent 7be6c4b
commit 09ce5da
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 39 deletions.
diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java
@@ -323,62 +323,76 @@ static void sendRedirect(Request request, Response response, Callback callback,
     }
 
     /**
-     * <p>Adds an HTTP cookie to the response.</p>
+     * <p>Adds an HTTP {@link HttpHeader#SET_COOKIE} header to the response.</p>
+     * <p>If a matching {@link HttpHeader#SET_COOKIE} already exists for matching name, path, domain etc.
+     * then it will be replaced.</p>
      *
      * @param response the HTTP response
      * @param cookie the HTTP cookie to add
      */
     static void addCookie(Response response, HttpCookie cookie)
-    {
-        if (StringUtil.isBlank(cookie.getName()))
-            throw new IllegalArgumentException("Cookie.name cannot be blank/null");
-
-        Request request = response.getRequest();
-        response.getHeaders().add(new HttpCookieUtils.SetCookieHttpField(HttpCookieUtils.checkSameSite(cookie, request.getContext()),
-            request.getConnectionMetaData().getHttpConfiguration().getResponseCookieCompliance()));
-
-        // Expire responses with set-cookie headers so they do not get cached.
-        response.getHeaders().put(HttpFields.EXPIRES_01JAN1970);
-    }
-
-    /**
-     * <p>Replaces (if already exists, otherwise adds) an HTTP cookie to the response.</p>
-     *
-     * @param response the HTTP response
-     * @param cookie the HTTP cookie to replace or add
-     */
-    static void replaceCookie(Response response, HttpCookie cookie)
     {
         if (StringUtil.isBlank(cookie.getName()))
             throw new IllegalArgumentException("Cookie.name cannot be blank/null");
 
         Request request = response.getRequest();
         HttpConfiguration httpConfiguration = request.getConnectionMetaData().getHttpConfiguration();
+        CookieCompliance compliance = httpConfiguration.getResponseCookieCompliance();
+        HttpField setCookie = new HttpCookieUtils.SetCookieHttpField(HttpCookieUtils.checkSameSite(cookie, request.getContext()), compliance);
+
+        boolean expires = false;
 
         for (ListIterator<HttpField> i = response.getHeaders().listIterator(); i.hasNext(); )
         {
             HttpField field = i.next();
+            HttpHeader header = field.getHeader();
+            if (header == null)
+                continue;
 
-            if (field.getHeader() == HttpHeader.SET_COOKIE)
+            switch (header)
             {
-                CookieCompliance compliance = httpConfiguration.getResponseCookieCompliance();
-                if (field instanceof HttpCookieUtils.SetCookieHttpField)
-                {
-                    if (!HttpCookieUtils.match(((HttpCookieUtils.SetCookieHttpField)field).getHttpCookie(), cookie.getName(), cookie.getDomain(), cookie.getPath()))
-                        continue;
-                }
-                else
+                case SET_COOKIE ->
                 {
-                    if (!HttpCookieUtils.match(field.getValue(), cookie.getName(), cookie.getDomain(), cookie.getPath()))
-                        continue;
+                    if (field instanceof HttpCookieUtils.SetCookieHttpField setCookieHttpField)
+                    {
+                        if (!HttpCookieUtils.match(setCookieHttpField.getHttpCookie(), cookie.getName(), cookie.getDomain(), cookie.getPath()))
+                            continue;
+                    }
+                    else
+                    {
+                        if (!HttpCookieUtils.match(field.getValue(), cookie.getName(), cookie.getDomain(), cookie.getPath()))
+                            continue;
+                    }
+
+                    if (setCookie == null)
+                    {
+                        i.remove();
+                    }
+                    else
+                    {
+                        i.set(setCookie);
+                        setCookie = null;
+                    }
                 }
 
-                i.set(new HttpCookieUtils.SetCookieHttpField(HttpCookieUtils.checkSameSite(cookie, request.getContext()), compliance));
-                return;
+                case EXPIRES -> expires = true;
             }
         }
 
-        // Not replaced, so add normally
+        if (setCookie != null)
+            response.getHeaders().add(setCookie);
+
+        // Expire responses with set-cookie headers, so they do not get cached.
+        if (!expires)
+            response.getHeaders().put(HttpFields.EXPIRES_01JAN1970);
+    }
+
+    /**
+     * @deprecated use {@link #addCookie(Response, HttpCookie)}
+     */
+    @Deprecated
+    static void replaceCookie(Response response, HttpCookie cookie)
+    {
         addCookie(response, cookie);
     }
 

diff --git a/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/ManagedSession.java b/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/ManagedSession.java
@@ -610,7 +610,7 @@ public void renewId(Request request, Response response)
         }
 
         if (response != null && isSetCookieNeeded())
-            Response.replaceCookie(response, getSessionManager().getSessionCookie(this, request.isSecure()));
+            Response.addCookie(response, getSessionManager().getSessionCookie(this, request.isSecure()));
         if (LOG.isDebugEnabled())
             LOG.debug("renew {}->{}", oldId, newId);
     }

diff --git a/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/SessionHandler.java b/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/SessionHandler.java
@@ -117,7 +117,7 @@ public Session getSession(boolean create)
                 session = _session.get();
                 HttpCookie cookie = getSessionCookie(session, getConnectionMetaData().isSecure());
                 if (cookie != null)
-                    Response.replaceCookie(_response, cookie);
+                    Response.addCookie(_response, cookie);
             }
 
             return session == null || !session.isValid() ? null : session;
@@ -136,7 +136,7 @@ public boolean process(Handler handler, Response response, Callback callback) th
                 _session.set(session);
                 HttpCookie cookie = access(session, getConnectionMetaData().isSecure());
                 if (cookie != null)
-                    Response.replaceCookie(_response, cookie);
+                    Response.addCookie(_response, cookie);
             }
 
             return handler.handle(this, _response, callback);

diff --git a/...etty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java b/...etty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java
@@ -483,7 +483,7 @@ public Session getSession(boolean create)
 
         HttpCookie cookie = _sessionManager.getSessionCookie(_managedSession, isSecure());
         if (cookie != null)
-            Response.replaceCookie(_response, cookie);
+            Response.addCookie(_response, cookie);
 
         return _managedSession;
     }

diff --git a/...-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/SessionHandler.java b/...-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/SessionHandler.java
@@ -38,7 +38,6 @@
 import jakarta.servlet.http.HttpSessionListener;
 import org.eclipse.jetty.http.HttpCookie;
 import org.eclipse.jetty.http.HttpCookie.SameSite;
-import org.eclipse.jetty.http.Syntax;
 import org.eclipse.jetty.server.Handler;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.Response;
@@ -687,7 +686,7 @@ public boolean handle(Request request, Response response, Callback callback) thr
         if (cookie != null)
         {
             ServletContextResponse servletContextResponse = servletContextRequest.getServletContextResponse();
-            Response.replaceCookie(servletContextResponse, cookie);
+            Response.addCookie(servletContextResponse, cookie);
         }
 
         return next.handle(request, response, callback);