Skip to content

#16889: Remove deprecated cookie comment and version methods #17006

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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").asBoolean());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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;
}
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ public class CookieMixinTests extends AbstractMixinTests {
" \"name\": \"demo\"," +
" \"value\": \"cookie1\"," +
" \"attributes\":{\"@class\":\"java.util.Collections$EmptyMap\"}," +
" \"comment\": null," +
" \"comment\": null," +
" \"maxAge\": -1," +
" \"path\": null," +
" \"secure\": false," +
" \"version\": 0," +
" \"version\": 0," +
" \"domain\": null" +
"}";
// @formatter:on
Expand All @@ -53,11 +53,11 @@ public class CookieMixinTests extends AbstractMixinTests {
" \"name\": \"demo\"," +
" \"value\": \"cookie1\"," +
" \"attributes\":{\"@class\":\"java.util.Collections$UnmodifiableMap\", \"HttpOnly\": \"true\"}," +
" \"comment\": null," +
" \"comment\": null," +
" \"maxAge\": -1," +
" \"path\": null," +
" \"secure\": false," +
" \"version\": 0," +
" \"version\": 0," +
" \"domain\": null" +
"}";
// @formatter:on
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,10 @@ public class SavedCookieMixinTests extends AbstractMixinTests {
+ "\"@class\": \"org.springframework.security.web.savedrequest.SavedCookie\", "
+ "\"name\": \"SESSION\", "
+ "\"value\": \"123456789\", "
+ "\"comment\": null, "
+ "\"domain\": null, "
+ "\"maxAge\": -1, "
+ "\"path\": null, "
+ "\"secure\":false, "
+ "\"version\": 0, "
+ "\"domain\": null"
+ "\"secure\":false "
+ "}";
// @formatter:on
// @formatter:off
Expand Down Expand Up @@ -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();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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());
Expand All @@ -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
Expand Down