Support cookie comments in MockHttpServletResponse and MockCookie

sbrannen · sbrannen · commit 9ea45697ac5f · 2022-06-30T16:51:43.000+02:00
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
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
@@ -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 @@ else if (StringUtils.startsWithIgnoreCase(attribute, "HttpOnly")) {
 			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
@@ -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 @@ else if (expires != null) {
 				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
@@ -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 @@ void parseHeaderWithoutAttributes() {
 
 	@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 @@ void parseHeaderWithAttributes() {
 		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
@@ -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 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 @@ void httpHeaderNameCasingIsPreserved() throws Exception {
 
 	@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 @@ void setCookieHeaderWithZeroExpiresAttribute() {
 		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 @@ void addCookieHeader() {
 		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 @@ private void assertCookieValues(String... expected) {
 
 	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
@@ -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 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 void setExpires(@Nullable ZonedDateTime expires) {
 
 	/**
 	 * 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 @@ else if (StringUtils.startsWithIgnoreCase(attribute, "HttpOnly")) {
 			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 @@ private static String extractAttributeValue(String attribute, String header) {
 		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
@@ -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 @@ else if (expires != null) {
 				buf.append("; SameSite=").append(mockCookie.getSameSite());
 			}
 		}
+		if (StringUtils.hasText(cookie.getComment())) {
+			buf.append("; Comment=").append(cookie.getComment());
+		}
 		return buf.toString();
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright 2002-2021 the original author or authors.`
	`2`	`+ * Copyright 2002-2022 the original author or authors.`
`3`	`3`	`*`
`4`	`4`	`* Licensed under the Apache License, Version 2.0 (the "License");`
`5`	`5`	`* you may not use this file except in compliance with the License.`
`@@ -142,6 +142,9 @@ else if (StringUtils.startsWithIgnoreCase(attribute, "HttpOnly")) {`
`142`	`142`	`else if (StringUtils.startsWithIgnoreCase(attribute, "SameSite")) {`
`143`	`143`	`cookie.setSameSite(extractAttributeValue(attribute, setCookieHeader));`
`144`	`144`	`}`
	`145`	`+ else if (StringUtils.startsWithIgnoreCase(attribute, "Comment")) {`
	`146`	`+ cookie.setComment(extractAttributeValue(attribute, setCookieHeader));`
	`147`	`+ }`
`145`	`148`	`}`
`146`	`149`	`return cookie;`
`147`	`150`	`}`