From 5df6e8825db86a88e06ea0bd189094d7a9a361ff Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 6 Nov 2023 11:31:39 +0000 Subject: [PATCH] Polishing in CookieWebSessionIdResolver See gh-31214 --- .../session/CookieWebSessionIdResolver.java | 38 +++++++++---------- .../CookieWebSessionIdResolverTests.java | 28 +++++++------- 2 files changed, 31 insertions(+), 35 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/server/session/CookieWebSessionIdResolver.java b/spring-web/src/main/java/org/springframework/web/server/session/CookieWebSessionIdResolver.java index 1bb7e4fa6d..f355abada8 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/CookieWebSessionIdResolver.java +++ b/spring-web/src/main/java/org/springframework/web/server/session/CookieWebSessionIdResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -42,11 +42,11 @@ public class CookieWebSessionIdResolver implements WebSessionIdResolver { private Duration cookieMaxAge = Duration.ofSeconds(-1); @Nullable - private Consumer cookieInitializer = null; + private Consumer initializer = null; /** - * Set the name of the cookie to use for the session ID. + * Set the name for the session id cookie. *

By default set to "SESSION". * @param cookieName the cookie name */ @@ -63,32 +63,32 @@ public class CookieWebSessionIdResolver implements WebSessionIdResolver { } /** - * Set the value for the "Max-Age" attribute of the cookie that holds the - * session ID. - *

For the range of values see {@link ResponseCookie#getMaxAge()}. - *

By default set to -1. + * Set the "Max-Age" attribute for the session id cookie. + *

By default set to -1 in which case the cookie is removed when the + * browser is closed. * @param maxAge the maxAge duration value + * @see ResponseCookie#getMaxAge() */ public void setCookieMaxAge(Duration maxAge) { this.cookieMaxAge = maxAge; } /** - * Get the configured "Max-Age" attribute value for the session cookie. + * Get the configured "Max-Age" for the session id cookie. */ public Duration getCookieMaxAge() { return this.cookieMaxAge; } /** - * Add a {@link Consumer} for a {@code ResponseCookieBuilder} that will be invoked - * for each cookie being built, just before the call to {@code build()}. - * @param initializer consumer for a cookie builder + * Add a {@link Consumer} to further initialize the session id cookie + * after {@link #getCookieName()} and {@link #getCookieMaxAge()} are applied. + * @param initializer consumer to initialize the cookie with * @since 5.1 */ public void addCookieInitializer(Consumer initializer) { - this.cookieInitializer = this.cookieInitializer != null ? - this.cookieInitializer.andThen(initializer) : initializer; + this.initializer = this.initializer != null ? + this.initializer.andThen(initializer) : initializer; } @@ -115,21 +115,19 @@ public class CookieWebSessionIdResolver implements WebSessionIdResolver { exchange.getResponse().getCookies().set(this.cookieName, cookie); } - private ResponseCookie initSessionCookie( - ServerWebExchange exchange, String id, Duration maxAge) { - - ResponseCookie.ResponseCookieBuilder cookieBuilder = ResponseCookie.from(this.cookieName, id) + private ResponseCookie initSessionCookie(ServerWebExchange exchange, String id, Duration maxAge) { + ResponseCookie.ResponseCookieBuilder builder = ResponseCookie.from(this.cookieName, id) .path(exchange.getRequest().getPath().contextPath().value() + "/") .maxAge(maxAge) .httpOnly(true) .secure("https".equalsIgnoreCase(exchange.getRequest().getURI().getScheme())) .sameSite("Lax"); - if (this.cookieInitializer != null) { - this.cookieInitializer.accept(cookieBuilder); + if (this.initializer != null) { + this.initializer.accept(builder); } - return cookieBuilder.build(); + return builder.build(); } } diff --git a/spring-web/src/test/java/org/springframework/web/server/session/CookieWebSessionIdResolverTests.java b/spring-web/src/test/java/org/springframework/web/server/session/CookieWebSessionIdResolverTests.java index 60b5716a06..cf1deb1c1c 100644 --- a/spring-web/src/test/java/org/springframework/web/server/session/CookieWebSessionIdResolverTests.java +++ b/spring-web/src/test/java/org/springframework/web/server/session/CookieWebSessionIdResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,7 @@ import org.junit.jupiter.api.Test; import org.springframework.http.ResponseCookie; import org.springframework.util.MultiValueMap; +import org.springframework.web.server.ServerWebExchange; import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest; import org.springframework.web.testfixture.server.MockServerWebExchange; @@ -33,18 +34,14 @@ public class CookieWebSessionIdResolverTests { private final CookieWebSessionIdResolver resolver = new CookieWebSessionIdResolver(); + private final ServerWebExchange exchange = + MockServerWebExchange.from(MockServerHttpRequest.get("https://example.org/path")); + @Test public void setSessionId() { - MockServerHttpRequest request = MockServerHttpRequest.get("https://example.org/path").build(); - MockServerWebExchange exchange = MockServerWebExchange.from(request); - this.resolver.setSessionId(exchange, "123"); - - MultiValueMap cookies = exchange.getResponse().getCookies(); - assertThat(cookies).hasSize(1); - ResponseCookie cookie = cookies.getFirst(this.resolver.getCookieName()); - assertThat(cookie).isNotNull(); - assertThat(cookie.toString()).isEqualTo("SESSION=123; Path=/; Secure; HttpOnly; SameSite=Lax"); + this.resolver.setSessionId(this.exchange, "123"); + assertCookieValue("SESSION=123; Path=/; Secure; HttpOnly; SameSite=Lax"); } @Test @@ -52,16 +49,17 @@ public class CookieWebSessionIdResolverTests { this.resolver.addCookieInitializer(builder -> builder.domain("example.org")); this.resolver.addCookieInitializer(builder -> builder.sameSite("Strict")); this.resolver.addCookieInitializer(builder -> builder.secure(false)); + this.resolver.setSessionId(this.exchange, "123"); - MockServerHttpRequest request = MockServerHttpRequest.get("https://example.org/path").build(); - MockServerWebExchange exchange = MockServerWebExchange.from(request); - this.resolver.setSessionId(exchange, "123"); + assertCookieValue("SESSION=123; Path=/; Domain=example.org; HttpOnly; SameSite=Strict"); + } - MultiValueMap cookies = exchange.getResponse().getCookies(); + private void assertCookieValue(String expected) { + MultiValueMap cookies = this.exchange.getResponse().getCookies(); assertThat(cookies).hasSize(1); ResponseCookie cookie = cookies.getFirst(this.resolver.getCookieName()); assertThat(cookie).isNotNull(); - assertThat(cookie.toString()).isEqualTo("SESSION=123; Path=/; Domain=example.org; HttpOnly; SameSite=Strict"); + assertThat(cookie.toString()).isEqualTo(expected); } }