Skip to content

[UNDERTOW-2580] Fix SameSite and cookie attributes #1761

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

Closed
wants to merge 1 commit into from
Closed

[UNDERTOW-2580] Fix SameSite and cookie attributes #1761

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
26 changes: 26 additions & 0 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 @@ -41,6 +41,8 @@
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Date;
import java.util.Iterator;
import java.util.Map.Entry;
import java.util.concurrent.Executor;
import java.util.concurrent.RejectedExecutionException;

Expand Down Expand Up @@ -270,6 +272,14 @@ private static String addRfc6265ResponseCookieToExchange(final Cookie cookie) {
header.append(cookie.getSameSiteMode());
}
}

final Iterator<Entry<String, String>> attributes = cookie.getAttributes().entrySet().iterator();
while(attributes.hasNext()) {
Entry<String, String> attr = attributes.next();
header.append("; ").append(attr.getKey()).append("=");
header.append(attr.getValue());
}

return header.toString();
}

Expand Down Expand Up @@ -320,6 +330,14 @@ private static String addVersion0ResponseCookieToExchange(final Cookie cookie) {
header.append(cookie.getSameSiteMode());
}
}

final Iterator<Entry<String, String>> attributes = cookie.getAttributes().entrySet().iterator();
while(attributes.hasNext()) {
Entry<String, String> attr = attributes.next();
header.append("; ").append(attr.getKey()).append("=");
header.append(attr.getValue());
}

return header.toString();

}
Expand Down Expand Up @@ -386,6 +404,14 @@ private static String addVersion1ResponseCookieToExchange(final Cookie cookie) {
header.append(cookie.getSameSiteMode());
}
}

final Iterator<Entry<String, String>> attributes = cookie.getAttributes().entrySet().iterator();
while(attributes.hasNext()) {
Entry<String, String> attr = attributes.next();
header.append("; ").append(attr.getKey()).append("=");
header.append(attr.getValue());
}

return header.toString();
}

Expand Down
5 changes: 5 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 @@ -19,6 +19,7 @@
package io.undertow.server.handlers;

import java.util.Date;
import java.util.Map;

