diff --git a/changes.md b/changes.md index fea3b6525b..936b8106cc 100644 --- a/changes.md +++ b/changes.md @@ -61,6 +61,16 @@ Changes log - Removed the `Protocol.WAR` constant ("war" scheme, Web Archive Access Protocol), a leftover of the Servlet mode, which is no longer supported. `Protocol.valueOf("war")` now returns a new, unknown protocol. Use CLAP or FILE references instead. + - Cookies now follow RFC 6265 only, the obsolete RFC 2109 and RFC 2965 formats are dropped. + Removed `Cookie.get/setVersion()`, `CookieSetting.get/setComment()` and the constructors + of `Cookie` and `CookieSetting` taking a version or a comment, as well as the + `Set-Cookie2` support (`HeaderConstants.HEADER_SET_COOKIE2`): a `Set-Cookie2` response + header is no longer parsed as a cookie setting. `Set-Cookie` no longer writes `Version`, + `Comment` or `Discard`, nor quotes values and attributes, and writes `Max-Age` together + with `Expires`. The request `Cookie` header no longer writes `$Version`, `$Path` and + `$Domain`. When reading, these legacy pairs and attributes are silently ignored, and a + comma is no longer a separator between cookies (several `Cookie` headers are joined with + semicolons). `Max-Age` now takes precedence over `Expires` when both are present. - Enhancements - Added the `EventStreamRepresentation`, `ServerSentEvent` and `EventStreamReader` APIs for streaming and reading Server-Sent Events over the HTTP connectors, including optional @@ -93,10 +103,15 @@ Changes log - HTTP/2 and HTTP/3 requests advertise `te: trailers`; stream header names are normalized to lowercase at the HTTP boundary. - Added the `Protocol.STDIO` pseudo-protocol constant for standard input/output connectors. + - Added the `SameSite` attribute to cookie settings (`CookieSetting.get/setSameSite()`, + `SameSite` enum), read and written in `Set-Cookie`. - Bugs fixed - Fixed the OpenAPI extension build path for Commons Lang types inherited from Swagger. - Slf4j bridge reports wrong line number and caller class in log4j. Issue #233. - When GET is an allowed method of a server resource, HEAD is now also listed (e.g. in the Allow header of a 405 response). Issue #266. + - The `Max-Age` and `Path` attributes of a cookie setting are no longer quoted. Issue #1334. + - `CookieAuthenticator` no longer lets the next restlet overwrite the redirection sent after + a login. Issue #554. - Misc - Fixed JDK HTTP client accessibility in core and WebSocket tests in Eclipse and VS Code, without adding a production module dependency. diff --git a/org.restlet.ext.crypto/src/main/java/org/restlet/ext/crypto/CookieAuthenticator.java b/org.restlet.ext.crypto/src/main/java/org/restlet/ext/crypto/CookieAuthenticator.java index cb38076388..9eadf57e7f 100644 --- a/org.restlet.ext.crypto/src/main/java/org/restlet/ext/crypto/CookieAuthenticator.java +++ b/org.restlet.ext.crypto/src/main/java/org/restlet/ext/crypto/CookieAuthenticator.java @@ -47,7 +47,8 @@ *
* Note that credentials, both identifier and secret, are stored in a cookie in an encrypted manner. * The default encryption algorithm is AES but can be changed with {@link - * #setEncryptAlgorithm(String)}. It is also strongly recommended to + * #setEncryptAlgorithm(String)}. It is also strongly recommended to serve the application over + * HTTPS, as the cookie is otherwise exposed to eavesdroppers, and to keep the secret key private. * * @author Remi Dewitte * @author Jerome Louvel @@ -172,7 +173,10 @@ protected int authenticated(Request request, Response response) { getLogger().log(Level.SEVERE, "Could not format credentials cookie", e); } - return super.authenticated(request, response); + int result = super.authenticated(request, response); + + // Don't let the next restlet overwrite a login redirection (issue #554) + return response.getStatus().isRedirection() ? STOP : result; } /** @@ -267,8 +271,12 @@ public String getCookieName() { } /** - * Returns the credentials cookie setting. It first try to find an existing cookie. If - * necessary, it creates a new one. + * Returns the credentials cookie setting. It first try to find an existing cookie setting in + * the response. If necessary, it creates a new one.
+ *
+ * Note that the cookie sent by the client is not looked up here: a new setting is created, and + * so the cookie is re-issued, on each authenticated request. This renews its expiration date + * (sliding expiration). * * @param request The current request. * @param response The current response. @@ -280,13 +288,10 @@ protected CookieSetting getCredentialsCookie(Request request, Response response) if (credentialsCookie == null) { credentialsCookie = new CookieSetting(getCookieName(), null); credentialsCookie.setAccessRestricted(true); - // authCookie.setVersion(1); if (request.getRootRef() != null) { String p = request.getRootRef().getPath(); credentialsCookie.setPath(p == null ? "/" : p); - } else { - // authCookie.setPath("/"); } response.getCookieSettings().add(credentialsCookie); diff --git a/org.restlet.ext.crypto/src/test/java/org/restlet/ext/crypto/CookieAuthenticatorTestCase.java b/org.restlet.ext.crypto/src/test/java/org/restlet/ext/crypto/CookieAuthenticatorTestCase.java index dfb97cd502..745a757030 100644 --- a/org.restlet.ext.crypto/src/test/java/org/restlet/ext/crypto/CookieAuthenticatorTestCase.java +++ b/org.restlet.ext.crypto/src/test/java/org/restlet/ext/crypto/CookieAuthenticatorTestCase.java @@ -12,6 +12,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.Test; import org.restlet.Application; import org.restlet.Component; @@ -21,6 +22,7 @@ import org.restlet.data.CookieSetting; import org.restlet.data.Form; import org.restlet.data.MediaType; +import org.restlet.data.Method; import org.restlet.data.Status; import org.restlet.resource.ClientResource; import org.restlet.resource.ResourceException; @@ -100,4 +102,36 @@ void testCookieAuth1() { cs = logoutCr.getCookieSettings().getFirst("Credentials"); assertEquals(0, cs.getMaxAge()); } + + @Test + void login_withTargetUri_redirectsWithoutCallingNextRestlet() { + AtomicInteger nextCalls = new AtomicInteger(); + CookieAuthenticator co = + new CookieAuthenticator(null, false, "realm", "MyExtraSecretKey".getBytes()); + MapVerifier mapVerifier = new MapVerifier(); + mapVerifier.getLocalSecrets().put("scott", "tiger".toCharArray()); + co.setVerifier(mapVerifier); + co.setNext( + new Restlet() { + @Override + public void handle(Request request, Response response) { + nextCalls.incrementAndGet(); + response.setStatus(Status.SUCCESS_OK); + } + }); + + Request request = new Request(Method.POST, "http://toto.com/login?targetUri=/home"); + Form form = new Form(); + form.add("login", "scott"); + form.add("password", "tiger"); + request.getResourceRef().setBaseRef("http://toto.com"); + request.setEntity(form.getWebRepresentation()); + Response response = new Response(request); + co.handle(request, response); + + assertEquals(Status.REDIRECTION_SEE_OTHER, response.getStatus()); + assertNotNull(response.getLocationRef()); + assertNotNull(response.getCookieSettings().getFirst("Credentials")); + assertEquals(0, nextCalls.get()); + } } diff --git a/org.restlet/src/main/java/org/restlet/Response.java b/org.restlet/src/main/java/org/restlet/Response.java index 82cc5a96ab..ed4c669641 100644 --- a/org.restlet/src/main/java/org/restlet/Response.java +++ b/org.restlet/src/main/java/org/restlet/Response.java @@ -394,8 +394,7 @@ public List getChallengeRequests() { * Returns the modifiable series of cookie settings provided by the server. Creates a new * instance if no one has been set.
*
- * Note that when used with HTTP connectors, this property maps to the "Set-Cookie" and - * "Set-Cookie2" headers. + * Note that when used with HTTP connectors, this property maps to the "Set-Cookie" header. * * @return The cookie settings provided by the server. */ @@ -859,8 +858,8 @@ public void setCommitted(boolean committed) { /** * Sets the modifiable series of cookie settings provided by the server. Note that when used - * with HTTP connectors, this property maps to the "Set-Cookie" and "Set-Cookie2" headers. This - * method clears the current series and adds all entries in the parameter series. + * with HTTP connectors, this property maps to the "Set-Cookie" header. This method clears the + * current series and adds all entries in the parameter series. * * @param cookieSettings A series of cookie settings provided by the server. */ diff --git a/org.restlet/src/main/java/org/restlet/data/Cookie.java b/org.restlet/src/main/java/org/restlet/data/Cookie.java index 70de428008..9968611441 100644 --- a/org.restlet/src/main/java/org/restlet/data/Cookie.java +++ b/org.restlet/src/main/java/org/restlet/data/Cookie.java @@ -39,36 +39,20 @@ public class Cookie implements NamedValue { /** The value. */ private volatile String value; - /** The version number. */ - private volatile int version; - /** Default constructor. */ public Cookie() { - this(0, null, null, null, null); - } - - /** - * Constructor. - * - * @param version The version number. - * @param name The name. - * @param value The value. - */ - public Cookie(int version, String name, String value) { - this(version, name, value, null, null); + this(null, null, null, null); } /** * Constructor. * - * @param version The version number. * @param name The name. * @param value The value. * @param path The validity path. * @param domain The domain name. */ - public Cookie(int version, String name, String value, String path, String domain) { - this.version = version; + public Cookie(String name, String value, String path, String domain) { this.name = name; this.value = value; this.path = path; @@ -82,7 +66,7 @@ public Cookie(int version, String name, String value, String path, String domain * @param value The value. */ public Cookie(String name, String value) { - this(0, name, value, null, null); + this(name, value, null, null); } /** {@inheritDoc} */ @@ -97,7 +81,6 @@ public boolean equals(Object obj) { return Objects.equals(getName(), that.getName()) && Objects.equals(getValue(), that.getValue()) - && (this.version == that.version) && Objects.equals(getDomain(), that.getDomain()) && Objects.equals(getPath(), that.getPath()); } @@ -138,19 +121,10 @@ public String getValue() { return value; } - /** - * Returns the cookie specification version. - * - * @return The cookie specification version. - */ - public int getVersion() { - return this.version; - } - /** {@inheritDoc} */ @Override public int hashCode() { - return SystemUtils.hashCode(getName(), getValue(), getVersion(), getPath(), getDomain()); + return SystemUtils.hashCode(getName(), getValue(), getPath(), getDomain()); } /** @@ -189,15 +163,6 @@ public void setValue(String value) { this.value = value; } - /** - * Sets the cookie specification version. - * - * @param version The cookie specification version. - */ - public void setVersion(int version) { - this.version = version; - } - @Override public String toString() { return "Cookie [domain=" @@ -208,8 +173,6 @@ public String toString() { + path + ", value=" + value - + ", version=" - + version + "]"; } } diff --git a/org.restlet/src/main/java/org/restlet/data/CookieSetting.java b/org.restlet/src/main/java/org/restlet/data/CookieSetting.java index dd94a311e4..cd2dea90d1 100644 --- a/org.restlet/src/main/java/org/restlet/data/CookieSetting.java +++ b/org.restlet/src/main/java/org/restlet/data/CookieSetting.java @@ -8,7 +8,6 @@ */ package org.restlet.data; -import java.util.Objects; import org.restlet.Response; import org.restlet.engine.util.SystemUtils; @@ -16,8 +15,7 @@ * Cookie setting provided by a server. This allows a server side application to add, modify, or * remove a cookie on the client.
*
- * Note that when used with HTTP connectors, this class maps to the "Set-Cookie" and "Set-Cookie2" - * headers. + * Note that when used with HTTP connectors, this class maps to the "Set-Cookie" header (RFC 6265). * * @see Response#getCookieSettings() * @author Jerome Louvel @@ -27,103 +25,77 @@ public final class CookieSetting extends Cookie { private static final String DESCRIPTION = "Cookie setting"; /** - * Indicates whether to restrict cookie access to untrusted parties. Currently, this toggles the - * non-standard but widely supported HttpOnly cookie parameter. + * Indicates whether to restrict cookie access to untrusted parties. This toggles the HttpOnly + * cookie attribute. */ private volatile boolean accessRestricted; - /** The user's comment. */ - private volatile String comment; - /** The maximum age in seconds. Use 0 to discard an existing cookie. */ private volatile int maxAge; /** Indicates if the cookie should only be transmitted by secure means. */ private volatile boolean secure; + /** The SameSite attribute, or null if not set. */ + private volatile SameSite sameSite; + /** Default constructor. */ public CookieSetting() { - this(0, null, null); + this(null, null); } /** * Constructor. * - * @param version The cookie's version. - * @param name The cookie's name. - * @param value The cookie's value. - */ - public CookieSetting(int version, String name, String value) { - this(version, name, value, null, null); - } - - /** - * Constructor. - * - * @param version The cookie's version. * @param name The cookie's name. * @param value The cookie's value. * @param path The cookie's path. * @param domain The cookie's domain name. */ - public CookieSetting(int version, String name, String value, String path, String domain) { - this(version, name, value, path, domain, null, -1, false, false); + public CookieSetting(String name, String value, String path, String domain) { + this(name, value, path, domain, -1, false, false); } /** * Constructor. * - * @param version The cookie's version. * @param name The cookie's name. * @param value The cookie's value. * @param path The cookie's path. * @param domain The cookie's domain name. - * @param comment The cookie's comment. * @param maxAge Sets the maximum age in seconds.
* Use 0 to immediately discard an existing cookie.
* Use -1 to discard the cookie at the end of the session (default). * @param secure Indicates if the cookie should only be transmitted by secure means. */ public CookieSetting( - int version, - String name, - String value, - String path, - String domain, - String comment, - int maxAge, - boolean secure) { - this(version, name, value, path, domain, comment, maxAge, secure, false); + String name, String value, String path, String domain, int maxAge, boolean secure) { + this(name, value, path, domain, maxAge, secure, false); } /** * Constructor. * - * @param version The cookie's version. * @param name The cookie's name. * @param value The cookie's value. * @param path The cookie's path. * @param domain The cookie's domain name. - * @param comment The cookie's comment. * @param maxAge Sets the maximum age in seconds.
* Use 0 to immediately discard an existing cookie.
* Use -1 to discard the cookie at the end of the session (default). * @param secure Indicates if the cookie should only be transmitted by secure means. * @param accessRestricted Indicates whether to restrict cookie access to untrusted parties. - * Currently, this toggles the non-standard but widely supported HttpOnly cookie parameter. + * This toggles the HttpOnly cookie attribute. */ public CookieSetting( - int version, String name, String value, String path, String domain, - String comment, int maxAge, boolean secure, boolean accessRestricted) { - super(version, name, value, path, domain); - this.comment = comment; + super(name, value, path, domain); this.maxAge = maxAge; this.secure = secure; this.accessRestricted = accessRestricted; @@ -136,7 +108,7 @@ public CookieSetting( * @param value The cookie's value. */ public CookieSetting(String name, String value) { - this(0, name, value, null, null); + this(name, value, null, null); } /** {@inheritDoc} */ @@ -153,16 +125,7 @@ public boolean equals(Object obj) { && this.maxAge == that.maxAge && this.secure == that.secure && accessRestricted == that.accessRestricted - && Objects.equals(this.comment, that.comment); - } - - /** - * Returns the comment for the user. - * - * @return The comment for the user. - */ - public String getComment() { - return this.comment; + && this.sameSite == that.sameSite; } /** @@ -187,12 +150,13 @@ public int getMaxAge() { /** {@inheritDoc} */ @Override public int hashCode() { - return SystemUtils.hashCode(super.hashCode(), getComment(), getMaxAge(), isSecure()); + return SystemUtils.hashCode( + super.hashCode(), getMaxAge(), isSecure(), isAccessRestricted(), getSameSite()); } /** - * Indicates if cookie access is restricted for untrusted parties. Currently, this toggles the - * non-standard but widely supported HttpOnly cookie parameter. + * Indicates if cookie access is restricted for untrusted parties. This toggles the HttpOnly + * cookie attribute. * * @return accessRestricted True if cookie access should be restricted */ @@ -200,6 +164,15 @@ public boolean isAccessRestricted() { return this.accessRestricted; } + /** + * Returns the SameSite attribute (RFC 6265bis). + * + * @return The SameSite attribute, or null if not set. + */ + public SameSite getSameSite() { + return this.sameSite; + } + /** * Indicates if the cookie should only be transmitted by secure means. * @@ -210,8 +183,8 @@ public boolean isSecure() { } /** - * Indicates whether to restrict cookie access to untrusted parties. Currently, this toggles the - * non-standard but widely supported HttpOnly cookie parameter. + * Indicates whether to restrict cookie access to untrusted parties. This toggles the HttpOnly + * cookie attribute. * * @param accessRestricted True if cookie access should be restricted */ @@ -220,12 +193,13 @@ public void setAccessRestricted(boolean accessRestricted) { } /** - * Sets the comment for the user. + * Sets the SameSite attribute (RFC 6265bis). Note that browsers require the Secure attribute + * when SameSite is {@link SameSite#NONE}. * - * @param comment The comment for the user. + * @param sameSite The SameSite attribute, or null to not send it. */ - public void setComment(String comment) { - this.comment = comment; + public void setSameSite(SameSite sameSite) { + this.sameSite = sameSite; } /** @@ -251,12 +225,12 @@ public void setSecure(boolean secure) { public String toString() { return "CookieSetting [accessRestricted=" + accessRestricted - + ", comment=" - + comment + ", maxAge=" + maxAge + ", secure=" + secure + + ", sameSite=" + + sameSite + ", domain=" + getDomain() + ", name=" @@ -265,8 +239,6 @@ public String toString() { + getPath() + ", value=" + getValue() - + ", version=" - + getVersion() + "]"; } } diff --git a/org.restlet/src/main/java/org/restlet/data/SameSite.java b/org.restlet/src/main/java/org/restlet/data/SameSite.java new file mode 100644 index 0000000000..76a6d0730b --- /dev/null +++ b/org.restlet/src/main/java/org/restlet/data/SameSite.java @@ -0,0 +1,60 @@ +/** + * Copyright 2005-2026 Qlik + *

+ * The content of this file is subject to the terms of the Apache 2.0 open + * source license available at https://www.opensource.org/licenses/apache-2.0 + *

+ * Restlet is a registered trademark of QlikTech International AB. + */ +package org.restlet.data; + +/** + * Value of the SameSite cookie attribute (RFC 6265bis), which controls whether a cookie is sent + * with cross-site requests. + * + * @see CookieSetting#setSameSite(SameSite) + */ +public enum SameSite { + + /** The cookie is only sent with same-site requests. */ + STRICT("Strict"), + + /** The cookie is also sent when navigating to the site from another one (browser default). */ + LAX("Lax"), + + /** The cookie is sent with all requests. Requires the Secure attribute. */ + NONE("None"); + + private final String headerValue; + + SameSite(String headerValue) { + this.headerValue = headerValue; + } + + /** + * Returns the value as written in the Set-Cookie header. + * + * @return The header value. + */ + public String getHeaderValue() { + return headerValue; + } + + /** + * Parses a header value, ignoring case. + * + * @param value The header value. + * @return The matching constant, or null if the value is not recognized. + */ + public static SameSite parse(String value) { + if (value != null) { + for (SameSite sameSite : values()) { + if (sameSite.headerValue.equalsIgnoreCase(value.trim())) { + return sameSite; + } + } + } + + return null; + } +} diff --git a/org.restlet/src/main/java/org/restlet/engine/header/CookieReader.java b/org.restlet/src/main/java/org/restlet/engine/header/CookieReader.java index ac95aa4723..2381b89782 100644 --- a/org.restlet/src/main/java/org/restlet/engine/header/CookieReader.java +++ b/org.restlet/src/main/java/org/restlet/engine/header/CookieReader.java @@ -15,16 +15,13 @@ /** * Cookie header reader. * + *

Reads the RFC 6265 format: cookies are separated by semicolons. The pairs of the obsolete RFC + * 2109 format ($Version, $Path and $Domain) are silently ignored. + * * @author Jerome Louvel */ public class CookieReader extends HeaderReader { - private static final String NAME_DOMAIN = "$Domain"; - - private static final String NAME_PATH = "$Path"; - - private static final String NAME_VERSION = "$Version"; - /** * Parses the given String to a Cookie * @@ -42,9 +39,6 @@ public static Cookie read(String cookie) throws IllegalArgumentException { } } - /** The global cookie specification version. */ - private volatile int globalVersion; - /** * Constructor. * @@ -52,74 +46,86 @@ public static Cookie read(String cookie) throws IllegalArgumentException { */ public CookieReader(String header) { super(header); - this.globalVersion = -1; } + /** Returned by {@link #readName} when the next pair is not a legacy attribute. */ + private static final int NOT_LEGACY = -2; + /** * Reads the next pair as a parameter. * - * @param readAttribute True, if the intention is to only read the cookie attribute. + * @param legacyOnly True, if the pair must be left unread unless it is a legacy attribute whose + * name starts with '$'. * @return The next pair as a parameter. * @throws IOException */ - private Parameter readPair(boolean readAttribute) throws IOException { - Parameter result = null; - - boolean readingName = true; - StringBuilder nameBuffer = new StringBuilder(); - StringBuilder valueBuffer = new StringBuilder(); - int nextChar = 0; - - while ((result == null) && (nextChar != -1)) { - nextChar = read(); - - if (readingName) { - if (nameBuffer.isEmpty() && HeaderUtils.isSpace(nextChar)) { - // Skip spaces - } else if ((nextChar == -1) || (nextChar == ';') || (nextChar == ',')) { - if (!nameBuffer.isEmpty()) { - // End of a pair with no value - result = Parameter.create(nameBuffer, null); - } else if (nextChar == -1) { - // Do nothing and return a null preference - } else { - throw new IOException( - "Empty cookie name detected. Please check your cookies"); - } - } else if (nextChar == '=') { - readingName = false; - } else if (HeaderUtils.isTokenChar(nextChar) || (this.globalVersion < 1)) { - if (readAttribute && nextChar != '$' && (nameBuffer.isEmpty())) { - unread(); - nextChar = -1; - } else { - nameBuffer.append((char) nextChar); - } - } else { - throw new IOException( - "Separator and control characters are not allowed within a token. Please check your cookie header"); - } + private Parameter readPair(boolean legacyOnly) throws IOException { + StringBuilder name = new StringBuilder(); + int end = readName(name, legacyOnly); + + if (end == NOT_LEGACY) { + return null; + } else if (end == '=') { + return Parameter.create(name, readPairValue()); + } else if (!name.isEmpty()) { + // End of a pair with no value + return Parameter.create(name, null); + } else if (end == ';') { + throw new IOException("Empty cookie name detected. Please check your cookies"); + } + + // End of the header + return null; + } + + /** + * Reads the name of a pair, skipping the leading spaces. + * + * @param name The buffer receiving the name. + * @param legacyOnly True, if the pair must be left unread unless its name starts with '$'. + * @return The character ending the name: '=', ';' or -1 at the end of the header, or {@link + * #NOT_LEGACY}. + */ + private int readName(StringBuilder name, boolean legacyOnly) { + int next; + + while (((next = read()) != -1) && (next != ';') && (next != '=')) { + if (name.isEmpty() && HeaderUtils.isSpace(next)) { + // Skip spaces + } else if (legacyOnly && name.isEmpty() && next != '$') { + unread(); + return NOT_LEGACY; } else { - // reading value - if (valueBuffer.isEmpty() && HeaderUtils.isSpace(nextChar)) { - // Skip spaces - } else if ((nextChar == -1) || (nextChar == ';')) { - // End of a pair - result = Parameter.create(nameBuffer, valueBuffer); - } else if ((nextChar == '"') && (valueBuffer.isEmpty())) { - // Step back - unread(); - valueBuffer.append(readQuotedString()); - } else if (HeaderUtils.isTokenChar(nextChar) || (this.globalVersion < 1)) { - valueBuffer.append((char) nextChar); - } else { - throw new IOException( - "Separator and control characters are not allowed within a token. Please check your cookie header"); - } + name.append((char) next); } } - return result; + return next; + } + + /** + * Reads the value of a pair, up to the next semicolon. Quotes are tolerated around the value. + * + * @return The value. + * @throws IOException + */ + private StringBuilder readPairValue() throws IOException { + StringBuilder value = new StringBuilder(); + int next; + + while (((next = read()) != -1) && (next != ';')) { + if (value.isEmpty() && HeaderUtils.isSpace(next)) { + // Skip spaces + } else if ((next == '"') && value.isEmpty()) { + // Step back + unread(); + value.append(readQuotedString()); + } else { + value.append((char) next); + } + } + + return value; } @Override @@ -127,49 +133,20 @@ public Cookie readValue() throws IOException { Cookie result = null; Parameter pair = readPair(false); - if (pair != null && this.globalVersion == -1) { - // Cookies version not yet detected - this.globalVersion = getVersion(pair); - } - + // Silently ignore the legacy $Version, $Path and $Domain pairs while ((pair != null) && (pair.getName().isEmpty() || pair.getName().charAt(0) == '$')) { - // Unexpected special attribute - // Silently ignore it as it may have been introduced by new specifications pair = readPair(false); } if (pair != null) { - // Set the cookie name and value - result = new Cookie(this.globalVersion, pair.getName(), pair.getValue()); - pair = readPair(true); - } + result = new Cookie(pair.getName(), pair.getValue()); - while ((pair != null) && (pair.getName().isEmpty() || pair.getName().charAt(0) == '$')) { - if (pair.getName().equalsIgnoreCase(NAME_PATH)) { - result.setPath(pair.getValue()); - } else if (pair.getName().equalsIgnoreCase(NAME_DOMAIN)) { - result.setDomain(pair.getValue()); - } else { - // Unexpected special attribute - // Silently ignore it as it may have been introduced by new specifications - } - - pair = readPair(true); + // Skip the legacy attributes following the cookie + do { + pair = readPair(true); + } while (pair != null); } return result; } - - private int getVersion(final Parameter pair) throws IOException { - if (NAME_VERSION.equalsIgnoreCase(pair.getName())) { - if (pair.getValue() != null) { - return Integer.parseInt(pair.getValue()); - } else { - throw new IOException( - "Empty cookies version attribute detected. Please check your cookie header"); - } - } else { // Set the default version for old Netscape cookies - return 0; - } - } } diff --git a/org.restlet/src/main/java/org/restlet/engine/header/CookieSettingReader.java b/org.restlet/src/main/java/org/restlet/engine/header/CookieSettingReader.java index ea2efeb471..2ddd97bd19 100644 --- a/org.restlet/src/main/java/org/restlet/engine/header/CookieSettingReader.java +++ b/org.restlet/src/main/java/org/restlet/engine/header/CookieSettingReader.java @@ -18,13 +18,15 @@ import org.restlet.Context; import org.restlet.data.CookieSetting; import org.restlet.data.Parameter; +import org.restlet.data.SameSite; import org.restlet.engine.util.DateUtils; import org.restlet.engine.util.StringUtils; /** * Cookie setting header reader. * - *

The commentURL and port attributes are not supported yet. + *

Reads RFC 6265 cookies. The attributes of the obsolete RFC 2109 and RFC 2965 formats (Version, + * Comment, CommentURL, Discard, Port) are silently ignored. * * @author Jerome Louvel */ @@ -32,10 +34,6 @@ public class CookieSettingReader extends HeaderReader { private static final String NAME_SET_ACCESS_RESTRICTED = "httpOnly"; - private static final String NAME_SET_COMMENT = "comment"; - - private static final String NAME_SET_DISCARD = "discard"; - private static final String NAME_SET_DOMAIN = "domain"; private static final String NAME_SET_EXPIRES = "expires"; @@ -44,9 +42,9 @@ public class CookieSettingReader extends HeaderReader { private static final String NAME_SET_PATH = "path"; - private static final String NAME_SET_SECURE = "secure"; + private static final String NAME_SET_SAME_SITE = "samesite"; - private static final String NAME_SET_VERSION = "version"; + private static final String NAME_SET_SECURE = "secure"; /** * Parses the given String to a CookieSetting @@ -68,9 +66,6 @@ public static CookieSetting read(String cookieSetting) throws IllegalArgumentExc /** The cached pair. Used by the readPair() method. */ private volatile Parameter cachedPair; - /** The global cookie specification version. */ - private final int globalVersion; - /** * Constructor. * @@ -79,7 +74,6 @@ public static CookieSetting read(String cookieSetting) throws IllegalArgumentExc public CookieSettingReader(String header) { super(header); this.cachedPair = null; - this.globalVersion = -1; } /** @@ -120,11 +114,8 @@ private Parameter readPair() throws IOException { } } else if (nextChar == '=') { readingName = false; - } else if (HeaderUtils.isTokenChar(nextChar) || (this.globalVersion < 1)) { - nameBuffer.append((char) nextChar); } else { - throw new IOException( - "Separator and control characters are not allowed within a token. Please check your cookie header"); + nameBuffer.append((char) nextChar); } } else { // reading value @@ -137,11 +128,8 @@ private Parameter readPair() throws IOException { // Step back unread(); valueBuffer.append(readQuotedString()); - } else if (HeaderUtils.isTokenChar(nextChar) || (this.globalVersion < 1)) { - valueBuffer.append((char) nextChar); } else { - throw new IOException( - "Separator and control characters are not allowed within a token. Please check your cookie header"); + valueBuffer.append((char) nextChar); } } } @@ -164,15 +152,15 @@ public CookieSetting readValue() throws IOException { // Set the cookie name and value CookieSetting result = new CookieSetting(pair.getName(), pair.getValue()); + // Max-Age takes precedence over Expires, whatever their order (RFC 6265 section 5.3) + Integer maxAge = null; + Integer expiresMaxAge = null; + while ((pair = readPair()) != null) { if (NAME_SET_PATH.equalsIgnoreCase(pair.getName())) { result.setPath(pair.getValue()); } else if (NAME_SET_DOMAIN.equalsIgnoreCase(pair.getName())) { result.setDomain(pair.getValue()); - } else if (NAME_SET_COMMENT.equalsIgnoreCase(pair.getName())) { - result.setComment(pair.getValue()); - } else if (NAME_SET_DISCARD.equalsIgnoreCase(pair.getName())) { - result.setMaxAge(-1); } else if (NAME_SET_EXPIRES.equalsIgnoreCase(pair.getName())) { Date expires = parseExpiresDate(pair); if (expires == null) { @@ -182,13 +170,13 @@ public CookieSetting readValue() throws IOException { "Ignoring cookie setting expiration date. Unable to parse the date: " + pair.getValue()); } else { - result.setMaxAge(fromExpirationDateToMaxAge(expires)); + expiresMaxAge = fromExpirationDateToMaxAge(expires); } } else if (NAME_SET_MAX_AGE.equalsIgnoreCase(pair.getName())) { try { - result.setMaxAge(Integer.parseInt(pair.getValue())); + maxAge = Integer.parseInt(pair.getValue()); } catch (NumberFormatException numberFormatException) { - result.setMaxAge(Integer.MAX_VALUE); + maxAge = Integer.MAX_VALUE; Context.getCurrentLogger() .warning( "Unable to parse the cookie setting max-age value \"" @@ -204,11 +192,11 @@ public CookieSetting readValue() throws IOException { if (StringUtils.isNullOrEmpty(pair.getValue())) { result.setAccessRestricted(true); } - } else if (NAME_SET_VERSION.equalsIgnoreCase(pair.getName())) { - result.setVersion(Integer.parseInt(pair.getValue())); + } else if (NAME_SET_SAME_SITE.equalsIgnoreCase(pair.getName())) { + result.setSameSite(SameSite.parse(pair.getValue())); } else { - // Silently ignore the parameter as it may have been introduced by new - // specifications + // Silently ignore the parameter, such as the obsolete RFC 2965 ones, or as it may + // have been introduced by new specifications Context.getCurrentLogger() .log( Level.FINE, @@ -217,6 +205,12 @@ public CookieSetting readValue() throws IOException { } } + if (maxAge != null) { + result.setMaxAge(maxAge); + } else if (expiresMaxAge != null) { + result.setMaxAge(expiresMaxAge); + } + return result; } diff --git a/org.restlet/src/main/java/org/restlet/engine/header/CookieSettingWriter.java b/org.restlet/src/main/java/org/restlet/engine/header/CookieSettingWriter.java index 2ca6d6dc3d..ded4a03274 100644 --- a/org.restlet/src/main/java/org/restlet/engine/header/CookieSettingWriter.java +++ b/org.restlet/src/main/java/org/restlet/engine/header/CookieSettingWriter.java @@ -11,6 +11,7 @@ import java.util.Date; import java.util.List; import org.restlet.data.CookieSetting; +import org.restlet.data.SameSite; import org.restlet.engine.util.DateUtils; /** @@ -44,7 +45,6 @@ public static String write(List cookieSettings) { public CookieSettingWriter append(CookieSetting cookieSetting) throws IllegalArgumentException { String name = cookieSetting.getName(); String value = cookieSetting.getValue(); - int version = cookieSetting.getVersion(); if ((name == null) || (name.isEmpty())) { throw new IllegalArgumentException("Can't write cookie. Invalid name detected"); @@ -52,97 +52,45 @@ public CookieSettingWriter append(CookieSetting cookieSetting) throws IllegalArg append(name).append('='); - // Append the value if ((value != null) && (!value.isEmpty())) { - appendValue(value, version); - } - - // Append the version - if (version > 0) { - append("; Version="); - appendValue(Integer.toString(version), version); + append(value); } - // Append the path + // RFC 6265 section 5.2.3 and 5.2.4: attribute values are never quoted String path = cookieSetting.getPath(); if ((path != null) && (!path.isEmpty())) { - append("; Path="); - - if (version == 0) { - append(path); - } else { - appendQuotedString(path); - } + append("; Path=").append(path); } - // Append the expiration date + // Max-Age is preferred by recent user agents, Expires is still needed by old ones int maxAge = cookieSetting.getMaxAge(); if (maxAge >= 0) { - if (version == 0) { - long currentTime = System.currentTimeMillis(); - long maxTime = (maxAge * 1000L); - long expiresTime = currentTime + maxTime; - Date expires = new Date(expiresTime); - - append("; Expires="); - appendValue( - DateUtils.format(expires, DateUtils.FORMAT_RFC_1123.getFirst()), version); - } else { - append("; Max-Age="); - appendValue(Integer.toString(cookieSetting.getMaxAge()), version); - } - } else if ((maxAge == -1) && (version > 0)) { - // Discard the cookie at the end of the user's session (RFC 2965) - append("; Discard"); - } else { - // NetScape cookies automatically expire at the end of the user's session + Date expires = new Date(System.currentTimeMillis() + (maxAge * 1000L)); + append("; Expires=") + .append(DateUtils.format(expires, DateUtils.FORMAT_RFC_1123.getFirst())); + append("; Max-Age=").append(Integer.toString(maxAge)); } - // Append the domain String domain = cookieSetting.getDomain(); if ((domain != null) && (!domain.isEmpty())) { - append("; Domain="); - appendValue(domain.toLowerCase(), version); + append("; Domain=").append(domain.toLowerCase()); } - // Append the secure flag if (cookieSetting.isSecure()) { append("; Secure"); } - // Append the secure flag if (cookieSetting.isAccessRestricted()) { append("; HttpOnly"); } - // Append the comment - if (version > 0) { - String comment = cookieSetting.getComment(); + SameSite sameSite = cookieSetting.getSameSite(); - if ((comment != null) && (!comment.isEmpty())) { - append("; Comment="); - appendValue(comment, version); - } - } - - return this; - } - - /** - * Appends a source string as an HTTP comment. - * - * @param value The source string to format. - * @param version The cookie version. - * @return This writer. - */ - public CookieSettingWriter appendValue(String value, int version) { - if (version == 0) { - append(value); - } else { - appendQuotedString(value); + if (sameSite != null) { + append("; SameSite=").append(sameSite.getHeaderValue()); } return this; diff --git a/org.restlet/src/main/java/org/restlet/engine/header/CookieWriter.java b/org.restlet/src/main/java/org/restlet/engine/header/CookieWriter.java index ec8c968299..fd11af3d79 100644 --- a/org.restlet/src/main/java/org/restlet/engine/header/CookieWriter.java +++ b/org.restlet/src/main/java/org/restlet/engine/header/CookieWriter.java @@ -63,82 +63,37 @@ public static String write(List cookies) { public CookieWriter append(Cookie cookie) throws IllegalArgumentException { String name = cookie.getName(); String value = cookie.getValue(); - int version = cookie.getVersion(); if ((name == null) || (name.isEmpty())) { throw new IllegalArgumentException("Can't write cookie. Invalid name detected"); } - appendValue(name, 0).append('='); + append(name).append('='); - // Append the value if ((value != null) && (!value.isEmpty())) { - appendValue(value, version); - } - - if (version > 0) { - // Append the path - String path = cookie.getPath(); - - if ((path != null) && (!path.isEmpty())) { - append("; $Path="); - appendQuotedString(path); - } - - // Append the domain - String domain = cookie.getDomain(); - - if ((domain != null) && (!domain.isEmpty())) { - append("; $Domain="); - appendQuotedString(domain); - } + append(value); } return this; } /** - * Appends a list of cookies as an HTTP header. + * Appends a list of cookies as an HTTP header, separated by semicolons (RFC 6265 section 5.4). * * @param cookies The list of cookies to format. * @return This writer. */ public CookieWriter append(List cookies) { if ((cookies != null) && !cookies.isEmpty()) { - Cookie cookie; - for (int i = 0; i < cookies.size(); i++) { - cookie = cookies.get(i); - - if (i == 0) { - if (cookie.getVersion() > 0) { - append("$Version=\"").append(cookie.getVersion()).append("\"; "); - } - } else { + if (i > 0) { append("; "); } - append(cookie); + append(cookies.get(i)); } } return this; } - - /** - * Appends a source string as an HTTP comment. - * - * @param value The source string to format. - * @param version The cookie version. - * @return This writer. - */ - public CookieWriter appendValue(String value, int version) { - if (version == 0) { - append(value); - } else { - appendQuotedString(value); - } - - return this; - } } diff --git a/org.restlet/src/main/java/org/restlet/engine/header/HeaderConstants.java b/org.restlet/src/main/java/org/restlet/engine/header/HeaderConstants.java index e02064219f..ad65800621 100644 --- a/org.restlet/src/main/java/org/restlet/engine/header/HeaderConstants.java +++ b/org.restlet/src/main/java/org/restlet/engine/header/HeaderConstants.java @@ -164,8 +164,6 @@ private HeaderConstants() { public static final String HEADER_SET_COOKIE = "Set-Cookie"; - public static final String HEADER_SET_COOKIE2 = "Set-Cookie2"; - public static final String HEADER_SLUG = "Slug"; public static final String HEADER_TRAILER = "Trailer"; diff --git a/org.restlet/src/main/java/org/restlet/engine/header/HeaderUtils.java b/org.restlet/src/main/java/org/restlet/engine/header/HeaderUtils.java index 43af76696f..25c822b3b2 100644 --- a/org.restlet/src/main/java/org/restlet/engine/header/HeaderUtils.java +++ b/org.restlet/src/main/java/org/restlet/engine/header/HeaderUtils.java @@ -67,7 +67,6 @@ import static org.restlet.engine.header.HeaderConstants.HEADER_RETRY_AFTER; import static org.restlet.engine.header.HeaderConstants.HEADER_SERVER; import static org.restlet.engine.header.HeaderConstants.HEADER_SET_COOKIE; -import static org.restlet.engine.header.HeaderConstants.HEADER_SET_COOKIE2; import static org.restlet.engine.header.HeaderConstants.HEADER_TRAILER; import static org.restlet.engine.header.HeaderConstants.HEADER_TRANSFER_ENCODING; import static org.restlet.engine.header.HeaderConstants.HEADER_TRANSFER_EXTENSION; @@ -173,7 +172,6 @@ public class HeaderUtils { HEADER_RETRY_AFTER, HEADER_SERVER, HEADER_SET_COOKIE, - HEADER_SET_COOKIE2, HEADER_USER_AGENT, HEADER_VARY, HEADER_VIA, @@ -274,23 +272,6 @@ public class HeaderUtils { + h.getValue()); } }), - Map.entry( - HEADER_SET_COOKIE2.toLowerCase(), - (h, r, all) -> { - try { - CookieSettingReader cr = - new CookieSettingReader(h.getValue()); - r.getCookieSettings().add(cr.readValue()); - } catch (Exception e) { - Context.getCurrentLogger() - .log( - Level.WARNING, - e, - () -> - "Error during cookie setting parsing. Header: " - + h.getValue()); - } - }), Map.entry( HEADER_WWW_AUTHENTICATE.toLowerCase(), (h, r, all) -> diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyRequest.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyRequest.java index 941942debc..9a6a442f6f 100644 --- a/org.restlet/src/main/java/org/restlet/engine/netty/NettyRequest.java +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyRequest.java @@ -344,7 +344,7 @@ private void addTags(String headerName, List tags) { public Series getCookies() { Series result = super.getCookies(); if (!cookiesAdded) { - String cookies = headers.getValues(HeaderConstants.HEADER_COOKIE); + String cookies = headers.getValues(HeaderConstants.HEADER_COOKIE, "; ", true); if (cookies != null) { new CookieReader(cookies).addValues(result); } diff --git a/org.restlet/src/test/java/org/restlet/data/CookieSettingTestCase.java b/org.restlet/src/test/java/org/restlet/data/CookieSettingTestCase.java index 0e1b326c9a..3bf2f0f6e5 100644 --- a/org.restlet/src/test/java/org/restlet/data/CookieSettingTestCase.java +++ b/org.restlet/src/test/java/org/restlet/data/CookieSettingTestCase.java @@ -22,12 +22,12 @@ class CookieSettingTestCase { @Test void defaultConstructor_hasDefaultValues() { CookieSetting cookie = new CookieSetting(); - assertEquals(0, cookie.getVersion()); assertNull(cookie.getName()); assertNull(cookie.getValue()); assertEquals(-1, cookie.getMaxAge()); assertFalse(cookie.isSecure()); assertFalse(cookie.isAccessRestricted()); + assertNull(cookie.getSameSite()); } @Test @@ -35,31 +35,20 @@ void preferredConstructor_setsNameAndValue() { CookieSetting cookie = new CookieSetting("session", "abc123"); assertEquals("session", cookie.getName()); assertEquals("abc123", cookie.getValue()); - assertEquals(0, cookie.getVersion()); - } - - @Test - void constructor_withVersionNameValue() { - CookieSetting cookie = new CookieSetting(1, "session", "abc123"); - assertEquals(1, cookie.getVersion()); - assertEquals("session", cookie.getName()); - assertEquals("abc123", cookie.getValue()); assertEquals(-1, cookie.getMaxAge()); } @Test void constructor_withPathAndDomain() { - CookieSetting cookie = new CookieSetting(1, "session", "abc123", "/app", "example.com"); + CookieSetting cookie = new CookieSetting("session", "abc123", "/app", "example.com"); assertEquals("/app", cookie.getPath()); assertEquals("example.com", cookie.getDomain()); } @Test - void constructor_withCommentMaxAgeSecure() { + void constructor_withMaxAgeSecure() { CookieSetting cookie = - new CookieSetting( - 1, "session", "abc123", "/app", "example.com", "a comment", 3600, true); - assertEquals("a comment", cookie.getComment()); + new CookieSetting("session", "abc123", "/app", "example.com", 3600, true); assertEquals(3600, cookie.getMaxAge()); assertTrue(cookie.isSecure()); assertFalse(cookie.isAccessRestricted()); @@ -68,31 +57,22 @@ void constructor_withCommentMaxAgeSecure() { @Test void constructor_withAccessRestricted() { CookieSetting cookie = - new CookieSetting( - 1, - "session", - "abc123", - "/app", - "example.com", - "a comment", - 3600, - true, - true); + new CookieSetting("session", "abc123", "/app", "example.com", 3600, true, true); assertTrue(cookie.isAccessRestricted()); } @Test void settersUpdateState() { CookieSetting cookie = new CookieSetting(); - cookie.setComment("comment"); cookie.setMaxAge(60); cookie.setSecure(true); cookie.setAccessRestricted(true); + cookie.setSameSite(SameSite.STRICT); - assertEquals("comment", cookie.getComment()); assertEquals(60, cookie.getMaxAge()); assertTrue(cookie.isSecure()); assertTrue(cookie.isAccessRestricted()); + assertEquals(SameSite.STRICT, cookie.getSameSite()); } @Test @@ -103,41 +83,59 @@ void getDescription_returnsFixedText() { @Test void equals_sameFields_returnsTrue() { CookieSetting c1 = - new CookieSetting( - 1, "session", "abc123", "/app", "example.com", "comment", 3600, true); + new CookieSetting("session", "abc123", "/app", "example.com", 3600, true); CookieSetting c2 = - new CookieSetting( - 1, "session", "abc123", "/app", "example.com", "comment", 3600, true); + new CookieSetting("session", "abc123", "/app", "example.com", 3600, true); assertEquals(c1, c2); assertEquals(c1.hashCode(), c2.hashCode()); } @Test void equals_differentMaxAge_returnsFalse() { - CookieSetting c1 = new CookieSetting(0, "session", "abc123", null, null, null, 10, false); - CookieSetting c2 = new CookieSetting(0, "session", "abc123", null, null, null, 20, false); + CookieSetting c1 = new CookieSetting("session", "abc123", null, null, 10, false); + CookieSetting c2 = new CookieSetting("session", "abc123", null, null, 20, false); + assertNotEquals(c1, c2); + } + + @Test + void equals_differentSameSite_returnsFalse() { + CookieSetting c1 = new CookieSetting("session", "abc123"); + CookieSetting c2 = new CookieSetting("session", "abc123"); + c1.setSameSite(SameSite.LAX); + c2.setSameSite(SameSite.STRICT); assertNotEquals(c1, c2); + assertNotEquals(c1.hashCode(), c2.hashCode()); } @Test - void equals_differentComment_returnsFalse() { - CookieSetting c1 = new CookieSetting(0, "session", "abc123", null, null, "a", 10, false); - CookieSetting c2 = new CookieSetting(0, "session", "abc123", null, null, "b", 10, false); + void hashCode_differentAccessRestricted_differs() { + CookieSetting c1 = new CookieSetting("session", "abc123"); + CookieSetting c2 = new CookieSetting("session", "abc123"); + c2.setAccessRestricted(true); assertNotEquals(c1, c2); + assertNotEquals(c1.hashCode(), c2.hashCode()); } @Test void toString_containsFields() { CookieSetting cookie = - new CookieSetting( - 1, "session", "abc123", "/app", "example.com", "comment", 3600, true, true); + new CookieSetting("session", "abc123", "/app", "example.com", 3600, true, true); + cookie.setSameSite(SameSite.LAX); String result = cookie.toString(); assertTrue(result.contains("session")); assertTrue(result.contains("abc123")); assertTrue(result.contains("example.com")); assertTrue(result.contains("/app")); - assertTrue(result.contains("comment")); assertTrue(result.contains("secure=true")); assertTrue(result.contains("accessRestricted=true")); + assertTrue(result.contains("sameSite=LAX")); + } + + @Test + void sameSite_parse_ignoresCaseAndUnknownValues() { + assertEquals(SameSite.LAX, SameSite.parse("lax")); + assertEquals(SameSite.NONE, SameSite.parse(" None ")); + assertNull(SameSite.parse("bogus")); + assertNull(SameSite.parse(null)); } } diff --git a/org.restlet/src/test/java/org/restlet/data/CookieTestCase.java b/org.restlet/src/test/java/org/restlet/data/CookieTestCase.java index 8ad9128295..4399deb8bf 100644 --- a/org.restlet/src/test/java/org/restlet/data/CookieTestCase.java +++ b/org.restlet/src/test/java/org/restlet/data/CookieTestCase.java @@ -23,26 +23,25 @@ class CookieTestCase { /** Equality tests. */ @Test void testEquals() { - Cookie c1 = new Cookie(1, "name1", "value1", "path1", "domain1"); - Cookie c2 = new Cookie(1, "name1", "value1", "path1", "domain1"); + Cookie c1 = new Cookie("name1", "value1", "path1", "domain1"); + Cookie c2 = new Cookie("name1", "value1", "path1", "domain1"); assertEquals(c1, c2); assertEquals(c1.hashCode(), c2.hashCode()); - assertEquals(c1, c2); } /** Inequality tests. */ @Test void testUnEquals() { - Cookie c1 = new Cookie(1, "name1", "value1", "path1", "domain1"); - Cookie c2 = new Cookie(2, "name2", "value2", "path2", "domain2"); + Cookie c1 = new Cookie("name1", "value1", "path1", "domain1"); + Cookie c2 = new Cookie("name2", "value2", "path2", "domain2"); assertNotEquals(c1, c2); assertNotEquals(c1.hashCode(), c2.hashCode()); assertNotEquals(null, c1); assertNotEquals(null, c2); - c1 = new Cookie(1, "name", "value", "path", "domain"); - c2 = new Cookie(2, "name", "value", "path", "domain"); + c1 = new Cookie("name", "value", "path", "domain"); + c2 = new Cookie("name", "value", "path", "other-domain"); assertNotEquals(c1, c2); assertNotEquals(c1.hashCode(), c2.hashCode()); } diff --git a/org.restlet/src/test/java/org/restlet/engine/connector/CookieHeadersTestCase.java b/org.restlet/src/test/java/org/restlet/engine/connector/CookieHeadersTestCase.java new file mode 100644 index 0000000000..a09f6cb649 --- /dev/null +++ b/org.restlet/src/test/java/org/restlet/engine/connector/CookieHeadersTestCase.java @@ -0,0 +1,158 @@ +/** + * Copyright 2005-2026 Qlik + *

+ * The content of this file is subject to the terms of the Apache 2.0 open + * source license available at https://www.opensource.org/licenses/apache-2.0 + *

+ * Restlet is a registered trademark of QlikTech International AB. + */ +package org.restlet.engine.connector; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.ByteArrayOutputStream; +import java.io.InputStream; +import java.io.OutputStream; +import java.net.Socket; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Stream; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.restlet.Context; +import org.restlet.Request; +import org.restlet.Response; +import org.restlet.Restlet; +import org.restlet.Server; +import org.restlet.data.Cookie; +import org.restlet.data.CookieSetting; +import org.restlet.data.MediaType; +import org.restlet.data.Protocol; +import org.restlet.data.SameSite; +import org.restlet.engine.Engine; + +/** Tests the cookie headers on the wire, as written and read by the Netty server connector. */ +class CookieHeadersTestCase { + + private Server server; + + @BeforeEach + void setUp() { + Engine.clearThreadLocalVariables(); + Engine.register(false); + Engine.getInstance().getRegisteredServers().add(new HttpServerHelper(null)); + Engine.getInstance().getRegisteredClients().add(new HttpClientHelper(null)); + } + + @AfterEach + void tearDown() throws Exception { + if (server != null) { + server.stop(); + } + Engine.clearThreadLocalVariables(); + } + + /** Sends a raw request and returns the raw response head and body. */ + private String exchange(String requestHeaders) throws Exception { + try (Socket socket = new Socket("localhost", server.getActualPort())) { + OutputStream out = socket.getOutputStream(); + out.write( + ("GET / HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n" + + requestHeaders + + "\r\n") + .getBytes(StandardCharsets.US_ASCII)); + out.flush(); + + InputStream in = socket.getInputStream(); + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + byte[] chunk = new byte[4096]; + int n; + while ((n = in.read(chunk)) != -1) { + buffer.write(chunk, 0, n); + } + return buffer.toString(StandardCharsets.ISO_8859_1); + } + } + + private static List headerValues(String response, String name) { + List result = new ArrayList<>(); + for (String line : response.split("\r\n")) { + if (line.regionMatches(true, 0, name + ":", 0, name.length() + 1)) { + result.add(line.substring(name.length() + 1).trim()); + } + } + return result; + } + + private void startEchoingServer() throws Exception { + server = + new Server( + new Context(), + Protocol.HTTP, + 0, + new Restlet() { + @Override + public void handle(Request request, Response response) { + StringBuilder sb = new StringBuilder(); + for (Cookie cookie : request.getCookies()) { + sb.append(cookie.getName()) + .append('=') + .append(cookie.getValue()) + .append('|'); + } + response.setEntity(sb.toString(), MediaType.TEXT_PLAIN); + + CookieSetting session = new CookieSetting("session", "abc"); + session.setPath("/app"); + session.setMaxAge(60); + session.setSecure(true); + session.setAccessRestricted(true); + session.setSameSite(SameSite.LAX); + response.getCookieSettings().add(session); + response.getCookieSettings().add(new CookieSetting("plain", "1")); + } + }); + server.start(); + } + + @Test + void setCookie_isWrittenAsRfc6265() throws Exception { + startEchoingServer(); + String response = exchange(""); + + List setCookies = headerValues(response, "Set-Cookie"); + assertEquals(2, setCookies.size(), response); + assertTrue(setCookies.get(0).startsWith("session=abc; Path=/app; Expires="), response); + assertTrue( + setCookies.get(0).endsWith("; Max-Age=60; Secure; HttpOnly; SameSite=Lax"), + response); + assertEquals("plain=1", setCookies.get(1)); + assertFalse(response.contains("Set-Cookie2"), response); + assertFalse(setCookies.get(0).contains("\""), response); + assertFalse(setCookies.get(0).contains("Version"), response); + } + + static Stream requestCookieHeaders() { + return Stream.of( + // One header + Arguments.of("Cookie: a=1; b=2\r\n"), + // HTTP/2 clients split the cookies, HTTP/1.1 allows it as well + Arguments.of("Cookie: a=1\r\nCookie: b=2\r\n"), + // Obsolete RFC 2109 format, read leniently + Arguments.of("Cookie: $Version=\"1\"; a=\"1\"; $Path=\"/x\"; b=\"2\"\r\n")); + } + + @ParameterizedTest + @MethodSource("requestCookieHeaders") + void cookie_isRead(String requestHeaders) throws Exception { + startEchoingServer(); + assertTrue(exchange(requestHeaders).endsWith("a=1|b=2|")); + } +} diff --git a/org.restlet/src/test/java/org/restlet/engine/header/CookieReaderTestCase.java b/org.restlet/src/test/java/org/restlet/engine/header/CookieReaderTestCase.java index 3bfbb107a8..6100de89d0 100644 --- a/org.restlet/src/test/java/org/restlet/engine/header/CookieReaderTestCase.java +++ b/org.restlet/src/test/java/org/restlet/engine/header/CookieReaderTestCase.java @@ -9,6 +9,7 @@ package org.restlet.engine.header; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; @@ -120,6 +121,46 @@ private void testCookieSetting(String headerValue, boolean compare) throws IOExc } } + /** + * Test the lenient reading of a cookie header in the obsolete RFC 2109 format. + * + * @param headerValue The legacy cookie header value. + * @param expected The expected RFC 6265 header once rewritten. + */ + private void testLegacyCookie(String headerValue, String expected) throws IOException { + CookieReader cr = new CookieReader(headerValue); + List cookies = new ArrayList<>(); + Cookie cookie = cr.readValue(); + + while (cookie != null) { + assertNull(cookie.getPath(), "The legacy $Path must be ignored"); + assertNull(cookie.getDomain(), "The legacy $Domain must be ignored"); + cookies.add(cookie); + cookie = cr.readValue(); + } + + assertEquals(expected, CookieWriter.write(cookies)); + } + + /** + * Test a set cookie header in the obsolete RFC 2109 format. + * + * @param headerValue The legacy set cookie header value. + * @param expected The expected RFC 6265 header once rewritten. + */ + private void testCookieSetting(String headerValue, String expected) throws IOException { + CookieSetting cookie = new CookieSettingReader(headerValue).readValue(); + assertEquals(expected, CookieSettingWriter.write(cookie)); + } + + @Test + void read_commaIsNotASeparator() throws IOException { + // RFC 6265 section 4.2.1: cookies are separated by semicolons only + Cookie cookie = new CookieReader("a=1, b=2; c=3").readValue(); + assertEquals("a", cookie.getName()); + assertEquals("1, b=2", cookie.getValue()); + } + /** Tests the parsing of cookies. */ @Test void testParsing() throws IOException { @@ -136,23 +177,26 @@ void testParsing() throws IOException { testCookieDate("Tuesday, 09-Nov-99 23:12:40 GMT"); - // RFC 2109 - testCookie("$Version=\"1\"; Customer=\"WILE_E_COYOTE\"; $Path=\"/acme\""); - testCookie( - "$Version=\"1\"; Customer=\"WILE_E_COYOTE\"; $Path=\"/acme\"; Part_Number=\"Rocket_Launcher_0001\"; $Path=\"/acme\""); - testCookie( - "$Version=\"1\"; Customer=\"WILE_E_COYOTE\"; $Path=\"/acme\"; Part_Number=\"Rocket_Launcher_0001\"; $Path=\"/acme\"; Shipping=\"FedEx\"; $Path=\"/acme\""); - testCookie( - "$Version=\"1\"; Part_Number=\"Riding_Rocket_0023\"; $Path=\"/acme/ammo\"; Part_Number=\"Rocket_Launcher_0001\"; $Path=\"/acme\""); + // Obsolete RFC 2109 samples: read leniently, written back as RFC 6265 + testLegacyCookie( + "$Version=\"1\"; Customer=\"WILE_E_COYOTE\"; $Path=\"/acme\"", + "Customer=WILE_E_COYOTE"); + testLegacyCookie( + "$Version=\"1\"; Customer=\"WILE_E_COYOTE\"; $Path=\"/acme\"; Part_Number=\"Rocket_Launcher_0001\"; $Path=\"/acme\"", + "Customer=WILE_E_COYOTE; Part_Number=Rocket_Launcher_0001"); + testLegacyCookie( + "$Version=\"1\"; Customer=\"WILE_E_COYOTE\"; $Path=\"/acme\"; Part_Number=\"Rocket_Launcher_0001\"; $Path=\"/acme\"; Shipping=\"FedEx\"; $Domain=\"example.com\"", + "Customer=WILE_E_COYOTE; Part_Number=Rocket_Launcher_0001; Shipping=FedEx"); + testLegacyCookie( + "$Version=\"1\"; Part_Number=\"Riding_Rocket_0023\"; $Path=\"/acme/ammo\"; Part_Number=\"Rocket_Launcher_0001\"; $Path=\"/acme\"", + "Part_Number=Riding_Rocket_0023; Part_Number=Rocket_Launcher_0001"); - testCookieSetting("Customer=\"WILE_E_COYOTE\"; Version=\"1\"; Path=\"/acme\"", true); - testCookieSetting( - "Part_Number=\"Rocket_Launcher_0001\"; Version=\"1\"; Path=\"/acme\"", true); - testCookieSetting("Shipping=\"FedEx\"; Version=\"1\"; Path=\"/acme\"", true); testCookieSetting( - "Part_Number=\"Rocket_Launcher_0001\"; Version=\"1\"; Path=\"/acme\"", true); + "Customer=\"WILE_E_COYOTE\"; Version=\"1\"; Path=\"/acme\"; Comment=\"x\"; Discard", + "Customer=WILE_E_COYOTE; Path=/acme"); testCookieSetting( - "Part_Number=\"Riding_Rocket_0023\"; Version=\"1\"; Path=\"/acme/ammo\"", true); + "Part_Number=\"Riding_Rocket_0023\"; Version=\"1\"; Path=\"/acme/ammo\"", + "Part_Number=Riding_Rocket_0023; Path=/acme/ammo"); // Bug #49 testCookieSetting( diff --git a/org.restlet/src/test/java/org/restlet/engine/header/CookieSettingReaderTestCase.java b/org.restlet/src/test/java/org/restlet/engine/header/CookieSettingReaderTestCase.java index 9d87a69872..15116aab55 100644 --- a/org.restlet/src/test/java/org/restlet/engine/header/CookieSettingReaderTestCase.java +++ b/org.restlet/src/test/java/org/restlet/engine/header/CookieSettingReaderTestCase.java @@ -14,6 +14,7 @@ import org.junit.jupiter.api.Test; import org.restlet.data.CookieSetting; +import org.restlet.data.SameSite; class CookieSettingReaderTestCase { @@ -48,14 +49,13 @@ void read_domain_setsDomain() { } @Test - void read_comment_setsComment() { - CookieSetting cookieSetting = CookieSettingReader.read("name=value; comment=hello"); - assertEquals("hello", cookieSetting.getComment()); - } - - @Test - void read_discard_setsMaxAgeToNegativeOne() { - CookieSetting cookieSetting = CookieSettingReader.read("name=value; discard"); + void read_legacyAttributes_areIgnored() { + CookieSetting cookieSetting = + CookieSettingReader.read( + "name=\"value\"; Version=\"1\"; Comment=hello; CommentURL=http://x; Discard; Port=80; Path=\"/app\""); + assertEquals("name", cookieSetting.getName()); + assertEquals("value", cookieSetting.getValue()); + assertEquals("/app", cookieSetting.getPath()); assertEquals(-1, cookieSetting.getMaxAge()); } @@ -84,9 +84,38 @@ void read_httpOnlyFlag_setsAccessRestrictedTrue() { } @Test - void read_version_setsVersion() { - CookieSetting cookieSetting = CookieSettingReader.read("name=value; version=1"); - assertEquals(1, cookieSetting.getVersion()); + void read_sameSite_setsSameSite() { + assertEquals( + SameSite.STRICT, + CookieSettingReader.read("name=value; SameSite=Strict").getSameSite()); + assertEquals( + SameSite.NONE, CookieSettingReader.read("name=value; samesite=none").getSameSite()); + assertNull(CookieSettingReader.read("name=value; SameSite=bogus").getSameSite()); + assertNull(CookieSettingReader.read("name=value").getSameSite()); + } + + @Test + void read_maxAgeAndExpires_maxAgeWinsWhateverTheOrder() { + String expires = + org.restlet.engine.util.DateUtils.format( + new java.util.Date(System.currentTimeMillis() + 7_200_000L), + org.restlet.engine.util.DateUtils.FORMAT_RFC_1123.getFirst()); + assertEquals( + 60, + CookieSettingReader.read("name=value; Max-Age=60; Expires=" + expires).getMaxAge()); + assertEquals( + 60, + CookieSettingReader.read("name=value; Expires=" + expires + "; Max-Age=60") + .getMaxAge()); + } + + @Test + void roundTrip_writtenCookieSetting_isReadBack() { + CookieSetting written = + new CookieSetting("id", "123", "/app", "example.com", 600, true, true); + written.setSameSite(SameSite.LAX); + CookieSetting read = CookieSettingReader.read(CookieSettingWriter.write(written)); + assertEquals(written, read); } @Test diff --git a/org.restlet/src/test/java/org/restlet/engine/header/CookieSettingWriterTestCase.java b/org.restlet/src/test/java/org/restlet/engine/header/CookieSettingWriterTestCase.java index fe4b533388..137fc67125 100644 --- a/org.restlet/src/test/java/org/restlet/engine/header/CookieSettingWriterTestCase.java +++ b/org.restlet/src/test/java/org/restlet/engine/header/CookieSettingWriterTestCase.java @@ -17,6 +17,7 @@ import java.util.List; import org.junit.jupiter.api.Test; import org.restlet.data.CookieSetting; +import org.restlet.data.SameSite; class CookieSettingWriterTestCase { @@ -27,55 +28,53 @@ void write_simpleCookie_writesNameEqualsValue() { @Test void write_missingName_throwsIllegalArgumentException() { - CookieSetting cookieSetting = new CookieSetting(0, "", "value"); + CookieSetting cookieSetting = new CookieSetting("", "value"); assertThrows( IllegalArgumentException.class, () -> CookieSettingWriter.write(cookieSetting)); } @Test - void write_versionOne_appendsQuotedVersion() { - CookieSetting cookieSetting = new CookieSetting(1, "name", "value"); - assertTrue( - CookieSettingWriter.write(cookieSetting) - .startsWith("name=\"value\"; Version=\"1\"")); + void write_neverWritesLegacyAttributes() { + CookieSetting cookieSetting = new CookieSetting("name", "value", "/path", "example.com"); + String result = CookieSettingWriter.write(cookieSetting); + assertFalse(result.contains("Version")); + assertFalse(result.contains("Comment")); + assertFalse(result.contains("Discard")); + assertFalse(result.contains("\"")); } @Test - void write_pathVersionZero_appendsUnquotedPath() { - CookieSetting cookieSetting = new CookieSetting(0, "name", "value", "/path", null); + void write_path_isNotQuoted() { + CookieSetting cookieSetting = new CookieSetting("name", "value", "/path", null); assertEquals("name=value; Path=/path", CookieSettingWriter.write(cookieSetting)); } @Test - void write_pathVersionOne_appendsQuotedPath() { - CookieSetting cookieSetting = new CookieSetting(1, "name", "value", "/path", null); - assertTrue(CookieSettingWriter.write(cookieSetting).contains("; Path=\"/path\"")); - } - - @Test - void write_maxAgeVersionZero_appendsExpiresDate() { + void write_maxAge_appendsExpiresAndUnquotedMaxAge() { CookieSetting cookieSetting = new CookieSetting("name", "value"); cookieSetting.setMaxAge(3600); - assertTrue(CookieSettingWriter.write(cookieSetting).contains("; Expires=")); + String result = CookieSettingWriter.write(cookieSetting); + assertTrue(result.contains("; Expires=")); + assertTrue(result.endsWith("; Max-Age=3600")); } @Test - void write_maxAgeVersionOne_appendsMaxAge() { - CookieSetting cookieSetting = new CookieSetting(1, "name", "value"); - cookieSetting.setMaxAge(3600); - assertTrue(CookieSettingWriter.write(cookieSetting).contains("; Max-Age=\"3600\"")); + void write_maxAgeZero_appendsMaxAgeZero() { + CookieSetting cookieSetting = new CookieSetting("name", "value"); + cookieSetting.setMaxAge(0); + assertTrue(CookieSettingWriter.write(cookieSetting).contains("; Max-Age=0")); } @Test - void write_discardVersionOne_appendsDiscard() { - CookieSetting cookieSetting = new CookieSetting(1, "name", "value"); + void write_sessionCookie_appendsNoExpirationAttribute() { + CookieSetting cookieSetting = new CookieSetting("name", "value"); cookieSetting.setMaxAge(-1); - assertTrue(CookieSettingWriter.write(cookieSetting).contains("; Discard")); + assertEquals("name=value", CookieSettingWriter.write(cookieSetting)); } @Test void write_domain_isLowercased() { - CookieSetting cookieSetting = new CookieSetting(0, "name", "value", null, "EXAMPLE.COM"); + CookieSetting cookieSetting = new CookieSetting("name", "value", null, "EXAMPLE.COM"); assertTrue(CookieSettingWriter.write(cookieSetting).contains("; Domain=example.com")); } @@ -94,17 +93,12 @@ void write_accessRestrictedFlag_appendsHttpOnly() { } @Test - void write_commentVersionOne_appendsQuotedComment() { - CookieSetting cookieSetting = new CookieSetting(1, "name", "value"); - cookieSetting.setComment("a comment"); - assertTrue(CookieSettingWriter.write(cookieSetting).contains("; Comment=\"a comment\"")); - } - - @Test - void write_commentVersionZero_isIgnored() { + void write_sameSite_appendsSameSite() { CookieSetting cookieSetting = new CookieSetting("name", "value"); - cookieSetting.setComment("a comment"); - assertFalse(CookieSettingWriter.write(cookieSetting).contains("Comment")); + cookieSetting.setSameSite(SameSite.LAX); + assertEquals("name=value; SameSite=Lax", CookieSettingWriter.write(cookieSetting)); + cookieSetting.setSameSite(SameSite.NONE); + assertEquals("name=value; SameSite=None", CookieSettingWriter.write(cookieSetting)); } @Test diff --git a/org.restlet/src/test/java/org/restlet/engine/header/CookieWriterTestCase.java b/org.restlet/src/test/java/org/restlet/engine/header/CookieWriterTestCase.java index eeac3bfa06..3be8f2158d 100644 --- a/org.restlet/src/test/java/org/restlet/engine/header/CookieWriterTestCase.java +++ b/org.restlet/src/test/java/org/restlet/engine/header/CookieWriterTestCase.java @@ -27,27 +27,20 @@ void write_simpleCookie_writesNameEqualsValue() { } @Test - void write_versionedCookie_quotesValueAndAppendsPathAndDomain() { - Cookie cookie = new Cookie(1, "name", "value", "/path", "example.com"); - String result = CookieWriter.write(cookie); - assertEquals("name=\"value\"; $Path=\"/path\"; $Domain=\"example.com\"", result); + void write_cookieWithPathAndDomain_writesNameAndValueOnly() { + // The obsolete $Path and $Domain pairs of RFC 2109 are not written anymore + Cookie cookie = new Cookie("name", "value", "/path", "example.com"); + assertEquals("name=value", CookieWriter.write(cookie)); } @Test void write_missingName_throwsIllegalArgumentException() { - Cookie cookie = new Cookie(0, "", "value"); + Cookie cookie = new Cookie("", "value"); assertThrows(IllegalArgumentException.class, () -> CookieWriter.write(cookie)); } @Test - void write_listOfCookies_prefixesVersionAndJoinsWithSemicolon() { - List cookies = Arrays.asList(new Cookie(1, "a", "1"), new Cookie(1, "b", "2")); - String result = CookieWriter.write(cookies); - assertEquals("$Version=\"1\"; a=\"1\"; b=\"2\"", result); - } - - @Test - void write_listWithVersionZero_omitsVersionPrefix() { + void write_listOfCookies_joinsWithSemicolon() { List cookies = Arrays.asList(new Cookie("a", "1"), new Cookie("b", "2")); assertEquals("a=1; b=2", CookieWriter.write(cookies)); } @@ -57,16 +50,6 @@ void write_emptyList_returnsEmptyString() { assertEquals("", CookieWriter.write(List.of())); } - @Test - void appendValue_versionZero_appendsUnquoted() { - assertEquals("value", new CookieWriter().appendValue("value", 0).toString()); - } - - @Test - void appendValue_nonZeroVersion_appendsQuoted() { - assertEquals("\"value\"", new CookieWriter().appendValue("value", 1).toString()); - } - @Test void getCookies_matchingNameInDestinationMap_isUpdated() { Cookie cookie = new Cookie("name", "value"); diff --git a/org.restlet/src/test/java/org/restlet/resource/ServerResourceTestCase.java b/org.restlet/src/test/java/org/restlet/resource/ServerResourceTestCase.java index f6058b2831..0a912bd713 100644 --- a/org.restlet/src/test/java/org/restlet/resource/ServerResourceTestCase.java +++ b/org.restlet/src/test/java/org/restlet/resource/ServerResourceTestCase.java @@ -434,7 +434,7 @@ void setCookieSettings_updatesResponseCookieSettings() { EchoServerResource resource = createInitializedResource(Method.GET); org.restlet.util.Series cookieSettings = new org.restlet.util.Series<>(org.restlet.data.CookieSetting.class); - cookieSettings.add(new org.restlet.data.CookieSetting(0, "name", "value")); + cookieSettings.add(new org.restlet.data.CookieSetting("name", "value")); resource.setCookieSettings(cookieSettings);