From 09ce5da8049abb6e61936a2f5d2acf020bd3fb9a Mon Sep 17 00:00:00 2001 From: gregw Date: Sat, 28 Oct 2023 08:08:03 +1100 Subject: [PATCH] The addCookie method always replaces existing SetCookie fields #10797 Fix #10797 by make addCookie always replace existing SetCookie fields. Deprecated replaceCookie method. --- .../org/eclipse/jetty/server/Response.java | 80 +++++++++++-------- .../eclipse/jetty/session/ManagedSession.java | 2 +- .../eclipse/jetty/session/SessionHandler.java | 4 +- .../ee10/servlet/ServletContextRequest.java | 2 +- .../jetty/ee10/servlet/SessionHandler.java | 3 +- 5 files changed, 52 insertions(+), 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 index 470e3d681730..304af13946c8 100644 --- 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, } /** - *

Adds an HTTP cookie to the response.

+ *

Adds an HTTP {@link HttpHeader#SET_COOKIE} header to the response.

+ *

If a matching {@link HttpHeader#SET_COOKIE} already exists for matching name, path, domain etc. + * then it will be replaced.

* * @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); - } - - /** - *

Replaces (if already exists, otherwise adds) an HTTP cookie to the response.

- * - * @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 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 index 2bfcd8185dde..68fb2a872eeb 100644 --- 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 index 042120466da7..b585e1e60bf4 100644 --- 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/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java index 6b55fcceaa92..0013fa4fd5a0 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java +++ b/jetty-ee10/jetty-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/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/SessionHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/SessionHandler.java index 72e01c8626cd..ce49fa38cbcb 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/SessionHandler.java +++ b/jetty-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);