/**
* A HTTP cookie.
Expand Down Expand Up @@ -86,6 +87,10 @@ default Cookie setSameSiteMode(final String mode) {
throw new UnsupportedOperationException("Not implemented");
}

void setAttributes(Map<String, String> nonStandardAttributes);

Map<String, String> getAttributes();

@Override
default int compareTo(final Object other) {
final Cookie o = (Cookie) other;
Expand Down
28 changes: 19 additions & 9 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 @@ -20,6 +20,7 @@

import java.util.Arrays;
import java.util.Date;
import java.util.Map;

import io.undertow.UndertowLogger;
import io.undertow.UndertowMessages;
Expand All @@ -41,8 +42,8 @@ public class CookieImpl implements Cookie {
private boolean httpOnly;
private int version = 0;
private String comment;
private boolean sameSite;
private String sameSiteMode;
private Map<String, String> attributes;

public CookieImpl(final String name, final String value) {
this.name = name;
Expand Down Expand Up @@ -142,21 +143,22 @@ public String getComment() {
return comment;
}

public Cookie setComment(final String comment) {
public CookieImpl setComment(final String comment) {
this.comment = comment;
return this;
}

@Override
public boolean isSameSite() {
return sameSite;
return this.sameSiteMode != null; //"None" should be false?
}

@Override
public Cookie setSameSite(final boolean sameSite) {
this.sameSite = sameSite;
return this;
}
// default to interface not-implemented for the time being.
// @Override
// public Cookie setSameSite(final boolean sameSite) {
// this.sameSite = sameSite;
// return this;
// }

@Override
public String getSameSiteMode() {
Expand All @@ -169,13 +171,21 @@ 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);
//this.setSameSite(true);
} else {
UndertowLogger.REQUEST_LOGGER.warnf(UndertowMessages.MESSAGES.invalidSameSiteMode(mode, Arrays.toString(CookieSameSiteMode.values())), "Ignoring specified SameSite mode [%s] for cookie [%s]", mode, this.name);
}
return this;
}

public void setAttributes(Map<String, String> nonStanradAttributes) {
this.attributes = nonStanradAttributes;
}

public Map<String, String> getAttributes(){
return this.attributes;
}

@Override
public final int hashCode() {
int result = 17;
Expand Down
142 changes: 133 additions & 9 deletions core/src/main/java/io/undertow/server/session/SessionCookieConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,19 @@

package io.undertow.server.session;

import java.util.Collections;
import java.util.HashSet;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.TreeMap;
import java.util.stream.Collectors;

import io.undertow.UndertowLogger;
import io.undertow.server.HttpServerExchange;
import io.undertow.server.handlers.Cookie;
import io.undertow.server.handlers.CookieImpl;
import io.undertow.server.handlers.CookieSameSiteMode;

/**
* Encapsulation of session cookie configuration. This removes the need for the session manager to
Expand All @@ -34,15 +43,42 @@ public class SessionCookieConfig implements SessionConfig {

public static final String DEFAULT_SESSION_ID = "JSESSIONID";

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

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 int DEFAULT_MAX_AGE = -1;
private static final boolean DEFAULT_HTTP_ONLY = false;
private static final boolean DEFAULT_SECURE = false;

private String cookieName = DEFAULT_SESSION_ID;
private boolean discard;
private String path = "/";
private String domain;
private boolean discard;
private boolean secure;
private boolean httpOnly;
private int maxAge = -1;
private boolean secure = DEFAULT_SECURE;
private boolean httpOnly = DEFAULT_HTTP_ONLY;
private int maxAge = DEFAULT_MAX_AGE;
private String comment;

private CookieSameSiteMode sameSite;
private final Map<String,String> attributes = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);

@Override
public String rewriteUrl(final String originalUrl, final String sessionId) {
Expand All @@ -51,7 +87,7 @@ public String rewriteUrl(final String originalUrl, final String sessionId) {

@Override
public void setSessionId(final HttpServerExchange exchange, final String sessionId) {
Cookie cookie = new CookieImpl(cookieName, sessionId)
CookieImpl cookie = new CookieImpl(cookieName, sessionId)
.setPath(path)
.setDomain(domain)
.setDiscard(discard)
Expand All @@ -61,19 +97,27 @@ public void setSessionId(final HttpServerExchange exchange, final String session
if (maxAge > 0) {
cookie.setMaxAge(maxAge);
}
if(this.sameSite != null) {
cookie.setSameSiteMode(String.valueOf(this.sameSite));
}
cookie.setAttributes(getNonStandardAttributes());
exchange.setResponseCookie(cookie);
UndertowLogger.SESSION_LOGGER.tracef("Setting session cookie session id %s on %s", sessionId, exchange);
}

@Override
public void clearSession(final HttpServerExchange exchange, final String sessionId) {
Cookie cookie = new CookieImpl(cookieName, sessionId)
CookieImpl cookie = new CookieImpl(cookieName, sessionId)
.setPath(path)
.setDomain(domain)
.setDiscard(discard)
.setSecure(secure)
.setHttpOnly(httpOnly)
.setMaxAge(0);
if(this.sameSite != null) {
cookie.setSameSiteMode(String.valueOf(this.sameSite));
}
cookie.setAttributes(getNonStandardAttributes());
exchange.setResponseCookie(cookie);
UndertowLogger.SESSION_LOGGER.tracef("Clearing session cookie session id %s on %s", sessionId, exchange);
}
Expand Down Expand Up @@ -108,6 +152,7 @@ public String getPath() {

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

Expand All @@ -117,6 +162,7 @@ public String getDomain() {

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

Expand All @@ -126,24 +172,27 @@ public boolean isDiscard() {

public SessionCookieConfig setDiscard(final boolean discard) {
this.discard = discard;
//atr ?
return this;
}

public boolean isSecure() {
return secure;
return this.secure;
}

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

public boolean isHttpOnly() {
return httpOnly;
return this.httpOnly;
}

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

Expand All @@ -153,6 +202,7 @@ public int getMaxAge() {

public SessionCookieConfig setMaxAge(final int maxAge) {
this.maxAge = maxAge;
setAttribute(COOKIE_MAX_AGE_ATTR, String.valueOf(maxAge));
return this;
}

Expand All @@ -162,6 +212,80 @@ public String getComment() {

public SessionCookieConfig setComment(final String comment) {
this.comment = comment;
setAttribute(COOKIE_COMMENT_ATTR, comment);
Copy link
Contributor Author

@baranowb baranowb Sep 4, 2025

Choose a reason for hiding this comment

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

setAttribute(COOKIE_SAME_SITE_ATTR, String.valueOf(sameSite), false); - needs flag.

return this;
}

public CookieSameSiteMode getSameSite() {
return this.sameSite;
}

public SessionCookieConfig setSameSite(final CookieSameSiteMode sameSite) {
this.sameSite = sameSite;
setAttribute(COOKIE_SAME_SITE_ATTR, String.valueOf(sameSite), false);
return this;
}

public boolean isSameSite() {
//should be fale for NONE as well?
return this.sameSite != null;
}
public SessionCookieConfig setAttribute(final String name, final String value) {
return setAttribute(name, value, true);
}

protected SessionCookieConfig 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(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.sameSite = CookieSameSiteMode.valueOf(value.toUpperCase());
break;
case COOKIE_SECURE_ATTR:
this.secure = Boolean.valueOf(value);
break;
case COOKIE_DISCARD_ATTR:
this.discard = Boolean.valueOf(value);
break;
}
}
attributes.put(name, value);
return this;
}

public String getAttribute(final String name) {
return attributes.get(name);
}

public Map<String, String> getAttributes() {
return Collections.unmodifiableMap(attributes);
}

/**
* Get non standard attribute map.
* @return
*/
protected Map<String, String> getNonStandardAttributes(){
//standard attribs are handled directly. We need to remove those from attribs and present
//during cookie dough kneeding so all of them end up sent over wire
return Collections.unmodifiableMap(attributes.entrySet().stream().filter(entry -> {
return !STANDARD_ATTR_NAMES.contains(entry.getKey());
}).collect(Collectors.toMap(Entry<String, String>::getKey, Entry<String, String>::getValue)));
}
}
2 changes: 1 addition & 1 deletion core/src/main/java/io/undertow/util/Cookies.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ private static void handleValue(CookieImpl cookie, String key, String value) {
} else if (key.equalsIgnoreCase("comment")) {
cookie.setComment(value);
} else if (key.equalsIgnoreCase("samesite")) {
cookie.setSameSite(true);
//cookie.setSameSite(true);
cookie.setSameSiteMode(value);
}
//otherwise ignore this key-value pair
Expand Down
Loading
Loading