Skip to content
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

Fix #571 - clarify behaviour of Cookie.setAttribute(String, String) #572

Merged
merged 2 commits into from
Mar 26, 2024
Merged
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
36 changes: 30 additions & 6 deletions api/src/main/java/jakarta/servlet/http/Cookie.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2023 Oracle and/or its affiliates and others.
* Copyright (c) 1997, 2024 Oracle and/or its affiliates and others.
* All rights reserved.
* Copyright 2004 The Apache Software Foundation
*
Expand Down 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 @@ -274,7 +275,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 @@ -286,7 +291,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 @@ -419,7 +424,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 @@ -430,7 +439,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 @@ -441,9 +450,24 @@ public boolean isHttpOnly() {
* except for <code>version</code>. E.g. when <code>cookie.setAttribute("domain", domain)</code> is invoked, then
* <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"} (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
*
* @param value the value of the cookie attribute associated with the given name, can be {@code null}
* @param value the value of the cookie attribute associated with the given name, can be {@code null} which removes the
* attribute with the given name if present
*
* @throws IllegalArgumentException if the cookie name is null or empty or contains any illegal characters (for example,
* a comma, space, or semicolon) or matches a token reserved for use by the cookie protocol.
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
Loading