Skip to content

Commit

Permalink
Make solution more generic based on joakime's feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
markt-asf committed Mar 26, 2024
1 parent 378c623 commit 7faa086
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 19 deletions.
36 changes: 25 additions & 11 deletions api/src/main/java/jakarta/servlet/http/Cookie.java
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public class Cookie implements Cloneable, Serializable {
private static final String PATH = "Path"; // ;Path=VALUE ... URLs that see the cookie
private static final String SECURE = "Secure"; // ;Secure ... e.g. use SSL
private static final String HTTP_ONLY = "HttpOnly";
private static final String EMPTY_STRING = "";

private static final ResourceBundle lStrings = ResourceBundle.getBundle(LSTRING_FILE);

Expand Down Expand Up @@ -272,7 +273,11 @@ public String getPath() {
* @see #getSecure
*/
public void setSecure(boolean flag) {
putAttribute(SECURE, String.valueOf(flag));
if (flag) {
putAttribute(SECURE, EMPTY_STRING);
} else {
putAttribute(SECURE, null);
}
}

/**
Expand All @@ -284,7 +289,7 @@ public void setSecure(boolean flag) {
* @see #setSecure
*/
public boolean getSecure() {
return Boolean.parseBoolean(getAttribute(SECURE));
return EMPTY_STRING.equals(getAttribute(SECURE));
}

/**
Expand Down Expand Up @@ -417,7 +422,11 @@ public Object clone() {
* @since Servlet 3.0
*/
public void setHttpOnly(boolean httpOnly) {
putAttribute(HTTP_ONLY, String.valueOf(httpOnly));
if (httpOnly) {
putAttribute(HTTP_ONLY, EMPTY_STRING);
} else {
putAttribute(HTTP_ONLY, null);
}
}

/**
Expand All @@ -428,7 +437,7 @@ public void setHttpOnly(boolean httpOnly) {
* @since Servlet 3.0
*/
public boolean isHttpOnly() {
return Boolean.parseBoolean(getAttribute(HTTP_ONLY));
return EMPTY_STRING.equals(getAttribute(HTTP_ONLY));
}

/**
Expand All @@ -440,13 +449,18 @@ public boolean isHttpOnly() {
* <code>cookie.getDomain()</code> should return exactly that value, and vice versa.
*
* <p>
* Attributes with a value of the empty string will appear as {@code "attribute-name="} in any {@code Set-Cookie} header
* generated from the {@code Cookie} instance unless special handling is required as defined in the following paragraph.
*
* <p>
* Attributes that are defined as not having a value, such as {@code HttpOnly}, must set with a value such that {@code
* Boolean.parseBoolean(attribute-value)} evaluates to {@code true} for the attribute to be included in any
* {@code Set-Cookie} header generated from the {@code Cookie} instance, otherwise the attribute is not included.
* Attributes with a value of the empty string will appear as {@code "attribute-name"} (a cookie attribute with neither
* a name or value) in any {@code Set-Cookie} header generated from the {@code Cookie} instance.
*
* <pre>{@code
* setAttribute("name", "value"); // becomes "name=value;"
* setAttribute("name", ""); // becomes "name;"
* setAttribute("name", null); // removes "name" from the cookie
* setAttribute("HttpOnly", ""); // becomes "HttpOnly;"
* setAttribute("Partitioned", ""); // becomes "Partitioned;
* setAttribute("SameSite", "Strict"); // becomes "SameSite=Strict;"
* }
* </pre>
*
* @param name the name of the cookie attribute to set the value for, case insensitive
*
Expand Down
17 changes: 9 additions & 8 deletions api/src/test/java/jakarta/servlet/http/CookieTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
Expand Down Expand Up @@ -131,12 +132,12 @@ public void testSecure() {
cookie.setSecure(true);
assertThat(cookie.getSecure(), is(true));
assertThat(cookie.getAttributes().keySet(), contains("Secure"));
assertThat(cookie.getAttributes().values(), contains("true"));
assertThat(cookie.getAttributes().values(), contains(""));
cookie.setSecure(false);
assertThat(cookie.getSecure(), is(false));
assertThat(cookie.getAttributes().keySet(), contains("Secure"));
assertThat(cookie.getAttributes().values(), contains("false"));
cookie.setAttribute("Secure", "true");
assertThat(cookie.getAttributes().keySet(), empty());
assertThat(cookie.getAttributes().values(), empty());
cookie.setAttribute("Secure", "");
assertThat(cookie.getSecure(), is(true));
cookie.setAttribute("Secure", "other");
assertThat(cookie.getAttributes().keySet(), contains("Secure"));
Expand Down Expand Up @@ -177,12 +178,12 @@ public void testHttpOnly() {
cookie.setHttpOnly(true);
assertThat(cookie.isHttpOnly(), is(true));
assertThat(cookie.getAttributes().keySet(), contains("HttpOnly"));
assertThat(cookie.getAttributes().values(), contains("true"));
assertThat(cookie.getAttributes().values(), contains(""));
cookie.setHttpOnly(false);
assertThat(cookie.isHttpOnly(), is(false));
assertThat(cookie.getAttributes().keySet(), contains("HttpOnly"));
assertThat(cookie.getAttributes().values(), contains("false"));
cookie.setAttribute("HttpOnly", "true");
assertThat(cookie.getAttributes().keySet(), empty());
assertThat(cookie.getAttributes().values(), empty());
cookie.setAttribute("HttpOnly", "");
assertThat(cookie.isHttpOnly(), is(true));
cookie.setAttribute("HttpOnly", "other");
assertThat(cookie.getAttributes().keySet(), contains("HttpOnly"));
Expand Down
5 changes: 5 additions & 0 deletions spec/src/main/asciidoc/servlet-spec-body.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -8691,6 +8691,11 @@ Provide a mechanism to allow applications to interact with the `HttpSession`
object outside of the standard HTTP request processing. This is expected to be
particularly useful to applications using the Jakarta WebSocket API.

link:https://github.com/eclipse-ee4j/servlet-api/issues/571[Issue 571]::
Clarify the behaviour of `Cookie.setAttribute(String,String)` and
`Cookie.getAttribute(String)` with cookie attributes such as `HttpOnly`,
`Secure` and `Partitioned` that do not have a value.

=== Changes Since Jakarta Servlet 5.0

The minimum Java version has been increased to Java 11.
Expand Down

0 comments on commit 7faa086

Please sign in to comment.