From 9ea45697ac5f99bd8f62a8391bdec4bbe35efab1 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Thu, 30 Jun 2022 16:51:43 +0200 Subject: [PATCH] Support cookie comments in MockHttpServletResponse and MockCookie Prior to this commit, if a cookie was added to MockHttpServletResponse, the comment attribute was not included in the generated Set-Cookie header. In addition, MockCookie.parse(String) did not support the Comment attribute. This commit addresses both of these issues. Closes gh-28730 --- .../springframework/mock/web/MockCookie.java | 5 +- .../mock/web/MockHttpServletResponse.java | 5 +- .../mock/web/MockCookieTests.java | 5 +- .../web/MockHttpServletResponseTests.java | 59 ++++++++++++++++--- .../web/testfixture/servlet/MockCookie.java | 26 +++++++- .../servlet/MockHttpServletResponse.java | 5 +- 6 files changed, 89 insertions(+), 16 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockCookie.java b/spring-test/src/main/java/org/springframework/mock/web/MockCookie.java index 759eae8f9f0..b55c5beafd1 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockCookie.java +++ b/spring-test/src/main/java/org/springframework/mock/web/MockCookie.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -142,6 +142,9 @@ public class MockCookie extends Cookie { else if (StringUtils.startsWithIgnoreCase(attribute, "SameSite")) { cookie.setSameSite(extractAttributeValue(attribute, setCookieHeader)); } + else if (StringUtils.startsWithIgnoreCase(attribute, "Comment")) { + cookie.setComment(extractAttributeValue(attribute, setCookieHeader)); + } } return cookie; } diff --git a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java index a1b3eca3b08..50c28212e7e 100644 --- a/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java +++ b/spring-test/src/main/java/org/springframework/mock/web/MockHttpServletResponse.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -450,6 +450,9 @@ public class MockHttpServletResponse implements HttpServletResponse { buf.append("; SameSite=").append(mockCookie.getSameSite()); } } + if (StringUtils.hasText(cookie.getComment())) { + buf.append("; Comment=").append(cookie.getComment()); + } return buf.toString(); } diff --git a/spring-test/src/test/java/org/springframework/mock/web/MockCookieTests.java b/spring-test/src/test/java/org/springframework/mock/web/MockCookieTests.java index 23dcd6bd688..9cb2247f8eb 100644 --- a/spring-test/src/test/java/org/springframework/mock/web/MockCookieTests.java +++ b/spring-test/src/test/java/org/springframework/mock/web/MockCookieTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 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. @@ -67,7 +67,7 @@ class MockCookieTests { @Test void parseHeaderWithAttributes() { - MockCookie cookie = MockCookie.parse("SESSION=123; Domain=example.com; Max-Age=60; " + + MockCookie cookie = MockCookie.parse("SESSION=123; Comment=Session Cookie; Domain=example.com; Max-Age=60; " + "Expires=Tue, 8 Oct 2019 19:50:00 GMT; Path=/; Secure; HttpOnly; SameSite=Lax"); assertCookie(cookie, "SESSION", "123"); @@ -79,6 +79,7 @@ class MockCookieTests { assertThat(cookie.getExpires()).isEqualTo(ZonedDateTime.parse("Tue, 8 Oct 2019 19:50:00 GMT", DateTimeFormatter.RFC_1123_DATE_TIME)); assertThat(cookie.getSameSite()).isEqualTo("Lax"); + assertThat(cookie.getComment()).isEqualTo("Session Cookie"); } @ParameterizedTest diff --git a/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java b/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java index fe0f90f7b50..6ab8fc8e163 100644 --- a/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java +++ b/spring-test/src/test/java/org/springframework/mock/web/MockHttpServletResponseTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -35,6 +35,7 @@ import org.springframework.web.util.WebUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.InstanceOfAssertFactories.type; import static org.assertj.core.api.SoftAssertions.assertSoftly; import static org.springframework.http.HttpHeaders.CONTENT_LANGUAGE; import static org.springframework.http.HttpHeaders.CONTENT_LENGTH; @@ -258,7 +259,7 @@ class MockHttpServletResponseTests { @Test void cookies() { - Cookie cookie = new Cookie("foo", "bar"); + Cookie cookie = new MockCookie("foo", "bar"); cookie.setPath("/path"); cookie.setDomain("example.com"); cookie.setMaxAge(0); @@ -474,6 +475,23 @@ class MockHttpServletResponseTests { assertThat(header).startsWith("SESSION=123; Path=/; Max-Age=100; Expires="); } + /** + * @since 5.3.22 + */ + @Test + void setCookieHeaderWithComment() { + response.setHeader(SET_COOKIE, "SESSION=123;Comment=Test Comment;Path=/"); + + assertThat(response.getHeader(SET_COOKIE)).isEqualTo(("SESSION=123; Path=/; Comment=Test Comment")); + + assertNumCookies(1); + assertThat(response.getCookies()[0]).isInstanceOf(MockCookie.class).satisfies(mockCookie -> { + assertThat(mockCookie.getName()).isEqualTo("SESSION"); + assertThat(mockCookie.getPath()).isEqualTo("/"); + assertThat(mockCookie.getComment()).isEqualTo("Test Comment"); + }); + } + @Test void addCookieHeader() { response.addHeader(SET_COOKIE, "SESSION=123; Path=/; Secure; HttpOnly; SameSite=Lax"); @@ -487,6 +505,26 @@ class MockHttpServletResponseTests { assertCookieValues("123", "999"); } + @Test + void addCookieHeaderWithComment() { + response.addHeader(SET_COOKIE, "SESSION=123; Path=/; Secure; HttpOnly; SameSite=Lax"); + assertNumCookies(1); + assertPrimarySessionCookie("123"); + + // Adding a 2nd cookie header should result in 2 cookies. + response.addHeader(SET_COOKIE, "SESSION=999; Comment=Test Comment; Path=/; Secure; HttpOnly; SameSite=Lax"); + assertNumCookies(2); + assertPrimarySessionCookie("123"); + assertThat(response.getCookies()[1]).isInstanceOf(MockCookie.class).satisfies(mockCookie -> { + assertThat(mockCookie.getName()).isEqualTo("SESSION"); + assertThat(mockCookie.getValue()).isEqualTo("999"); + assertThat(mockCookie.getComment()).isEqualTo("Test Comment"); + assertThat(mockCookie.getPath()).isEqualTo("/"); + assertThat(mockCookie.getSecure()).isTrue(); + assertThat(mockCookie.isHttpOnly()).isTrue(); + }); + } + /** * @since 5.1.11 */ @@ -570,13 +608,16 @@ class MockHttpServletResponseTests { private void assertPrimarySessionCookie(String expectedValue) { Cookie cookie = this.response.getCookie("SESSION"); - assertThat(cookie).isInstanceOf(MockCookie.class); - assertThat(cookie.getName()).isEqualTo("SESSION"); - assertThat(cookie.getValue()).isEqualTo(expectedValue); - assertThat(cookie.getPath()).isEqualTo("/"); - assertThat(cookie.getSecure()).isTrue(); - assertThat(cookie.isHttpOnly()).isTrue(); - assertThat(((MockCookie) cookie).getSameSite()).isEqualTo("Lax"); + assertThat(cookie).asInstanceOf(type(MockCookie.class)).satisfies(mockCookie -> { + assertThat(mockCookie.getName()).isEqualTo("SESSION"); + assertThat(mockCookie.getValue()).isEqualTo(expectedValue); + assertThat(mockCookie.getPath()).isEqualTo("/"); + assertThat(mockCookie.getSecure()).isTrue(); + assertThat(mockCookie.isHttpOnly()).isTrue(); + assertThat(mockCookie.getComment()).isNull(); + assertThat(mockCookie.getExpires()).isNull(); + assertThat(mockCookie.getSameSite()).isEqualTo("Lax"); + }); } @Test // gh-25501 diff --git a/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockCookie.java b/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockCookie.java index 8f2e5eaa975..018c90b54ca 100644 --- a/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockCookie.java +++ b/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockCookie.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 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. @@ -22,6 +22,7 @@ import java.time.format.DateTimeFormatter; import javax.servlet.http.Cookie; +import org.springframework.core.style.ToStringCreator; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -67,8 +68,8 @@ public class MockCookie extends Cookie { /** * Get the "Expires" attribute for this cookie. - * @since 5.1.11 * @return the "Expires" attribute for this cookie, or {@code null} if not set + * @since 5.1.11 */ @Nullable public ZonedDateTime getExpires() { @@ -141,6 +142,9 @@ public class MockCookie extends Cookie { else if (StringUtils.startsWithIgnoreCase(attribute, "SameSite")) { cookie.setSameSite(extractAttributeValue(attribute, setCookieHeader)); } + else if (StringUtils.startsWithIgnoreCase(attribute, "Comment")) { + cookie.setComment(extractAttributeValue(attribute, setCookieHeader)); + } } return cookie; } @@ -152,4 +156,22 @@ public class MockCookie extends Cookie { return nameAndValue[1]; } + @Override + public String toString() { + return new ToStringCreator(this) + .append("name", getName()) + .append("value", getValue()) + .append("Path", getPath()) + .append("Domain", getDomain()) + .append("Version", getVersion()) + .append("Comment", getComment()) + .append("Secure", getSecure()) + .append("HttpOnly", isHttpOnly()) + .append("SameSite", this.sameSite) + .append("Max-Age", getMaxAge()) + .append("Expires", (this.expires != null ? + DateTimeFormatter.RFC_1123_DATE_TIME.format(this.expires) : null)) + .toString(); + } + } diff --git a/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockHttpServletResponse.java b/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockHttpServletResponse.java index 4a343c8344c..dc46769a197 100644 --- a/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockHttpServletResponse.java +++ b/spring-web/src/testFixtures/java/org/springframework/web/testfixture/servlet/MockHttpServletResponse.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -450,6 +450,9 @@ public class MockHttpServletResponse implements HttpServletResponse { buf.append("; SameSite=").append(mockCookie.getSameSite()); } } + if (StringUtils.hasText(cookie.getComment())) { + buf.append("; Comment=").append(cookie.getComment()); + } return buf.toString(); }