From b72f2395809c476cd2e0f5cf20cd0ddb57ab23bc Mon Sep 17 00:00:00 2001 From: David Latorre Date: Tue, 4 Feb 2020 10:26:53 +0000 Subject: [PATCH 1/8] Uncomment TLS Settings modifications test. --- .../com/hotels/styx/admin/OriginsFileCompatibilitySpec.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/OriginsFileCompatibilitySpec.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/OriginsFileCompatibilitySpec.kt index 321fd0d8c5..9273977db8 100644 --- a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/OriginsFileCompatibilitySpec.kt +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/OriginsFileCompatibilitySpec.kt @@ -286,7 +286,7 @@ class OriginsFileCompatibilitySpec : FunSpec() { } } - test("!TLS Settings modifications") { + test("TLS Settings modifications") { writeOrigins(""" - id: appTls path: "/" From e49c23253cb66d6c19431280bf086b15e26f15ca Mon Sep 17 00:00:00 2001 From: David Latorre Date: Thu, 6 Feb 2020 16:39:54 +0000 Subject: [PATCH 2/8] Add support for the SameSite cookie attribute. --- codequality/checkstyle_suppressions.xml | 2 + .../hotels/styx/api/ClientCookieDecoder.java | 356 ++++++++++++++++++ .../hotels/styx/api/CookieHeaderNames.java | 61 +++ .../java/com/hotels/styx/api/HttpVersion.java | 6 +- .../java/com/hotels/styx/api/NettyCookie.java | 77 ++++ .../com/hotels/styx/api/ResponseCookie.java | 43 ++- .../hotels/styx/api/ServerCookieEncoder.java | 39 +- .../styx/api/ClientCookieDecoderTest.java | 50 +++ .../hotels/styx/api/ResponseCookieTest.java | 15 +- .../styx/api/ServerCookieEncoderTest.java | 28 +- pom.xml | 1 + 11 files changed, 639 insertions(+), 39 deletions(-) create mode 100644 components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java create mode 100644 components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java create mode 100644 components/api/src/main/java/com/hotels/styx/api/NettyCookie.java create mode 100644 components/api/src/test/java/com/hotels/styx/api/ClientCookieDecoderTest.java diff --git a/codequality/checkstyle_suppressions.xml b/codequality/checkstyle_suppressions.xml index 952267f908..09fe1d7c15 100644 --- a/codequality/checkstyle_suppressions.xml +++ b/codequality/checkstyle_suppressions.xml @@ -20,6 +20,8 @@ + + diff --git a/components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java b/components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java new file mode 100644 index 0000000000..b46bdf1c27 --- /dev/null +++ b/components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java @@ -0,0 +1,356 @@ +/* + Copyright (C) 2013-2020 Expedia Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package com.hotels.styx.api; + +import com.hotels.styx.api.CookieHeaderNames.SameSite; +import io.netty.handler.codec.DateFormatter; +import io.netty.handler.codec.http.cookie.ClientCookieEncoder; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.nio.CharBuffer; +import java.util.Date; + +import static com.hotels.styx.api.CookieUtil.firstInvalidCookieNameOctet; +import static com.hotels.styx.api.CookieUtil.firstInvalidCookieValueOctet; +import static com.hotels.styx.api.CookieUtil.unwrapValue; +import static io.netty.util.internal.ObjectUtil.checkNotNull; + +/** + * A RFC6265 compliant cookie decoder to be used client side. + *

+ * It will store the way the raw value was wrapped in {@link NettyCookie#setWrap(boolean)} so it can be + * eventually sent back to the Origin server as is. + * + * @see ClientCookieEncoder + */ +public final class ClientCookieDecoder { + + private static final Logger logger = LoggerFactory.getLogger(ClientCookieDecoder.class); + /** + * Strict encoder that validates that name and value chars are in the valid scope + * defined in RFC6265 + */ + public static final ClientCookieDecoder STRICT = new ClientCookieDecoder(true); + + /** + * Lax instance that doesn't validate name and value + */ + public static final ClientCookieDecoder LAX = new ClientCookieDecoder(false); + private final boolean strict; + + + private ClientCookieDecoder(boolean strict) { + this.strict = strict; + } + + /** + * Decodes the specified Set-Cookie HTTP header value into a {@link NettyCookie}. + * + * @return the decoded {@link NettyCookie} + */ + public NettyCookie decode(String header) { + final int headerLen = checkNotNull(header, "header").length(); + + if (headerLen == 0) { + return null; + } + + CookieBuilder cookieBuilder = null; + + loop: + for (int i = 0; ; ) { + + // Skip spaces and separators. + for (; ; ) { + if (i == headerLen) { + break loop; + } + char c = header.charAt(i); + if (c == ',') { + // Having multiple cookies in a single Set-Cookie header is + // deprecated, modern browsers only parse the first one + break loop; + + } else if (c == '\t' || c == '\n' || c == 0x0b || c == '\f' + || c == '\r' || c == ' ' || c == ';') { + i++; + continue; + } + break; + } + + int nameBegin = i; + int nameEnd; + int valueBegin; + int valueEnd; + + for (; ; ) { + char curChar = header.charAt(i); + if (curChar == ';') { + // NAME; (no value till ';') + nameEnd = i; + valueBegin = valueEnd = -1; + break; + + } else if (curChar == '=') { + // NAME=VALUE + nameEnd = i; + i++; + if (i == headerLen) { + // NAME= (empty value, i.e. nothing after '=') + valueBegin = valueEnd = 0; + break; + } + + valueBegin = i; + // NAME=VALUE; + int semiPos = header.indexOf(';', i); + valueEnd = i = semiPos > 0 ? semiPos : headerLen; + break; + } else { + i++; + } + + if (i == headerLen) { + // NAME (no value till the end of string) + nameEnd = headerLen; + valueBegin = valueEnd = -1; + break; + } + } + + if (valueEnd > 0 && header.charAt(valueEnd - 1) == ',') { + // old multiple cookies separator, skipping it + valueEnd--; + } + + if (cookieBuilder == null) { + // cookie name-value pair + NettyCookie cookie = null; + if (nameBegin == -1 || nameBegin == nameEnd) { + logger.debug("Skipping cookie with null name"); + } else if (valueBegin == -1) { + logger.debug("Skipping cookie with null value"); + } else { + CharSequence wrappedValue = CharBuffer.wrap(header, valueBegin, valueEnd); + CharSequence unwrappedValue = unwrapValue(wrappedValue); + if (unwrappedValue == null) { + logger.debug("Skipping cookie because starting quotes are not properly balanced in '{}'", + wrappedValue); + } else { + final String name = header.substring(nameBegin, nameEnd); + int invalidOctetPos; + if (strict && (invalidOctetPos = firstInvalidCookieNameOctet(name)) >= 0) { + if (logger.isDebugEnabled()) { + logger.debug("Skipping cookie because name '{}' contains invalid char '{}'", + name, name.charAt(invalidOctetPos)); + } + } else { + final boolean wrap = unwrappedValue.length() != valueEnd - valueBegin; + if (strict && (invalidOctetPos = firstInvalidCookieValueOctet(unwrappedValue)) >= 0) { + if (logger.isDebugEnabled()) { + logger.debug("Skipping cookie because value '{}' contains invalid char '{}'", + unwrappedValue, unwrappedValue.charAt(invalidOctetPos)); + } + } else { + NettyCookie cookie1 = new NettyCookie(name, unwrappedValue.toString()); + cookie1.setWrap(wrap); + cookie = cookie1; + } + } + } + } + + if (cookie == null) { + return null; + } + + cookieBuilder = new CookieBuilder(cookie, header); + } else { + // cookie attribute + cookieBuilder.appendAttribute(nameBegin, nameEnd, valueBegin, valueEnd); + } + } + return cookieBuilder != null ? cookieBuilder.cookie() : null; + } + + protected NettyCookie initCookie(String header, int nameBegin, int nameEnd, int valueBegin, int valueEnd) { + if (nameBegin == -1 || nameBegin == nameEnd) { + logger.debug("Skipping cookie with null name"); + return null; + } + + if (valueBegin == -1) { + logger.debug("Skipping cookie with null value"); + return null; + } + + CharSequence wrappedValue = CharBuffer.wrap(header, valueBegin, valueEnd); + CharSequence unwrappedValue = unwrapValue(wrappedValue); + if (unwrappedValue == null) { + logger.debug("Skipping cookie because starting quotes are not properly balanced in '{}'", + wrappedValue); + return null; + } + + final String name = header.substring(nameBegin, nameEnd); + + int invalidOctetPos; + if (strict && (invalidOctetPos = firstInvalidCookieNameOctet(name)) >= 0) { + if (logger.isDebugEnabled()) { + logger.debug("Skipping cookie because name '{}' contains invalid char '{}'", + name, name.charAt(invalidOctetPos)); + } + return null; + } + + final boolean wrap = unwrappedValue.length() != valueEnd - valueBegin; + + if (strict && (invalidOctetPos = firstInvalidCookieValueOctet(unwrappedValue)) >= 0) { + if (logger.isDebugEnabled()) { + logger.debug("Skipping cookie because value '{}' contains invalid char '{}'", + unwrappedValue, unwrappedValue.charAt(invalidOctetPos)); + } + return null; + } + + NettyCookie cookie = new NettyCookie(name, unwrappedValue.toString()); + cookie.setWrap(wrap); + return cookie; + } + + + private static class CookieBuilder { + + private final String header; + private final NettyCookie cookie; + private String domain; + private String path; + private long maxAge = Long.MIN_VALUE; + private int expiresStart; + private int expiresEnd; + private boolean secure; + private boolean httpOnly; + private SameSite sameSite; + + CookieBuilder(NettyCookie cookie, String header) { + this.cookie = cookie; + this.header = header; + } + + private long mergeMaxAgeAndExpires() { + // max age has precedence over expires + if (maxAge != Long.MIN_VALUE) { + return maxAge; + } else if (isValueDefined(expiresStart, expiresEnd)) { + Date expiresDate = DateFormatter.parseHttpDate(header, expiresStart, expiresEnd); + if (expiresDate != null) { + long maxAgeMillis = expiresDate.getTime() - System.currentTimeMillis(); + return maxAgeMillis / 1000 + (maxAgeMillis % 1000 != 0 ? 1 : 0); + } + } + return Long.MIN_VALUE; + } + + NettyCookie cookie() { + cookie.setDomain(domain); + cookie.setPath(path); + cookie.setMaxAge(mergeMaxAgeAndExpires()); + cookie.setSecure(secure); + cookie.setHttpOnly(httpOnly); + cookie.setSameSite(sameSite); + return cookie; + } + + /** + * Parse and store a key-value pair. First one is considered to be the + * cookie name/value. Unknown attribute names are silently discarded. + * + * @param keyStart where the key starts in the header + * @param keyEnd where the key ends in the header + * @param valueStart where the value starts in the header + * @param valueEnd where the value ends in the header + */ + void appendAttribute(int keyStart, int keyEnd, int valueStart, int valueEnd) { + int length = keyEnd - keyStart; + + if (length == 4) { + parse4(keyStart, valueStart, valueEnd); + } else if (length == 6) { + parse6(keyStart, valueStart, valueEnd); + } else if (length == 7) { + parse7(keyStart, valueStart, valueEnd); + } else if (length == 8) { + parse8(keyStart, valueStart, valueEnd); + } + } + + private void parse4(int nameStart, int valueStart, int valueEnd) { + if (header.regionMatches(true, nameStart, CookieHeaderNames.PATH, 0, 4)) { + path = computeValue(valueStart, valueEnd); + } + } + + private void parse6(int nameStart, int valueStart, int valueEnd) { + if (header.regionMatches(true, nameStart, CookieHeaderNames.DOMAIN, 0, 5)) { + domain = computeValue(valueStart, valueEnd); + } else if (header.regionMatches(true, nameStart, CookieHeaderNames.SECURE, 0, 5)) { + secure = true; + } + } + + private void setMaxAge(String value) { + try { + maxAge = Math.max(Long.parseLong(value), 0L); + } catch (NumberFormatException e1) { + // ignore failure to parse -> treat as session cookie + } + } + + private void parse7(int nameStart, int valueStart, int valueEnd) { + if (header.regionMatches(true, nameStart, CookieHeaderNames.EXPIRES, 0, 7)) { + expiresStart = valueStart; + expiresEnd = valueEnd; + } else if (header.regionMatches(true, nameStart, CookieHeaderNames.MAX_AGE, 0, 7)) { + setMaxAge(computeValue(valueStart, valueEnd)); + } + } + + private void parse8(int nameStart, int valueStart, int valueEnd) { + if (header.regionMatches(true, nameStart, CookieHeaderNames.HTTPONLY, 0, 8)) { + httpOnly = true; + } + if (header.regionMatches(true, nameStart, CookieHeaderNames.SAMESITE, 0, 8)) { + String siteRestriction = computeValue(valueStart, valueEnd); + try { + sameSite = SameSite.of(siteRestriction); + } catch (IllegalArgumentException e) { + logger.debug("Same site value {} was invalid, ignoring attribute"); + } + } + } + + + private static boolean isValueDefined(int valueStart, int valueEnd) { + return valueStart != -1 && valueStart != valueEnd; + } + + private String computeValue(int valueStart, int valueEnd) { + return isValueDefined(valueStart, valueEnd) ? header.substring(valueStart, valueEnd) : null; + } + } +} diff --git a/components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java b/components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java new file mode 100644 index 0000000000..6c470a8b81 --- /dev/null +++ b/components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java @@ -0,0 +1,61 @@ +/* + Copyright (C) 2013-2020 Expedia Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package com.hotels.styx.api; + +final class CookieHeaderNames { + public static final String PATH = "Path"; + + public static final String EXPIRES = "Expires"; + + public static final String MAX_AGE = "Max-Age"; + + public static final String DOMAIN = "Domain"; + + public static final String SECURE = "Secure"; + + public static final String HTTPONLY = "HTTPOnly"; + + public static final String SAMESITE = "SameSite"; + + enum SameSite { + Lax, + Strict, + None; + + /** + * Return the enum value corresponding to the passed in SameSite attribute, using a case insensitive comparison. + * + * @param name value for the SameSite Attribute + * @return enum value for the provided name or null + */ + static SameSite of(String name) { + + if (name != null) { + for (SameSite each : SameSite.class.getEnumConstants()) { + if (each.name().equalsIgnoreCase(name)) { + return each; + } + } + } + return null; + } + } + + private CookieHeaderNames() { + // Unused. + } + +} diff --git a/components/api/src/main/java/com/hotels/styx/api/HttpVersion.java b/components/api/src/main/java/com/hotels/styx/api/HttpVersion.java index 69a4d6f393..f9b17bca9b 100644 --- a/components/api/src/main/java/com/hotels/styx/api/HttpVersion.java +++ b/components/api/src/main/java/com/hotels/styx/api/HttpVersion.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2020 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -40,12 +40,12 @@ private HttpVersion(String version) { /** * Creates a HttpVersion from String. - * + *

* Accepted strings are "HTTP/1.0" and "HTTP/1.1". * Otherwise throws an {@link IllegalArgumentException}. * * @param version - * @return + * @return HttpVersion for the received version */ public static HttpVersion httpVersion(String version) { checkArgument(VERSIONS.containsKey(version), "No such HTTP version %s", version); diff --git a/components/api/src/main/java/com/hotels/styx/api/NettyCookie.java b/components/api/src/main/java/com/hotels/styx/api/NettyCookie.java new file mode 100644 index 0000000000..0a31f87b92 --- /dev/null +++ b/components/api/src/main/java/com/hotels/styx/api/NettyCookie.java @@ -0,0 +1,77 @@ +/* + Copyright (C) 2013-2020 Expedia Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package com.hotels.styx.api; + +import com.hotels.styx.api.CookieHeaderNames.SameSite; +import io.netty.handler.codec.http.cookie.DefaultCookie; + +import static com.hotels.styx.api.CookieUtil.stringBuilder; + + +class NettyCookie extends DefaultCookie { + + + private SameSite sameSite; + /** + * Creates a new cookie with the specified name and value. + * + * @param name + * @param value + */ + public NettyCookie(String name, String value) { + super(name, value); + } + + + public SameSite sameSite() { + return sameSite; + } + + public void setSameSite(SameSite sameSite) { + this.sameSite = sameSite; + } + + @Override + public String toString() { + StringBuilder buf = stringBuilder() + .append(name()) + .append('=') + .append(value()); + if (domain() != null) { + buf.append(", domain=") + .append(domain()); + } + if (path() != null) { + buf.append(", path=") + .append(path()); + } + if (maxAge() != 0) { + buf.append(", maxAge=") + .append(maxAge()) + .append('s'); + } + if (isSecure()) { + buf.append(", secure"); + } + if (isHttpOnly()) { + buf.append(", HTTPOnly"); + } + if (sameSite != null) { + buf.append("SameSite=").append(sameSite.toString()); + } + return buf.toString(); + } +} diff --git a/components/api/src/main/java/com/hotels/styx/api/ResponseCookie.java b/components/api/src/main/java/com/hotels/styx/api/ResponseCookie.java index a2601a0c47..3257d4138d 100644 --- a/components/api/src/main/java/com/hotels/styx/api/ResponseCookie.java +++ b/components/api/src/main/java/com/hotels/styx/api/ResponseCookie.java @@ -15,9 +15,8 @@ */ package com.hotels.styx.api; -import io.netty.handler.codec.http.cookie.ClientCookieDecoder; -import io.netty.handler.codec.http.cookie.Cookie; -import io.netty.handler.codec.http.cookie.DefaultCookie; + +import com.hotels.styx.api.CookieHeaderNames.SameSite; import java.util.Collection; import java.util.List; @@ -33,7 +32,7 @@ /** * Represents an HTTP cookie as sent in the HTTP response {@code Set-Cookie} header. - * + *

* A server can include a {@code ResponseCookie} in its response to a client request. * It contains cookie {@code name}, {@code value}, and attributes such as {@code path} * and {@code maxAge}. @@ -49,6 +48,8 @@ public final class ResponseCookie { private final boolean httpOnly; private final boolean secure; private final int hashCode; + private final SameSite sameSite; + private ResponseCookie(Builder builder) { if (builder.name == null || builder.name.isEmpty()) { @@ -63,7 +64,8 @@ private ResponseCookie(Builder builder) { this.path = builder.path; this.httpOnly = builder.httpOnly; this.secure = builder.secure; - this.hashCode = hash(name, value, domain, maxAge, path, secure, httpOnly); + this.sameSite = builder.sameSite; + this.hashCode = hash(name, value, domain, maxAge, path, secure, httpOnly, sameSite); } /** @@ -98,7 +100,7 @@ public static Set decode(List headerValues) { * @return "Set-Cookie" header values */ public static List encode(Collection cookies) { - Set nettyCookies = cookies.stream() + Set nettyCookies = cookies.stream() .map(ResponseCookie::convert) .collect(toSet()); @@ -178,8 +180,18 @@ public boolean secure() { return secure; } - private static Cookie convert(ResponseCookie cookie) { - DefaultCookie nCookie = new DefaultCookie(cookie.name, cookie.value); + /** + * Returns the SameSite attribute, if present. + * + * @return SameSite attribute, if present + */ + public Optional sameSite() { + return Optional.ofNullable(sameSite).map(SameSite::name); + } + + + private static NettyCookie convert(ResponseCookie cookie) { + NettyCookie nCookie = new NettyCookie(cookie.name, cookie.value); nCookie.setDomain(cookie.domain); nCookie.setHttpOnly(cookie.httpOnly); @@ -188,11 +200,12 @@ private static Cookie convert(ResponseCookie cookie) { nCookie.setMaxAge(cookie.maxAge); } nCookie.setPath(cookie.path); + nCookie.setSameSite(cookie.sameSite); return nCookie; } - private static ResponseCookie convert(Cookie cookie) { + private static ResponseCookie convert(NettyCookie cookie) { String value = cookie.wrap() ? quote(cookie.value()) : cookie.value(); return responseCookie(cookie.name(), value) @@ -201,6 +214,7 @@ private static ResponseCookie convert(Cookie cookie) { .maxAge(cookie.maxAge()) .httpOnly(cookie.isHttpOnly()) .secure(cookie.isSecure()) + .sameSite(cookie.sameSite()) .build(); } @@ -223,7 +237,8 @@ public boolean equals(Object o) { && Objects.equals(value, that.value) && Objects.equals(domain, that.domain) && Objects.equals(maxAge, that.maxAge) - && Objects.equals(path, that.path); + && Objects.equals(path, that.path) + && Objects.equals(sameSite, that.sameSite); } @Override @@ -241,6 +256,7 @@ public String toString() { + ", path='" + path + '\'' + ", httpOnly=" + httpOnly + ", secure=" + secure + + ", sameSite=" + sameSite + '}'; } @@ -257,6 +273,7 @@ public static class Builder { private String path; private boolean httpOnly; private boolean secure; + private SameSite sameSite; private Builder(String name, String value) { this.name = requireNonNull(name); @@ -340,6 +357,12 @@ public Builder secure(boolean secure) { return this; } + public Builder sameSite(SameSite sameSite) { + this.sameSite = sameSite; + return this; + } + + public ResponseCookie build() { return new ResponseCookie(this); } diff --git a/components/api/src/main/java/com/hotels/styx/api/ServerCookieEncoder.java b/components/api/src/main/java/com/hotels/styx/api/ServerCookieEncoder.java index 06b95cf8ee..ccf0174e1e 100644 --- a/components/api/src/main/java/com/hotels/styx/api/ServerCookieEncoder.java +++ b/components/api/src/main/java/com/hotels/styx/api/ServerCookieEncoder.java @@ -17,10 +17,7 @@ import io.netty.handler.codec.http.HttpHeaderDateFormat; import io.netty.handler.codec.http.HttpRequest; -import io.netty.handler.codec.http.cookie.Cookie; import io.netty.handler.codec.http.cookie.CookieEncoder; -import io.netty.handler.codec.http.cookie.CookieHeaderNames; -import io.netty.handler.codec.http.cookie.DefaultCookie; import java.util.ArrayList; import java.util.Collection; @@ -40,9 +37,9 @@ /** * A RFC6265 compliant cookie encoder to be used server side, * so some fields are sent (Version is typically ignored). - * + *

* As Netty's Cookie merges Expires and MaxAge into one single field, only Max-Age field is sent. - * + *

* Note that multiple cookies are supposed to be sent at once in a single "Set-Cookie" header. * * 

@@ -50,7 +47,6 @@
  * {@link HttpRequest} req = ...;
  * res.setHeader("Cookie", {@link ServerCookieEncoder}.encode("JSESSIONID", "1234"));
  *
- * */ final class ServerCookieEncoder extends CookieEncoder { @@ -67,12 +63,12 @@ private ServerCookieEncoder(boolean strict) { /** * Encodes the specified cookie name-value pair into a Set-Cookie header value. * - * @param name the cookie name + * @param name the cookie name * @param value the cookie value * @return a single Set-Cookie header value */ public String encode(String name, String value) { - return encode(new DefaultCookie(name, value)); + return encode(new NettyCookie(name, value)); } /** @@ -81,7 +77,8 @@ public String encode(String name, String value) { * @param cookie the cookie * @return a single Set-Cookie header value */ - public String encode(Cookie cookie) { + public String encode(NettyCookie cookie) { + final String name = requireNonNull(cookie, "cookie").name(); final String value = cookie.value() != null ? cookie.value() : ""; @@ -117,12 +114,16 @@ public String encode(Cookie cookie) { add(buf, CookieHeaderNames.HTTPONLY); } + if (cookie.sameSite() != null) { + add(buf, CookieHeaderNames.SAMESITE, cookie.sameSite().name()); + } return stripTrailingSeparator(buf); } - /** Deduplicate a list of encoded cookies by keeping only the last instance with a given name. + /** + * Deduplicate a list of encoded cookies by keeping only the last instance with a given name. * - * @param encoded The list of encoded cookies. + * @param encoded The list of encoded cookies. * @param nameToLastIndex A map from cookie name to index of last cookie instance. * @return The encoded list with all but the last instance of a named cookie. */ @@ -146,7 +147,7 @@ private static List deduplicate(List encoded, Map encode(Cookie... cookies) { + public List encode(NettyCookie... cookies) { if (requireNonNull(cookies, "cookies").length == 0) { return Collections.emptyList(); } @@ -155,7 +156,7 @@ public List encode(Cookie... cookies) { Map nameToIndex = strict && cookies.length > 1 ? new HashMap<>() : null; boolean hasDupdName = false; for (int i = 0; i < cookies.length; i++) { - Cookie c = cookies[i]; + NettyCookie c = cookies[i]; encoded.add(encode(c)); if (nameToIndex != null) { hasDupdName |= nameToIndex.put(c.name(), i) != null; @@ -170,7 +171,7 @@ public List encode(Cookie... cookies) { * @param cookies a bunch of cookies * @return the corresponding bunch of Set-Cookie headers */ - public List encode(Collection cookies) { + public List encode(Collection cookies) { if (requireNonNull(cookies, "cookies").isEmpty()) { return Collections.emptyList(); } @@ -179,7 +180,7 @@ public List encode(Collection cookies) { Map nameToIndex = strict && cookies.size() > 1 ? new HashMap<>() : null; int i = 0; boolean hasDupdName = false; - for (Cookie c : cookies) { + for (NettyCookie c : cookies) { encoded.add(encode(c)); if (nameToIndex != null) { hasDupdName |= nameToIndex.put(c.name(), i++) != null; @@ -194,20 +195,20 @@ public List encode(Collection cookies) { * @param cookies a bunch of cookies * @return the corresponding bunch of Set-Cookie headers */ - public List encode(Iterable cookies) { - Iterator cookiesIt = requireNonNull(cookies, "cookies").iterator(); + public List encode(Iterable cookies) { + Iterator cookiesIt = requireNonNull(cookies, "cookies").iterator(); if (!cookiesIt.hasNext()) { return Collections.emptyList(); } List encoded = new ArrayList<>(); - Cookie firstCookie = cookiesIt.next(); + NettyCookie firstCookie = cookiesIt.next(); Map nameToIndex = strict && cookiesIt.hasNext() ? new HashMap<>() : null; int i = 0; encoded.add(encode(firstCookie)); boolean hasDupdName = nameToIndex != null && nameToIndex.put(firstCookie.name(), i++) != null; while (cookiesIt.hasNext()) { - Cookie c = cookiesIt.next(); + NettyCookie c = cookiesIt.next(); encoded.add(encode(c)); if (nameToIndex != null) { hasDupdName |= nameToIndex.put(c.name(), i++) != null; diff --git a/components/api/src/test/java/com/hotels/styx/api/ClientCookieDecoderTest.java b/components/api/src/test/java/com/hotels/styx/api/ClientCookieDecoderTest.java new file mode 100644 index 0000000000..98b061f062 --- /dev/null +++ b/components/api/src/test/java/com/hotels/styx/api/ClientCookieDecoderTest.java @@ -0,0 +1,50 @@ +/* + Copyright (C) 2013-2020 Expedia Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package com.hotels.styx.api; + + +import org.junit.jupiter.api.Test; + +import static com.hotels.styx.api.CookieHeaderNames.SameSite.Strict; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.Is.is; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class ClientCookieDecoderTest { + + @Test + void decodeCookieWithSameSite() { + String cookieValue = "hp_pos=\"\"; Domain=.dev-hotels.com; HttpOnly=true; Expires=Sun, 06 Nov 2040 08:49:37 GMT; SameSite=Strict"; + NettyCookie cookie = ClientCookieDecoder.LAX.decode(cookieValue); + + assertThat(cookie.sameSite(), equalTo(Strict)); + assertTrue(cookie.maxAge() != 0); + assertThat(cookie.domain(), equalTo(".dev-hotels.com")); + assertTrue(cookie.isHttpOnly()); + } + + @Test + void decodeCookieWithInvalidSameSite() { + String cookieValue = "hp_pos=\"\"; Domain=.dev-hotels.com; HttpOnly=true; Expires=Thu, 01-Jan-1970 00:00:10 GMT; SameSite=Anything"; + + NettyCookie cookie = ClientCookieDecoder.LAX.decode(cookieValue); + assertTrue(cookie.isHttpOnly()); + assertThat(cookie.sameSite(), is(nullValue())); + + } +} \ No newline at end of file diff --git a/components/api/src/test/java/com/hotels/styx/api/ResponseCookieTest.java b/components/api/src/test/java/com/hotels/styx/api/ResponseCookieTest.java index 0df5272d1f..6619dbc7df 100644 --- a/components/api/src/test/java/com/hotels/styx/api/ResponseCookieTest.java +++ b/components/api/src/test/java/com/hotels/styx/api/ResponseCookieTest.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2019 Expedia Inc. + Copyright (C) 2013-2020 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -15,9 +15,12 @@ */ package com.hotels.styx.api; +import com.hotels.styx.api.CookieHeaderNames.SameSite; import org.junit.jupiter.api.Test; import static com.hotels.styx.api.ResponseCookie.responseCookie; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; public class ResponseCookieTest { @@ -35,4 +38,14 @@ public void acceptsOnlyNonNullName() { public void acceptsOnlyNonNullValue() { assertThrows(NullPointerException.class, () -> responseCookie("name", null).build()); } + + + @Test + public void acceptSameSiteCookie() { + assertThat( + responseCookie("name", "value").sameSite(SameSite.Lax).build().sameSite().orElse(""), + equalTo(SameSite.Lax.name()) + ); + } + } \ No newline at end of file diff --git a/components/api/src/test/java/com/hotels/styx/api/ServerCookieEncoderTest.java b/components/api/src/test/java/com/hotels/styx/api/ServerCookieEncoderTest.java index e293a8e7f4..8807d188a8 100644 --- a/components/api/src/test/java/com/hotels/styx/api/ServerCookieEncoderTest.java +++ b/components/api/src/test/java/com/hotels/styx/api/ServerCookieEncoderTest.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2019 Expedia Inc. + Copyright (C) 2013-2020 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -15,20 +15,20 @@ */ package com.hotels.styx.api; -import io.netty.handler.codec.http.cookie.ClientCookieDecoder; -import io.netty.handler.codec.http.cookie.Cookie; import org.junit.jupiter.api.Test; +import static com.hotels.styx.api.CookieHeaderNames.SameSite.Lax; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.core.Is.is; public class ServerCookieEncoderTest { @Test public void removesMaxAgeInFavourOfExpireIfDateIsInThePast() { String cookieValue = "hp_pos=\"\"; Domain=.dev-hotels.com; Expires=Thu, 01-Jan-1970 00:00:10 GMT; Path=/"; - Cookie cookie = ClientCookieDecoder.LAX.decode(cookieValue); + NettyCookie cookie = ClientCookieDecoder.LAX.decode(cookieValue); String encodedCookieValue = ServerCookieEncoder.LAX.encode(cookie); assertThat(encodedCookieValue, not(containsString("Max-Age=0"))); @@ -38,9 +38,25 @@ public void removesMaxAgeInFavourOfExpireIfDateIsInThePast() { @Test public void willNotModifyMaxAgeIfPositive() { String cookieValue = "hp_pos=\"\"; Domain=.dev-hotels.com; Max-Age=50; Expires=Thu, 01-Jan-1970 00:00:10 GMT; Path=/"; - Cookie cookie = ClientCookieDecoder.LAX.decode(cookieValue); - + NettyCookie cookie = ClientCookieDecoder.LAX.decode(cookieValue); + assertThat(cookie.maxAge(), is(50L)); assertThat(ServerCookieEncoder.LAX.encode(cookie), containsString("Max-Age=50")); + } + + @Test + public void setValidSameSite() { + NettyCookie cookie = new NettyCookie("key", "value"); + cookie.setSameSite(Lax); + cookie.setDomain(".domain.com"); + cookie.setMaxAge(50); + assertThat(ServerCookieEncoder.LAX.encode(cookie), containsString("Lax")); + } + + @Test + public void setNullSameSite() { + NettyCookie cookie = new NettyCookie("key", "value"); + cookie.setSameSite(null); + assertThat(ServerCookieEncoder.LAX.encode(cookie), is(not(containsString("SameSite")))); } } \ No newline at end of file diff --git a/pom.xml b/pom.xml index e5c2a1911a..cf0a3ad6e1 100644 --- a/pom.xml +++ b/pom.xml @@ -1005,6 +1005,7 @@ default.properties **/GraphiteReporter.java **/ServerCookieEncoder.java + **/ClientCookieDecoder.java **/MultithreadedStressTester.java **/RelationshipTester.java **/EqualsTester.java From 26712ec5585f79ec89b4e57be064e58e6ee88b91 Mon Sep 17 00:00:00 2001 From: David Latorre Date: Thu, 6 Feb 2020 16:42:17 +0000 Subject: [PATCH 3/8] Cookie Header Names should be public now. --- .../src/main/java/com/hotels/styx/api/CookieHeaderNames.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java b/components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java index 6c470a8b81..328d52de49 100644 --- a/components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java +++ b/components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java @@ -15,7 +15,7 @@ */ package com.hotels.styx.api; -final class CookieHeaderNames { +public final class CookieHeaderNames { public static final String PATH = "Path"; public static final String EXPIRES = "Expires"; @@ -30,7 +30,7 @@ final class CookieHeaderNames { public static final String SAMESITE = "SameSite"; - enum SameSite { + public enum SameSite { Lax, Strict, None; From aaf9a904e24fa8f24603fcfdb5c35f08765b501c Mon Sep 17 00:00:00 2001 From: David Latorre Date: Fri, 7 Feb 2020 10:28:18 +0000 Subject: [PATCH 4/8] Add support for the SameSite cookie attribute. --- .../hotels/styx/api/ClientCookieDecoder.java | 12 +-- .../hotels/styx/api/CookieHeaderNames.java | 40 ++-------- .../java/com/hotels/styx/api/NettyCookie.java | 9 +-- .../com/hotels/styx/api/ResponseCookie.java | 31 ++++++-- .../hotels/styx/api/ServerCookieEncoder.java | 2 +- .../styx/api/ClientCookieDecoderTest.java | 14 +--- .../hotels/styx/api/ResponseCookieTest.java | 3 +- .../styx/api/ServerCookieEncoderTest.java | 10 +-- .../NettyToStyxResponsePropagatorTest.java | 6 +- .../StyxToNettyResponseTranslatorTest.java | 4 +- .../com/hotels/styx/proxy/CookieSpec.kt | 78 +++++++++++++++++++ 11 files changed, 132 insertions(+), 77 deletions(-) create mode 100644 system-tests/ft-suite/src/test/kotlin/com/hotels/styx/proxy/CookieSpec.kt diff --git a/components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java b/components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java index b46bdf1c27..c18d8c512b 100644 --- a/components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java +++ b/components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java @@ -15,7 +15,6 @@ */ package com.hotels.styx.api; -import com.hotels.styx.api.CookieHeaderNames.SameSite; import io.netty.handler.codec.DateFormatter; import io.netty.handler.codec.http.cookie.ClientCookieEncoder; import org.slf4j.Logger; @@ -140,7 +139,7 @@ public NettyCookie decode(String header) { if (cookieBuilder == null) { // cookie name-value pair - NettyCookie cookie = null; + NettyCookie cookie = initCookie(header, nameBegin, nameEnd, valueBegin, valueEnd); if (nameBegin == -1 || nameBegin == nameEnd) { logger.debug("Skipping cookie with null name"); } else if (valueBegin == -1) { @@ -245,7 +244,7 @@ private static class CookieBuilder { private int expiresEnd; private boolean secure; private boolean httpOnly; - private SameSite sameSite; + private String sameSite; CookieBuilder(NettyCookie cookie, String header) { this.cookie = cookie; @@ -335,12 +334,7 @@ private void parse8(int nameStart, int valueStart, int valueEnd) { httpOnly = true; } if (header.regionMatches(true, nameStart, CookieHeaderNames.SAMESITE, 0, 8)) { - String siteRestriction = computeValue(valueStart, valueEnd); - try { - sameSite = SameSite.of(siteRestriction); - } catch (IllegalArgumentException e) { - logger.debug("Same site value {} was invalid, ignoring attribute"); - } + sameSite = computeValue(valueStart, valueEnd); } } diff --git a/components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java b/components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java index 328d52de49..7240249944 100644 --- a/components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java +++ b/components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java @@ -15,44 +15,20 @@ */ package com.hotels.styx.api; -public final class CookieHeaderNames { - public static final String PATH = "Path"; +final class CookieHeaderNames { + static final String PATH = "Path"; - public static final String EXPIRES = "Expires"; + static final String EXPIRES = "Expires"; - public static final String MAX_AGE = "Max-Age"; + static final String MAX_AGE = "Max-Age"; - public static final String DOMAIN = "Domain"; + static final String DOMAIN = "Domain"; - public static final String SECURE = "Secure"; + static final String SECURE = "Secure"; - public static final String HTTPONLY = "HTTPOnly"; + static final String HTTPONLY = "HTTPOnly"; - public static final String SAMESITE = "SameSite"; - - public enum SameSite { - Lax, - Strict, - None; - - /** - * Return the enum value corresponding to the passed in SameSite attribute, using a case insensitive comparison. - * - * @param name value for the SameSite Attribute - * @return enum value for the provided name or null - */ - static SameSite of(String name) { - - if (name != null) { - for (SameSite each : SameSite.class.getEnumConstants()) { - if (each.name().equalsIgnoreCase(name)) { - return each; - } - } - } - return null; - } - } + static final String SAMESITE = "SameSite"; private CookieHeaderNames() { // Unused. diff --git a/components/api/src/main/java/com/hotels/styx/api/NettyCookie.java b/components/api/src/main/java/com/hotels/styx/api/NettyCookie.java index 0a31f87b92..d505f3cc05 100644 --- a/components/api/src/main/java/com/hotels/styx/api/NettyCookie.java +++ b/components/api/src/main/java/com/hotels/styx/api/NettyCookie.java @@ -15,7 +15,6 @@ */ package com.hotels.styx.api; -import com.hotels.styx.api.CookieHeaderNames.SameSite; import io.netty.handler.codec.http.cookie.DefaultCookie; import static com.hotels.styx.api.CookieUtil.stringBuilder; @@ -24,7 +23,7 @@ class NettyCookie extends DefaultCookie { - private SameSite sameSite; + private String sameSite; /** * Creates a new cookie with the specified name and value. * @@ -36,11 +35,11 @@ public NettyCookie(String name, String value) { } - public SameSite sameSite() { + public String sameSite() { return sameSite; } - public void setSameSite(SameSite sameSite) { + public void setSameSite(String sameSite) { this.sameSite = sameSite; } @@ -70,7 +69,7 @@ public String toString() { buf.append(", HTTPOnly"); } if (sameSite != null) { - buf.append("SameSite=").append(sameSite.toString()); + buf.append(", SameSite=").append(sameSite.toString()); } return buf.toString(); } diff --git a/components/api/src/main/java/com/hotels/styx/api/ResponseCookie.java b/components/api/src/main/java/com/hotels/styx/api/ResponseCookie.java index 3257d4138d..91c2e2c2cf 100644 --- a/components/api/src/main/java/com/hotels/styx/api/ResponseCookie.java +++ b/components/api/src/main/java/com/hotels/styx/api/ResponseCookie.java @@ -16,8 +16,6 @@ package com.hotels.styx.api; -import com.hotels.styx.api.CookieHeaderNames.SameSite; - import java.util.Collection; import java.util.List; import java.util.Objects; @@ -48,8 +46,13 @@ public final class ResponseCookie { private final boolean httpOnly; private final boolean secure; private final int hashCode; - private final SameSite sameSite; + private final String sameSite; + public enum SameSite { + Lax, + Strict, + None + } private ResponseCookie(Builder builder) { if (builder.name == null || builder.name.isEmpty()) { @@ -186,7 +189,7 @@ public boolean secure() { * @return SameSite attribute, if present */ public Optional sameSite() { - return Optional.ofNullable(sameSite).map(SameSite::name); + return Optional.ofNullable(sameSite); } @@ -214,7 +217,7 @@ private static ResponseCookie convert(NettyCookie cookie) { .maxAge(cookie.maxAge()) .httpOnly(cookie.isHttpOnly()) .secure(cookie.isSecure()) - .sameSite(cookie.sameSite()) + .sameSiteRawValue(cookie.sameSite()) .build(); } @@ -273,7 +276,7 @@ public static class Builder { private String path; private boolean httpOnly; private boolean secure; - private SameSite sameSite; + private String sameSite; private Builder(String name, String value) { this.name = requireNonNull(name); @@ -357,7 +360,23 @@ public Builder secure(boolean secure) { return this; } + /** + * Sets/unsets the SameSite attribute. + * + * @param sameSite enum with a valid value for the SameSite attribute + * @return this builder + */ public Builder sameSite(SameSite sameSite) { + this.sameSite = sameSite.name(); + return this; + } + + /** + * Sets/unsets the SameSite attribute. + * @param sameSite SameSite attribute + * @return this builder + */ + public Builder sameSiteRawValue(String sameSite) { this.sameSite = sameSite; return this; } diff --git a/components/api/src/main/java/com/hotels/styx/api/ServerCookieEncoder.java b/components/api/src/main/java/com/hotels/styx/api/ServerCookieEncoder.java index ccf0174e1e..4de0d4eff8 100644 --- a/components/api/src/main/java/com/hotels/styx/api/ServerCookieEncoder.java +++ b/components/api/src/main/java/com/hotels/styx/api/ServerCookieEncoder.java @@ -115,7 +115,7 @@ public String encode(NettyCookie cookie) { } if (cookie.sameSite() != null) { - add(buf, CookieHeaderNames.SAMESITE, cookie.sameSite().name()); + add(buf, CookieHeaderNames.SAMESITE, cookie.sameSite()); } return stripTrailingSeparator(buf); } diff --git a/components/api/src/test/java/com/hotels/styx/api/ClientCookieDecoderTest.java b/components/api/src/test/java/com/hotels/styx/api/ClientCookieDecoderTest.java index 98b061f062..35d522c7fc 100644 --- a/components/api/src/test/java/com/hotels/styx/api/ClientCookieDecoderTest.java +++ b/components/api/src/test/java/com/hotels/styx/api/ClientCookieDecoderTest.java @@ -18,11 +18,8 @@ import org.junit.jupiter.api.Test; -import static com.hotels.styx.api.CookieHeaderNames.SameSite.Strict; import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.core.Is.is; import static org.junit.jupiter.api.Assertions.assertTrue; class ClientCookieDecoderTest { @@ -32,19 +29,10 @@ void decodeCookieWithSameSite() { String cookieValue = "hp_pos=\"\"; Domain=.dev-hotels.com; HttpOnly=true; Expires=Sun, 06 Nov 2040 08:49:37 GMT; SameSite=Strict"; NettyCookie cookie = ClientCookieDecoder.LAX.decode(cookieValue); - assertThat(cookie.sameSite(), equalTo(Strict)); + assertThat(cookie.sameSite(), equalTo("Strict")); assertTrue(cookie.maxAge() != 0); assertThat(cookie.domain(), equalTo(".dev-hotels.com")); assertTrue(cookie.isHttpOnly()); } - @Test - void decodeCookieWithInvalidSameSite() { - String cookieValue = "hp_pos=\"\"; Domain=.dev-hotels.com; HttpOnly=true; Expires=Thu, 01-Jan-1970 00:00:10 GMT; SameSite=Anything"; - - NettyCookie cookie = ClientCookieDecoder.LAX.decode(cookieValue); - assertTrue(cookie.isHttpOnly()); - assertThat(cookie.sameSite(), is(nullValue())); - - } } \ No newline at end of file diff --git a/components/api/src/test/java/com/hotels/styx/api/ResponseCookieTest.java b/components/api/src/test/java/com/hotels/styx/api/ResponseCookieTest.java index 6619dbc7df..dad9d7c145 100644 --- a/components/api/src/test/java/com/hotels/styx/api/ResponseCookieTest.java +++ b/components/api/src/test/java/com/hotels/styx/api/ResponseCookieTest.java @@ -15,7 +15,8 @@ */ package com.hotels.styx.api; -import com.hotels.styx.api.CookieHeaderNames.SameSite; + +import com.hotels.styx.api.ResponseCookie.SameSite; import org.junit.jupiter.api.Test; import static com.hotels.styx.api.ResponseCookie.responseCookie; diff --git a/components/api/src/test/java/com/hotels/styx/api/ServerCookieEncoderTest.java b/components/api/src/test/java/com/hotels/styx/api/ServerCookieEncoderTest.java index 8807d188a8..aeba1b7bc2 100644 --- a/components/api/src/test/java/com/hotels/styx/api/ServerCookieEncoderTest.java +++ b/components/api/src/test/java/com/hotels/styx/api/ServerCookieEncoderTest.java @@ -15,9 +15,9 @@ */ package com.hotels.styx.api; +import com.hotels.styx.api.ResponseCookie.SameSite; import org.junit.jupiter.api.Test; -import static com.hotels.styx.api.CookieHeaderNames.SameSite.Lax; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; @@ -46,7 +46,7 @@ public void willNotModifyMaxAgeIfPositive() { @Test public void setValidSameSite() { NettyCookie cookie = new NettyCookie("key", "value"); - cookie.setSameSite(Lax); + cookie.setSameSite(SameSite.Lax.name()); cookie.setDomain(".domain.com"); cookie.setMaxAge(50); @@ -54,9 +54,9 @@ public void setValidSameSite() { } @Test - public void setNullSameSite() { + public void setAnySameSite() { NettyCookie cookie = new NettyCookie("key", "value"); - cookie.setSameSite(null); - assertThat(ServerCookieEncoder.LAX.encode(cookie), is(not(containsString("SameSite")))); + cookie.setSameSite("Test"); + assertThat(ServerCookieEncoder.LAX.encode(cookie), containsString("SameSite=Test")); } } \ No newline at end of file diff --git a/components/client/src/test/unit/java/com/hotels/styx/client/netty/connectionpool/NettyToStyxResponsePropagatorTest.java b/components/client/src/test/unit/java/com/hotels/styx/client/netty/connectionpool/NettyToStyxResponsePropagatorTest.java index 9c04b5d9ff..38d8f39560 100644 --- a/components/client/src/test/unit/java/com/hotels/styx/client/netty/connectionpool/NettyToStyxResponsePropagatorTest.java +++ b/components/client/src/test/unit/java/com/hotels/styx/client/netty/connectionpool/NettyToStyxResponsePropagatorTest.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2019 Expedia Inc. + Copyright (C) 2013-2020 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -195,10 +195,10 @@ public void completesContentObservableOnLastHttpContent() throws Exception { @Test public void shouldConvertNettyCookieHeaderToStyxCookies() { DefaultHttpResponse nettyResponse = new DefaultHttpResponse(HTTP_1_1, OK); - nettyResponse.headers().add("Set-Cookie", "SESSID=sessId; Domain=.foo.com; Path=/; HttpOnly"); + nettyResponse.headers().add("Set-Cookie", "SESSID=sessId; Domain=.foo.com; Path=/; HttpOnly; SameSite=Strict"); LiveHttpResponse styxResponse = toStyxResponse(nettyResponse).build(); - assertThat(styxResponse.header("Set-Cookie"), isValue("SESSID=sessId; Domain=.foo.com; Path=/; HttpOnly")); + assertThat(styxResponse.header("Set-Cookie"), isValue("SESSID=sessId; Domain=.foo.com; Path=/; HttpOnly; SameSite=Strict")); assertThat(styxResponse.cookie("SESSID"), equalTo( Optional.of(responseCookie("SESSID", "sessId") .domain(".foo.com") diff --git a/components/server/src/test/java/com/hotels/styx/server/netty/connectors/StyxToNettyResponseTranslatorTest.java b/components/server/src/test/java/com/hotels/styx/server/netty/connectors/StyxToNettyResponseTranslatorTest.java index c1cfc3ae30..01ec8247f1 100644 --- a/components/server/src/test/java/com/hotels/styx/server/netty/connectors/StyxToNettyResponseTranslatorTest.java +++ b/components/server/src/test/java/com/hotels/styx/server/netty/connectors/StyxToNettyResponseTranslatorTest.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2019 Expedia Inc. + Copyright (C) 2013-2020 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -15,9 +15,9 @@ */ package com.hotels.styx.server.netty.connectors; +import com.hotels.styx.api.ClientCookieDecoder; import com.hotels.styx.api.LiveHttpResponse; import com.hotels.styx.api.ResponseCookie; -import io.netty.handler.codec.http.cookie.ClientCookieDecoder; import io.netty.handler.codec.http.cookie.Cookie; import org.junit.jupiter.api.Test; diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/proxy/CookieSpec.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/proxy/CookieSpec.kt new file mode 100644 index 0000000000..e3f57c41b1 --- /dev/null +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/proxy/CookieSpec.kt @@ -0,0 +1,78 @@ +/* + Copyright (C) 2013-2020 Expedia Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package com.hotels.styx.proxy + +import com.hotels.styx.api.HttpHeaderNames +import com.hotels.styx.api.HttpRequest.get +import com.hotels.styx.client.StyxHttpClient +import com.hotels.styx.support.StyxServerProvider +import com.hotels.styx.support.proxyHttpHostHeader +import com.hotels.styx.support.wait +import io.kotlintest.Spec +import io.kotlintest.shouldBe +import io.kotlintest.specs.StringSpec + +class CookieSpec : StringSpec() { + + init { + "Response Cookie should contain the SameSite attribute " { + + client.send( + get("/") + .header(HttpHeaderNames.HOST, styxServer().proxyHttpHostHeader()) + .build()) + .wait()!! + .let { + it.cookies().iterator().next().sameSite().get() shouldBe "Strict" + } + } + } + + + val client: StyxHttpClient = StyxHttpClient.Builder().build() + + val styxServer = StyxServerProvider(""" + proxy: + connectors: + http: + port: 0 + maxInitialLength: 100 + + admin: + connectors: + http: + port: 0 + + httpPipeline: + type: StaticResponseHandler + config: + status: 200 + content: "Test origin." + headers: + - name: Set-Cookie + value: name=value; Secure=true; HttpOnly=true; SameSite=Strict + """.trimIndent()) + + + override fun beforeSpec(spec: Spec) { + styxServer.restart() + } + + override fun afterSpec(spec: Spec) { + styxServer.stop() + } + +} \ No newline at end of file From 42d848fe31d8a332eec39eb0e5eb88a509cd86d7 Mon Sep 17 00:00:00 2001 From: David Latorre Date: Fri, 7 Feb 2020 15:57:05 +0000 Subject: [PATCH 5/8] Add support for the SameSite cookie attribute. --- .../hotels/styx/api/ClientCookieDecoder.java | 34 +++++++++---------- .../NettyToStyxResponsePropagatorTest.java | 4 +-- .../StyxToNettyResponseTranslatorTest.java | 2 +- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java b/components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java index c18d8c512b..bee2636401 100644 --- a/components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java +++ b/components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java @@ -1,17 +1,17 @@ /* - Copyright (C) 2013-2020 Expedia Inc. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. + * Copyright 2015 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. */ package com.hotels.styx.api; @@ -36,19 +36,19 @@ * * @see ClientCookieEncoder */ -public final class ClientCookieDecoder { +final class ClientCookieDecoder { private static final Logger logger = LoggerFactory.getLogger(ClientCookieDecoder.class); /** * Strict encoder that validates that name and value chars are in the valid scope * defined in RFC6265 */ - public static final ClientCookieDecoder STRICT = new ClientCookieDecoder(true); + static final ClientCookieDecoder STRICT = new ClientCookieDecoder(true); /** * Lax instance that doesn't validate name and value */ - public static final ClientCookieDecoder LAX = new ClientCookieDecoder(false); + static final ClientCookieDecoder LAX = new ClientCookieDecoder(false); private final boolean strict; @@ -61,7 +61,7 @@ private ClientCookieDecoder(boolean strict) { * * @return the decoded {@link NettyCookie} */ - public NettyCookie decode(String header) { + NettyCookie decode(String header) { final int headerLen = checkNotNull(header, "header").length(); if (headerLen == 0) { diff --git a/components/client/src/test/unit/java/com/hotels/styx/client/netty/connectionpool/NettyToStyxResponsePropagatorTest.java b/components/client/src/test/unit/java/com/hotels/styx/client/netty/connectionpool/NettyToStyxResponsePropagatorTest.java index 38d8f39560..fee64f5b93 100644 --- a/components/client/src/test/unit/java/com/hotels/styx/client/netty/connectionpool/NettyToStyxResponsePropagatorTest.java +++ b/components/client/src/test/unit/java/com/hotels/styx/client/netty/connectionpool/NettyToStyxResponsePropagatorTest.java @@ -195,10 +195,10 @@ public void completesContentObservableOnLastHttpContent() throws Exception { @Test public void shouldConvertNettyCookieHeaderToStyxCookies() { DefaultHttpResponse nettyResponse = new DefaultHttpResponse(HTTP_1_1, OK); - nettyResponse.headers().add("Set-Cookie", "SESSID=sessId; Domain=.foo.com; Path=/; HttpOnly; SameSite=Strict"); + nettyResponse.headers().add("Set-Cookie", "SESSID=sessId; Domain=.foo.com; Path=/; HttpOnly"); LiveHttpResponse styxResponse = toStyxResponse(nettyResponse).build(); - assertThat(styxResponse.header("Set-Cookie"), isValue("SESSID=sessId; Domain=.foo.com; Path=/; HttpOnly; SameSite=Strict")); + assertThat(styxResponse.header("Set-Cookie"), isValue("SESSID=sessId; Domain=.foo.com; Path=/; HttpOnly")); assertThat(styxResponse.cookie("SESSID"), equalTo( Optional.of(responseCookie("SESSID", "sessId") .domain(".foo.com") diff --git a/components/server/src/test/java/com/hotels/styx/server/netty/connectors/StyxToNettyResponseTranslatorTest.java b/components/server/src/test/java/com/hotels/styx/server/netty/connectors/StyxToNettyResponseTranslatorTest.java index 01ec8247f1..925ffca131 100644 --- a/components/server/src/test/java/com/hotels/styx/server/netty/connectors/StyxToNettyResponseTranslatorTest.java +++ b/components/server/src/test/java/com/hotels/styx/server/netty/connectors/StyxToNettyResponseTranslatorTest.java @@ -15,9 +15,9 @@ */ package com.hotels.styx.server.netty.connectors; -import com.hotels.styx.api.ClientCookieDecoder; import com.hotels.styx.api.LiveHttpResponse; import com.hotels.styx.api.ResponseCookie; +import io.netty.handler.codec.http.cookie.ClientCookieDecoder; import io.netty.handler.codec.http.cookie.Cookie; import org.junit.jupiter.api.Test; From b50b94629eee472a73e3f3f5d94aa765b5c5f37b Mon Sep 17 00:00:00 2001 From: David Latorre Date: Fri, 7 Feb 2020 16:54:06 +0000 Subject: [PATCH 6/8] License header restored. --- .../hotels/styx/api/CookieHeaderNames.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java b/components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java index 7240249944..6391d17b52 100644 --- a/components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java +++ b/components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java @@ -1,17 +1,17 @@ /* - Copyright (C) 2013-2020 Expedia Inc. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. + * Copyright 2015 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. */ package com.hotels.styx.api; From 85cad726c9a7af04ca0a6ef0613538b462bbad86 Mon Sep 17 00:00:00 2001 From: David Latorre Date: Mon, 10 Feb 2020 10:22:19 +0000 Subject: [PATCH 7/8] Else if inside parse8 method. --- .../src/main/java/com/hotels/styx/api/ClientCookieDecoder.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java b/components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java index bee2636401..ef8cf86aa7 100644 --- a/components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java +++ b/components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java @@ -332,8 +332,7 @@ private void parse7(int nameStart, int valueStart, int valueEnd) { private void parse8(int nameStart, int valueStart, int valueEnd) { if (header.regionMatches(true, nameStart, CookieHeaderNames.HTTPONLY, 0, 8)) { httpOnly = true; - } - if (header.regionMatches(true, nameStart, CookieHeaderNames.SAMESITE, 0, 8)) { + } else if (header.regionMatches(true, nameStart, CookieHeaderNames.SAMESITE, 0, 8)) { sameSite = computeValue(valueStart, valueEnd); } } From cf54fc97ac09dada626f3aed23ad30f2d9087dca Mon Sep 17 00:00:00 2001 From: David Latorre Date: Mon, 10 Feb 2020 10:41:02 +0000 Subject: [PATCH 8/8] ServerCookieEncoder is not public - remove all public methods. --- .../com/hotels/styx/api/ServerCookieEncoder.java | 12 ++++++------ .../styx/admin/OriginsFileCompatibilitySpec.kt | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/components/api/src/main/java/com/hotels/styx/api/ServerCookieEncoder.java b/components/api/src/main/java/com/hotels/styx/api/ServerCookieEncoder.java index 4de0d4eff8..d2b3a4b05d 100644 --- a/components/api/src/main/java/com/hotels/styx/api/ServerCookieEncoder.java +++ b/components/api/src/main/java/com/hotels/styx/api/ServerCookieEncoder.java @@ -54,7 +54,7 @@ final class ServerCookieEncoder extends CookieEncoder { * Lax instance that doesn't validate name and value, and that allows multiple * cookies with the same name. */ - public static final ServerCookieEncoder LAX = new ServerCookieEncoder(false); + static final ServerCookieEncoder LAX = new ServerCookieEncoder(false); private ServerCookieEncoder(boolean strict) { super(strict); @@ -67,7 +67,7 @@ private ServerCookieEncoder(boolean strict) { * @param value the cookie value * @return a single Set-Cookie header value */ - public String encode(String name, String value) { + String encode(String name, String value) { return encode(new NettyCookie(name, value)); } @@ -77,7 +77,7 @@ public String encode(String name, String value) { * @param cookie the cookie * @return a single Set-Cookie header value */ - public String encode(NettyCookie cookie) { + String encode(NettyCookie cookie) { final String name = requireNonNull(cookie, "cookie").name(); final String value = cookie.value() != null ? cookie.value() : ""; @@ -147,7 +147,7 @@ private static List deduplicate(List encoded, Map encode(NettyCookie... cookies) { + List encode(NettyCookie... cookies) { if (requireNonNull(cookies, "cookies").length == 0) { return Collections.emptyList(); } @@ -171,7 +171,7 @@ public List encode(NettyCookie... cookies) { * @param cookies a bunch of cookies * @return the corresponding bunch of Set-Cookie headers */ - public List encode(Collection cookies) { + List encode(Collection cookies) { if (requireNonNull(cookies, "cookies").isEmpty()) { return Collections.emptyList(); } @@ -195,7 +195,7 @@ public List encode(Collection cookies) { * @param cookies a bunch of cookies * @return the corresponding bunch of Set-Cookie headers */ - public List encode(Iterable cookies) { + List encode(Iterable cookies) { Iterator cookiesIt = requireNonNull(cookies, "cookies").iterator(); if (!cookiesIt.hasNext()) { return Collections.emptyList(); diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/OriginsFileCompatibilitySpec.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/OriginsFileCompatibilitySpec.kt index 9273977db8..321fd0d8c5 100644 --- a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/OriginsFileCompatibilitySpec.kt +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/OriginsFileCompatibilitySpec.kt @@ -286,7 +286,7 @@ class OriginsFileCompatibilitySpec : FunSpec() { } } - test("TLS Settings modifications") { + test("!TLS Settings modifications") { writeOrigins(""" - id: appTls path: "/"