Skip to content

[UNDERTOW-2580] Fix same site attribs handling and consolidate cookie… #1779

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.

Sign up for GitHub

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 1 commit into
base: main
Choose a base branch
from
Open

[UNDERTOW-2580] Fix same site attribs handling and consolidate cookie… #1779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 9 additions & 9 deletions core/src/main/java/io/undertow/server/Connectors.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,15 @@ public class Connectors {
}
}

KNOWN_ATTRIBUTE_NAMES.add("Path");
KNOWN_ATTRIBUTE_NAMES.add("Domain");
KNOWN_ATTRIBUTE_NAMES.add("Discard");
KNOWN_ATTRIBUTE_NAMES.add("Secure");
KNOWN_ATTRIBUTE_NAMES.add("HttpOnly");
KNOWN_ATTRIBUTE_NAMES.add("Max-Age");
KNOWN_ATTRIBUTE_NAMES.add("Expires");
KNOWN_ATTRIBUTE_NAMES.add("Comment");
KNOWN_ATTRIBUTE_NAMES.add("SameSite");
KNOWN_ATTRIBUTE_NAMES.add(Cookie.COOKIE_PATH_ATTR);
KNOWN_ATTRIBUTE_NAMES.add(Cookie.COOKIE_DOMAIN_ATTR);
KNOWN_ATTRIBUTE_NAMES.add(Cookie.COOKIE_DISCARD_ATTR);
KNOWN_ATTRIBUTE_NAMES.add(Cookie.COOKIE_SECURE_ATTR);
KNOWN_ATTRIBUTE_NAMES.add(Cookie.COOKIE_HTTP_ONLY_ATTR);
KNOWN_ATTRIBUTE_NAMES.add(Cookie.COOKIE_MAX_AGE_ATTR);
KNOWN_ATTRIBUTE_NAMES.add(Cookie.COOKIE_EXPIRES_ATTR);
KNOWN_ATTRIBUTE_NAMES.add(Cookie.COOKIE_COMMENT_ATTR);
KNOWN_ATTRIBUTE_NAMES.add(Cookie.COOKIE_SAME_SITE_ATTR);
}
/**
* Flattens the exchange cookie map into the response header map. This should be called by a
Expand Down
12 changes: 12 additions & 0 deletions core/src/main/java/io/undertow/server/handlers/Cookie.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@
*/
public interface Cookie extends Comparable {

String COOKIE_COMMENT_ATTR = "Comment";
String COOKIE_DOMAIN_ATTR = "Domain";
String COOKIE_MAX_AGE_ATTR = "Max-Age";
String COOKIE_PATH_ATTR = "Path";
String COOKIE_SECURE_ATTR = "Secure";
String COOKIE_HTTP_ONLY_ATTR = "HttpOnly";
String COOKIE_SAME_SITE_ATTR = "SameSite";
String COOKIE_DISCARD_ATTR = "Discard";
String COOKIE_EXPIRES_ATTR = "Expires";

String getName();

String getValue();
Expand Down Expand Up @@ -75,6 +85,7 @@ default boolean isSameSite() {
return false;
}

@Deprecated(forRemoval = true)
Copy link
Contributor Author

@baranowb baranowb Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any one understand what could be the use case to have value and set this flag to false?

default Cookie setSameSite(final boolean sameSite) {
throw new UnsupportedOperationException("Not implemented");
}
Expand All @@ -101,6 +112,7 @@ default String getAttribute(final String name) {
/**
* Sets an attribute for the cookie. If the value is {@code null}, the attribute is removed. If the value is not
* {@code null}, the attribute is added to the attributes for this cookie.
* If name match pre-existing attribute, like "{@link Cookie#COOKIE_PATH_ATTR}" it will override that value
*
* @param name the name of the attribute
* @param value the value of the attribute or {@code null} to remove it
Expand Down
133 changes: 123 additions & 10 deletions core/src/main/java/io/undertow/server/handlers/CookieImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,43 +19,75 @@
package io.undertow.server.handlers;

import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.HashSet;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.TreeMap;

import io.undertow.UndertowLogger;
import io.undertow.UndertowMessages;
import io.undertow.util.DateUtils;

/**
* @author Stuart Douglas
* @author <a href="mailto:[email protected]">Richard Opalka</a>
*/
public class CookieImpl implements Cookie {

private static final Set<String> STANDARD_ATTR_NAMES;
static {
Set<String> tmp = new HashSet<String>(8);
tmp.add(COOKIE_COMMENT_ATTR);
tmp.add(COOKIE_DOMAIN_ATTR);
tmp.add(COOKIE_MAX_AGE_ATTR);
tmp.add(COOKIE_PATH_ATTR);
tmp.add(COOKIE_SECURE_ATTR);
tmp.add(COOKIE_HTTP_ONLY_ATTR);
tmp.add(COOKIE_SAME_SITE_ATTR);
tmp.add(COOKIE_DISCARD_ATTR);
STANDARD_ATTR_NAMES = Collections.unmodifiableSet(tmp);
}

private static final Integer DEFAULT_MAX_AGE = Integer.valueOf(-1);
private static final boolean DEFAULT_HTTP_ONLY = false;
private static final boolean DEFAULT_SECURE = false;
private static final boolean DEFAULT_DISCARD = false;

private final String name;
private String value;
private String path;
private String domain;
private Integer maxAge;
private Integer maxAge = DEFAULT_MAX_AGE;
private Date expires;
private boolean discard;
private boolean secure;
private boolean httpOnly;
private boolean secure = DEFAULT_SECURE;
private boolean httpOnly = DEFAULT_HTTP_ONLY;
private int version = 0;
private String comment;
private boolean sameSite;
private String sameSiteMode;
private final Map<String, String> attributes;

public CookieImpl(final String name, final String value) {
this.name = name;
this.value = value;
this.attributes = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
this(name, value, null);
}

public CookieImpl(final String name) {
this(name, null);
}

public CookieImpl(final String name, final String value, final Cookie cookiePrimer) {
this.name = name;
this.value = value;
this.attributes = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
//attribs will be synced one way or ther other, might as well just iterate over attrib
if(cookiePrimer != null) {
for (Entry<String, String> primers : cookiePrimer.getAttributes().entrySet()) {
this.setAttribute(primers.getKey(), primers.getValue());
}
}
}

public String getName() {
Expand All @@ -77,6 +109,7 @@ public String getPath() {

public CookieImpl setPath(final String path) {
this.path = path;
setAttribute(COOKIE_PATH_ATTR, path, false);
return this;
}

Expand All @@ -86,6 +119,7 @@ public String getDomain() {

public CookieImpl setDomain(final String domain) {
this.domain = domain;
setAttribute(COOKIE_DOMAIN_ATTR, domain, false);
return this;
}

Expand All @@ -95,6 +129,7 @@ public Integer getMaxAge() {

public CookieImpl setMaxAge(final Integer maxAge) {
this.maxAge = maxAge;
setAttribute(COOKIE_MAX_AGE_ATTR, String.valueOf(maxAge), false);
return this;
}

Expand All @@ -104,6 +139,7 @@ public boolean isDiscard() {

public CookieImpl setDiscard(final boolean discard) {
this.discard = discard;
setAttribute(COOKIE_DISCARD_ATTR, String.valueOf(discard), false);
return this;
}

Expand All @@ -113,6 +149,7 @@ public boolean isSecure() {

public CookieImpl setSecure(final boolean secure) {
this.secure = secure;
setAttribute(COOKIE_SECURE_ATTR, String.valueOf(secure), false);
return this;
}

Expand All @@ -131,6 +168,7 @@ public boolean isHttpOnly() {

public CookieImpl setHttpOnly(final boolean httpOnly) {
this.httpOnly = httpOnly;
setAttribute(COOKIE_HTTP_ONLY_ATTR, String.valueOf(httpOnly), false);
return this;
}

Expand All @@ -140,6 +178,11 @@ public Date getExpires() {

public CookieImpl setExpires(final Date expires) {
this.expires = expires;
if(expires != null) {
setAttribute(COOKIE_EXPIRES_ATTR, DateUtils.toDateString(expires), false);
} else {
setAttribute(COOKIE_EXPIRES_ATTR, null, false);
}
return this;
}

Expand All @@ -148,18 +191,19 @@ public String getComment() {
}

public Cookie setComment(final String comment) {
setAttribute(COOKIE_COMMENT_ATTR, comment, false);
this.comment = comment;
return this;
}

@Override
public boolean isSameSite() {
return sameSite;
return this.sameSiteMode != null;
}

@Override
public Cookie setSameSite(final boolean sameSite) {
this.sameSite = sameSite;
//NOP
return this;
}

Expand All @@ -174,7 +218,7 @@ public Cookie setSameSiteMode(final String mode) {
if (m != null) {
UndertowLogger.REQUEST_LOGGER.tracef("Setting SameSite mode to [%s] for cookie [%s]", m, this.name);
this.sameSiteMode = m;
this.setSameSite(true);
setAttribute(COOKIE_SAME_SITE_ATTR, mode, false);
} else {
UndertowLogger.REQUEST_LOGGER.warnf(UndertowMessages.MESSAGES.invalidSameSiteMode(mode, Arrays.toString(CookieSameSiteMode.values())), "Ignoring specified SameSite mode [%s] for cookie [%s]", mode, this.name);
}
Expand All @@ -188,9 +232,78 @@ public String getAttribute(final String name) {

@Override
public Cookie setAttribute(final String name, final String value) {
return setAttribute(name, value, true);
}

protected Cookie setAttribute(final String name, final String value, boolean performSync) {
// less than ideal, but users may want to fiddle with it like that, we need to sync
if (value != null) {
if (performSync) {
switch (name) {
case COOKIE_COMMENT_ATTR:
this.comment = value;
break;
case COOKIE_DOMAIN_ATTR:
this.domain = value;
break;
case COOKIE_HTTP_ONLY_ATTR:
this.httpOnly = Boolean.parseBoolean(value);
break;
case COOKIE_MAX_AGE_ATTR:
this.maxAge = Integer.parseInt(value);
break;
case COOKIE_PATH_ATTR:
this.path = value;
break;
case COOKIE_SAME_SITE_ATTR:
// enum will match constant name, no inner representation
this.sameSiteMode = CookieSameSiteMode.valueOf(value.toUpperCase()).toString();
break;
case COOKIE_SECURE_ATTR:
this.secure = Boolean.valueOf(value);
break;
case COOKIE_DISCARD_ATTR:
this.discard = Boolean.valueOf(value);
break;
case COOKIE_EXPIRES_ATTR:
this.expires = DateUtils.parseDate(value);
break;
}
}

attributes.put(name, value);
} else {
switch (name) {
case COOKIE_COMMENT_ATTR:
this.comment = null;
break;
case COOKIE_DOMAIN_ATTR:
this.domain = null;
break;
case COOKIE_HTTP_ONLY_ATTR:
this.httpOnly = DEFAULT_HTTP_ONLY;
break;
case COOKIE_MAX_AGE_ATTR:
this.maxAge = DEFAULT_MAX_AGE;
break;
case COOKIE_PATH_ATTR:
this.path = null;
break;
case COOKIE_SAME_SITE_ATTR:
// enum will match constant name, no inner representation
this.sameSiteMode = null;
break;
case COOKIE_SECURE_ATTR:
this.secure = DEFAULT_SECURE;
break;
case COOKIE_DISCARD_ATTR:
this.discard = DEFAULT_DISCARD;
break;
case COOKIE_EXPIRES_ATTR:
this.expires = null;
break;
}

attributes.remove(name);
}
return this;
Expand Down
Loading