From 00a25f5d89d3c9e18ef9bbcf97cc3ae3498b3b8d Mon Sep 17 00:00:00 2001 From: Moritz Halbritter Date: Wed, 24 Apr 2024 15:37:43 +0200 Subject: [PATCH] Don't apply CookieSameSiteSupplier to session cookies when using Jetty Closes gh-39766 --- .../jetty/JettyServletWebServerFactory.java | 25 ++++++++++++++++--- .../AbstractServletWebServerFactoryTests.java | 20 ++++++++++++++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactory.java index 127cb3758b9..63f0299463b 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactory.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/JettyServletWebServerFactory.java @@ -77,6 +77,7 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.handler.StatisticsHandler; import org.eclipse.jetty.session.DefaultSessionCache; import org.eclipse.jetty.session.FileSessionDataStore; +import org.eclipse.jetty.session.SessionConfig; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.resource.CombinedResource; import org.eclipse.jetty.util.resource.Resource; @@ -237,11 +238,17 @@ public class JettyServletWebServerFactory extends AbstractServletWebServerFactor handler = applyWrapper(handler, JettyHandlerWrappers.createServerHeaderHandlerWrapper(getServerHeader())); } if (!CollectionUtils.isEmpty(getCookieSameSiteSuppliers())) { - handler = applyWrapper(handler, new SuppliedSameSiteCookieHandlerWrapper(getCookieSameSiteSuppliers())); + handler = applyWrapper(handler, + new SuppliedSameSiteCookieHandlerWrapper(getSessionCookieName(), getCookieSameSiteSuppliers())); } return handler; } + private String getSessionCookieName() { + String name = getSession().getCookie().getName(); + return (name != null) ? name : SessionConfig.__DefaultSessionCookie; + } + private Handler applyWrapper(Handler handler, Handler.Wrapper wrapper) { wrapper.setHandler(handler); return wrapper; @@ -779,9 +786,12 @@ public class JettyServletWebServerFactory extends AbstractServletWebServerFactor private static final SetCookieParser setCookieParser = SetCookieParser.newInstance(); + private final String sessionCookieName; + private final List suppliers; - SuppliedSameSiteCookieHandlerWrapper(List suppliers) { + SuppliedSameSiteCookieHandlerWrapper(String sessionCookieName, List suppliers) { + this.sessionCookieName = sessionCookieName; this.suppliers = suppliers; } @@ -793,7 +803,7 @@ public class JettyServletWebServerFactory extends AbstractServletWebServerFactor private class SuppliedSameSiteCookieResponse extends Response.Wrapper { - private HttpFields.Mutable wrappedHeaders; + private final HttpFields.Mutable wrappedHeaders; SuppliedSameSiteCookieResponse(Request request, Response wrapped) { super(request, wrapped); @@ -825,7 +835,10 @@ public class JettyServletWebServerFactory extends AbstractServletWebServerFactor private HttpField onAddSetCookieField(HttpField field) { HttpCookie cookie = setCookieParser.parse(field.getValue()); - SameSite sameSite = (cookie != null) ? getSameSite(cookie) : null; + if (cookie == null || isSessionCookie(cookie)) { + return field; + } + SameSite sameSite = getSameSite(cookie); if (sameSite == null) { return field; } @@ -833,6 +846,10 @@ public class JettyServletWebServerFactory extends AbstractServletWebServerFactor return new HttpCookieUtils.SetCookieHttpField(updatedCookie, this.compliance); } + private boolean isSessionCookie(HttpCookie cookie) { + return SuppliedSameSiteCookieHandlerWrapper.this.sessionCookieName.equals(cookie.getName()); + } + private HttpCookie buildCookieWithUpdatedSameSite(HttpCookie cookie, SameSite sameSite) { return HttpCookie.build(cookie) .sameSite(org.eclipse.jetty.http.HttpCookie.SameSite.from(sameSite.name())) diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java index a3bb489a5ec..521e547ae32 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/server/AbstractServletWebServerFactoryTests.java @@ -177,6 +177,7 @@ import static org.mockito.Mockito.times; * @author Andy Wilkinson * @author Raja Kolli * @author Scott Frederick + * @author Moritz Halbritter */ @SuppressWarnings("removal") @ExtendWith(OutputCaptureExtension.class) @@ -957,6 +958,23 @@ public abstract class AbstractServletWebServerFactoryTests { (header) -> assertThat(header).contains("controlled=test").contains("SameSite=Strict")); } + @Test + void cookieSameSiteSuppliersShouldNotAffectSessionCookie() throws IOException, URISyntaxException { + AbstractServletWebServerFactory factory = getFactory(); + factory.getSession().getCookie().setSameSite(SameSite.LAX); + factory.getSession().getCookie().setName("SESSIONCOOKIE"); + factory.addCookieSameSiteSuppliers(CookieSameSiteSupplier.ofStrict()); + factory.addInitializers(new ServletRegistrationBean<>(new CookieServlet(false), "/")); + this.webServer = factory.getWebServer(); + this.webServer.start(); + ClientHttpResponse clientResponse = getClientResponse(getLocalUrl("/")); + assertThat(clientResponse.getStatusCode()).isEqualTo(HttpStatus.OK); + List setCookieHeaders = clientResponse.getHeaders().get("Set-Cookie"); + assertThat(setCookieHeaders).satisfiesExactlyInAnyOrder( + (header) -> assertThat(header).contains("SESSIONCOOKIE").contains("SameSite=Lax"), + (header) -> assertThat(header).contains("test=test").contains("SameSite=Strict")); + } + @Test protected void sslSessionTracking() { AbstractServletWebServerFactory factory = getFactory(); @@ -1803,7 +1821,7 @@ public abstract class AbstractServletWebServerFactoryTests { } @Override - protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + protected void doGet(HttpServletRequest req, HttpServletResponse resp) { req.getSession(true); resp.addCookie(new Cookie("test", "test")); if (this.addSupplierTestCookies) {