From a4f95d6fbe079e96896dceb9d295cb229e07dca9 Mon Sep 17 00:00:00 2001 From: M-Faheem-Khan Date: Sun, 27 Apr 2025 23:53:19 -0400 Subject: [PATCH 1/2] Remove deprecated Cookie usage Remove usage of comment and verison usage Signed-off-by: M-Faheem-Khan --- .../AbstractRememberMeServices.java | 3 -- .../web/firewall/FirewalledResponse.java | 1 - .../web/jackson2/CookieDeserializer.java | 2 -- .../web/jackson2/SavedCookieMixin.java | 6 ++-- .../web/savedrequest/SavedCookie.java | 33 +------------------ .../AbstractRememberMeServicesTests.java | 11 ------- .../web/firewall/FirewalledResponseTests.java | 1 - .../DefaultSavedRequestMixinTests.java | 2 -- .../web/jackson2/SavedCookieMixinTests.java | 6 +--- .../web/savedrequest/SavedCookieTests.java | 14 -------- 10 files changed, 4 insertions(+), 75 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java b/web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java index 30b9e261d05..fd1c38b18aa 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java +++ b/web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java @@ -372,9 +372,6 @@ protected void setCookie(String[] tokens, int maxAge, HttpServletRequest request if (this.cookieDomain != null) { cookie.setDomain(this.cookieDomain); } - if (maxAge < 1) { - cookie.setVersion(1); - } cookie.setSecure((this.useSecureCookie != null) ? this.useSecureCookie : request.isSecure()); cookie.setHttpOnly(true); diff --git a/web/src/main/java/org/springframework/security/web/firewall/FirewalledResponse.java b/web/src/main/java/org/springframework/security/web/firewall/FirewalledResponse.java index e5d34ff197e..12bfacedd36 100644 --- a/web/src/main/java/org/springframework/security/web/firewall/FirewalledResponse.java +++ b/web/src/main/java/org/springframework/security/web/firewall/FirewalledResponse.java @@ -67,7 +67,6 @@ public void addCookie(Cookie cookie) { validateCrlf(SET_COOKIE_HEADER, cookie.getValue()); validateCrlf(SET_COOKIE_HEADER, cookie.getPath()); validateCrlf(SET_COOKIE_HEADER, cookie.getDomain()); - validateCrlf(SET_COOKIE_HEADER, cookie.getComment()); } super.addCookie(cookie); } diff --git a/web/src/main/java/org/springframework/security/web/jackson2/CookieDeserializer.java b/web/src/main/java/org/springframework/security/web/jackson2/CookieDeserializer.java index d4123a8188c..99b3bc9d1a7 100644 --- a/web/src/main/java/org/springframework/security/web/jackson2/CookieDeserializer.java +++ b/web/src/main/java/org/springframework/security/web/jackson2/CookieDeserializer.java @@ -45,11 +45,9 @@ public Cookie deserialize(JsonParser jp, DeserializationContext ctxt) throws IOE ObjectMapper mapper = (ObjectMapper) jp.getCodec(); JsonNode jsonNode = mapper.readTree(jp); Cookie cookie = new Cookie(readJsonNode(jsonNode, "name").asText(), readJsonNode(jsonNode, "value").asText()); - cookie.setComment(readJsonNode(jsonNode, "comment").asText()); cookie.setDomain(readJsonNode(jsonNode, "domain").asText()); cookie.setMaxAge(readJsonNode(jsonNode, "maxAge").asInt(-1)); cookie.setSecure(readJsonNode(jsonNode, "secure").asBoolean()); - cookie.setVersion(readJsonNode(jsonNode, "version").asInt()); cookie.setPath(readJsonNode(jsonNode, "path").asText()); JsonNode attributes = readJsonNode(jsonNode, "attributes"); cookie.setHttpOnly(readJsonNode(attributes, "HttpOnly") != null); diff --git a/web/src/main/java/org/springframework/security/web/jackson2/SavedCookieMixin.java b/web/src/main/java/org/springframework/security/web/jackson2/SavedCookieMixin.java index 3859b3ae245..817972db154 100644 --- a/web/src/main/java/org/springframework/security/web/jackson2/SavedCookieMixin.java +++ b/web/src/main/java/org/springframework/security/web/jackson2/SavedCookieMixin.java @@ -44,10 +44,8 @@ abstract class SavedCookieMixin { @JsonCreator SavedCookieMixin(@JsonProperty("name") String name, @JsonProperty("value") String value, - @JsonProperty("comment") String comment, @JsonProperty("domain") String domain, - @JsonProperty("maxAge") int maxAge, @JsonProperty("path") String path, - @JsonProperty("secure") boolean secure, @JsonProperty("version") int version) { - + @JsonProperty("domain") String domain, @JsonProperty("maxAge") int maxAge, + @JsonProperty("path") String path, @JsonProperty("secure") boolean secure) { } } diff --git a/web/src/main/java/org/springframework/security/web/savedrequest/SavedCookie.java b/web/src/main/java/org/springframework/security/web/savedrequest/SavedCookie.java index c4d6dbce33a..5f55c16df95 100644 --- a/web/src/main/java/org/springframework/security/web/savedrequest/SavedCookie.java +++ b/web/src/main/java/org/springframework/security/web/savedrequest/SavedCookie.java @@ -35,8 +35,6 @@ public class SavedCookie implements Serializable { private final String value; - private final String comment; - private final String domain; private final int maxAge; @@ -45,28 +43,13 @@ public class SavedCookie implements Serializable { private final boolean secure; - private final int version; - - /** - * @deprecated use - * {@link org.springframework.security.web.savedrequest.SavedCookie#SavedCookie(String, String, String, int, String, boolean)} - * instead - */ - @Deprecated(forRemoval = true, since = "6.1") - public SavedCookie(String name, String value, String comment, String domain, int maxAge, String path, - boolean secure, int version) { + public SavedCookie(String name, String value, String domain, int maxAge, String path, boolean secure) { this.name = name; this.value = value; - this.comment = comment; this.domain = domain; this.maxAge = maxAge; this.path = path; this.secure = secure; - this.version = version; - } - - public SavedCookie(String name, String value, String domain, int maxAge, String path, boolean secure) { - this(name, value, null, domain, maxAge, path, secure, 0); } public SavedCookie(Cookie cookie) { @@ -82,11 +65,6 @@ public String getValue() { return this.value; } - @Deprecated(forRemoval = true, since = "6.1") - public String getComment() { - return this.comment; - } - public String getDomain() { return this.domain; } @@ -103,23 +81,14 @@ public boolean isSecure() { return this.secure; } - @Deprecated(forRemoval = true, since = "6.1") - public int getVersion() { - return this.version; - } - public Cookie getCookie() { Cookie cookie = new Cookie(getName(), getValue()); - if (getComment() != null) { - cookie.setComment(getComment()); - } if (getDomain() != null) { cookie.setDomain(getDomain()); } if (getPath() != null) { cookie.setPath(getPath()); } - cookie.setVersion(getVersion()); cookie.setMaxAge(getMaxAge()); cookie.setSecure(isSecure()); return cookie; diff --git a/web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesTests.java b/web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesTests.java index ba399db5b13..ec541fa7bcc 100644 --- a/web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesTests.java +++ b/web/src/test/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServicesTests.java @@ -362,17 +362,6 @@ public void setCookieSetsIsHttpOnlyFlagByDefault() { assertThat(cookie.isHttpOnly()).isTrue(); } - // SEC-2791 - @Test - public void setCookieMaxAge1VersionSet() { - MockRememberMeServices services = new MockRememberMeServices(); - MockHttpServletRequest request = new MockHttpServletRequest(); - MockHttpServletResponse response = new MockHttpServletResponse(); - services.setCookie(new String[] { "value" }, 1, request, response); - Cookie cookie = response.getCookie(AbstractRememberMeServices.SPRING_SECURITY_REMEMBER_ME_COOKIE_KEY); - assertThat(cookie.getVersion()).isZero(); - } - @Test public void setCookieDomainValue() { MockRememberMeServices services = new MockRememberMeServices(); diff --git a/web/src/test/java/org/springframework/security/web/firewall/FirewalledResponseTests.java b/web/src/test/java/org/springframework/security/web/firewall/FirewalledResponseTests.java index e5d7c03a4b0..4db7f46b85f 100644 --- a/web/src/test/java/org/springframework/security/web/firewall/FirewalledResponseTests.java +++ b/web/src/test/java/org/springframework/security/web/firewall/FirewalledResponseTests.java @@ -93,7 +93,6 @@ public void addCookieWhenValidThenDelegateInvoked() { Cookie cookie = new Cookie("foo", "bar"); cookie.setPath("/foobar"); cookie.setDomain("foobar"); - cookie.setComment("foobar"); this.fwResponse.addCookie(cookie); verify(this.response).addCookie(cookie); } diff --git a/web/src/test/java/org/springframework/security/web/jackson2/DefaultSavedRequestMixinTests.java b/web/src/test/java/org/springframework/security/web/jackson2/DefaultSavedRequestMixinTests.java index 6e7a8bc0541..e290e06828d 100644 --- a/web/src/test/java/org/springframework/security/web/jackson2/DefaultSavedRequestMixinTests.java +++ b/web/src/test/java/org/springframework/security/web/jackson2/DefaultSavedRequestMixinTests.java @@ -45,11 +45,9 @@ public class DefaultSavedRequestMixinTests extends AbstractMixinTests { + "\"@class\": \"org.springframework.security.web.savedrequest.SavedCookie\", " + "\"name\": \"SESSION\", " + "\"value\": \"123456789\", " - + "\"comment\": null, " + "\"maxAge\": -1, " + "\"path\": null, " + "\"secure\":false, " - + "\"version\": 0, " + "\"domain\": null" + "}]]"; // @formatter:on diff --git a/web/src/test/java/org/springframework/security/web/jackson2/SavedCookieMixinTests.java b/web/src/test/java/org/springframework/security/web/jackson2/SavedCookieMixinTests.java index 2582bddd9a0..9374654d14f 100644 --- a/web/src/test/java/org/springframework/security/web/jackson2/SavedCookieMixinTests.java +++ b/web/src/test/java/org/springframework/security/web/jackson2/SavedCookieMixinTests.java @@ -42,11 +42,9 @@ public class SavedCookieMixinTests extends AbstractMixinTests { + "\"@class\": \"org.springframework.security.web.savedrequest.SavedCookie\", " + "\"name\": \"SESSION\", " + "\"value\": \"123456789\", " - + "\"comment\": null, " + "\"maxAge\": -1, " + "\"path\": null, " + "\"secure\":false, " - + "\"version\": 0, " + "\"domain\": null" + "}"; // @formatter:on @@ -90,13 +88,11 @@ public void deserializeSavedCookieWithList() throws IOException { @Test public void deserializeSavedCookieJsonTest() throws IOException { - SavedCookie savedCookie = (SavedCookie) this.mapper.readValue(COOKIE_JSON, Object.class); + SavedCookie savedCookie = this.mapper.readValue(COOKIE_JSON, SavedCookie.class); assertThat(savedCookie).isNotNull(); assertThat(savedCookie.getName()).isEqualTo("SESSION"); assertThat(savedCookie.getValue()).isEqualTo("123456789"); assertThat(savedCookie.isSecure()).isEqualTo(false); - assertThat(savedCookie.getVersion()).isZero(); - assertThat(savedCookie.getComment()).isNull(); } } diff --git a/web/src/test/java/org/springframework/security/web/savedrequest/SavedCookieTests.java b/web/src/test/java/org/springframework/security/web/savedrequest/SavedCookieTests.java index 77d07596577..9fdee451d43 100644 --- a/web/src/test/java/org/springframework/security/web/savedrequest/SavedCookieTests.java +++ b/web/src/test/java/org/springframework/security/web/savedrequest/SavedCookieTests.java @@ -33,12 +33,10 @@ public class SavedCookieTests { @BeforeEach public void setUp() { this.cookie = new Cookie("name", "value"); - this.cookie.setComment("comment"); this.cookie.setDomain("domain"); this.cookie.setMaxAge(100); this.cookie.setPath("path"); this.cookie.setSecure(true); - this.cookie.setVersion(11); this.savedCookie = new SavedCookie(this.cookie); } @@ -52,11 +50,6 @@ public void testGetValue() { assertThat(this.savedCookie.getValue()).isEqualTo(this.cookie.getValue()); } - @Test - public void testGetComment() { - assertThat(this.savedCookie.getComment()).isEqualTo(this.cookie.getComment()); - } - @Test public void testGetDomain() { assertThat(this.savedCookie.getDomain()).isEqualTo(this.cookie.getDomain()); @@ -72,22 +65,15 @@ public void testGetPath() { assertThat(this.savedCookie.getPath()).isEqualTo(this.cookie.getPath()); } - @Test - public void testGetVersion() { - assertThat(this.savedCookie.getVersion()).isEqualTo(this.cookie.getVersion()); - } - @Test public void testGetCookie() { Cookie other = this.savedCookie.getCookie(); - assertThat(other.getComment()).isEqualTo(this.cookie.getComment()); assertThat(other.getDomain()).isEqualTo(this.cookie.getDomain()); assertThat(other.getMaxAge()).isEqualTo(this.cookie.getMaxAge()); assertThat(other.getName()).isEqualTo(this.cookie.getName()); assertThat(other.getPath()).isEqualTo(this.cookie.getPath()); assertThat(other.getSecure()).isEqualTo(this.cookie.getSecure()); assertThat(other.getValue()).isEqualTo(this.cookie.getValue()); - assertThat(other.getVersion()).isEqualTo(this.cookie.getVersion()); } @Test From dd240a6ea514601bb586ef20c366ad214d541407 Mon Sep 17 00:00:00 2001 From: Rob Winch <362503+rwinch@users.noreply.github.com> Date: Wed, 7 May 2025 14:38:09 -0500 Subject: [PATCH 2/2] SavedCookieMixinTests uses readValue(String,Object.class) The test should not provide SavedCookie.class to the ObjectMapper since this is not done in production. In particular, it provides the type that it should be deserialized, but this must be provided in the JSON since the type is unknown at the time of deserialization. Issue gh-17006 --- .../security/web/jackson2/SavedCookieMixinTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/test/java/org/springframework/security/web/jackson2/SavedCookieMixinTests.java b/web/src/test/java/org/springframework/security/web/jackson2/SavedCookieMixinTests.java index 9374654d14f..1965146a118 100644 --- a/web/src/test/java/org/springframework/security/web/jackson2/SavedCookieMixinTests.java +++ b/web/src/test/java/org/springframework/security/web/jackson2/SavedCookieMixinTests.java @@ -88,7 +88,7 @@ public void deserializeSavedCookieWithList() throws IOException { @Test public void deserializeSavedCookieJsonTest() throws IOException { - SavedCookie savedCookie = this.mapper.readValue(COOKIE_JSON, SavedCookie.class); + SavedCookie savedCookie = (SavedCookie) this.mapper.readValue(COOKIE_JSON, Object.class); assertThat(savedCookie).isNotNull(); assertThat(savedCookie.getName()).isEqualTo("SESSION"); assertThat(savedCookie.getValue()).isEqualTo("123456789");