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..ef8cf86aa7 --- /dev/null +++ b/components/api/src/main/java/com/hotels/styx/api/ClientCookieDecoder.java @@ -0,0 +1,349 @@ +/* + * 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; + +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 + */ +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 + */ + static final ClientCookieDecoder STRICT = new ClientCookieDecoder(true); + + /** + * Lax instance that doesn't validate name and value + */ + 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} + */ + 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 = initCookie(header, nameBegin, nameEnd, valueBegin, valueEnd); + 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 String 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; + } else if (header.regionMatches(true, nameStart, CookieHeaderNames.SAMESITE, 0, 8)) { + sameSite = computeValue(valueStart, valueEnd); + } + } + + + 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..6391d17b52 --- /dev/null +++ b/components/api/src/main/java/com/hotels/styx/api/CookieHeaderNames.java @@ -0,0 +1,37 @@ +/* + * 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; + +final class CookieHeaderNames { + static final String PATH = "Path"; + + static final String EXPIRES = "Expires"; + + static final String MAX_AGE = "Max-Age"; + + static final String DOMAIN = "Domain"; + + static final String SECURE = "Secure"; + + static final String HTTPONLY = "HTTPOnly"; + + static final String SAMESITE = "SameSite"; + + private CookieHeaderNames() { + // Unused. + } + +} diff --git a/components/api/src/main/java/com/hotels/styx/api/HttpHeaders.java b/components/api/src/main/java/com/hotels/styx/api/HttpHeaders.java index 2511f2a65b..ed8e503ec3 100644 --- a/components/api/src/main/java/com/hotels/styx/api/HttpHeaders.java +++ b/components/api/src/main/java/com/hotels/styx/api/HttpHeaders.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. @@ -142,11 +142,11 @@ public static class Builder { private final DefaultHttpHeaders nettyHeaders; public Builder() { - this.nettyHeaders = new DefaultHttpHeaders(false); + this.nettyHeaders = new DefaultHttpHeaders(true); } public Builder(HttpHeaders headers) { - this.nettyHeaders = new DefaultHttpHeaders(false); + this.nettyHeaders = new DefaultHttpHeaders(true); this.nettyHeaders.set(headers.nettyHeaders); } 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..d505f3cc05 --- /dev/null +++ b/components/api/src/main/java/com/hotels/styx/api/NettyCookie.java @@ -0,0 +1,76 @@ +/* + 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 io.netty.handler.codec.http.cookie.DefaultCookie; + +import static com.hotels.styx.api.CookieUtil.stringBuilder; + + +class NettyCookie extends DefaultCookie { + + + private String 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 String sameSite() { + return sameSite; + } + + public void setSameSite(String 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..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 @@ -15,9 +15,6 @@ */ 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 java.util.Collection; import java.util.List; @@ -33,7 +30,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 +46,13 @@ public final class ResponseCookie { private final boolean httpOnly; private final boolean secure; private final int hashCode; + private final String sameSite; + + public enum SameSite { + Lax, + Strict, + None + } private ResponseCookie(Builder builder) { if (builder.name == null || builder.name.isEmpty()) { @@ -63,7 +67,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 +103,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 +183,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); + } + + + private static NettyCookie convert(ResponseCookie cookie) { + NettyCookie nCookie = new NettyCookie(cookie.name, cookie.value); nCookie.setDomain(cookie.domain); nCookie.setHttpOnly(cookie.httpOnly); @@ -188,11 +203,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 +217,7 @@ private static ResponseCookie convert(Cookie cookie) { .maxAge(cookie.maxAge()) .httpOnly(cookie.isHttpOnly()) .secure(cookie.isSecure()) + .sameSiteRawValue(cookie.sameSite()) .build(); } @@ -223,7 +240,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 +259,7 @@ public String toString() { + ", path='" + path + '\'' + ", httpOnly=" + httpOnly + ", secure=" + secure + + ", sameSite=" + sameSite + '}'; } @@ -257,6 +276,7 @@ public static class Builder { private String path; private boolean httpOnly; private boolean secure; + private String sameSite; private Builder(String name, String value) { this.name = requireNonNull(name); @@ -340,6 +360,28 @@ 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; + } + + 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..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 @@ -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 { @@ -58,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,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)); + String encode(String name, String 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) { + 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()); + } 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) { + 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) { + 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(); + 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..35d522c7fc --- /dev/null +++ b/components/api/src/test/java/com/hotels/styx/api/ClientCookieDecoderTest.java @@ -0,0 +1,38 @@ +/* + 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 org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; +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()); + } + +} \ No newline at end of file diff --git a/components/api/src/test/java/com/hotels/styx/api/HttpHeadersTest.java b/components/api/src/test/java/com/hotels/styx/api/HttpHeadersTest.java index e17bb8b3a1..e4c5ab28f8 100644 --- a/components/api/src/test/java/com/hotels/styx/api/HttpHeadersTest.java +++ b/components/api/src/test/java/com/hotels/styx/api/HttpHeadersTest.java @@ -128,11 +128,26 @@ public void removesNullValues() { @Test public void doesNotAllowNullName() { - assertThrows(NullPointerException.class, () -> new HttpHeaders.Builder() + assertThrows(IllegalArgumentException.class, () -> new HttpHeaders.Builder() .add(null, "value") .build()); } + + @Test + public void doesNotAllowNewLines() { + assertThrows(IllegalArgumentException.class, () -> new HttpHeaders.Builder() + .add("key", "value\rvalue2") + .build()); + + assertThrows(IllegalArgumentException.class, () -> new HttpHeaders.Builder() + .add("key", "value\r\nvalue2") + .build()); + assertThrows(IllegalArgumentException.class, () -> new HttpHeaders.Builder() + .add("key", "value\nvalue2") + .build()); + } + @Test public void setsDateHeaders() { Instant time = ZonedDateTime.of(2015, 9, 10, 12, 2, 28, 0, UTC).toInstant(); 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..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 @@ -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,13 @@ */ package com.hotels.styx.api; + +import com.hotels.styx.api.ResponseCookie.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 +39,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..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 @@ -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 com.hotels.styx.api.ResponseCookie.SameSite; import org.junit.jupiter.api.Test; 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(SameSite.Lax.name()); + cookie.setDomain(".domain.com"); + cookie.setMaxAge(50); + assertThat(ServerCookieEncoder.LAX.encode(cookie), containsString("Lax")); + } + + @Test + public void setAnySameSite() { + NettyCookie cookie = new NettyCookie("key", "value"); + cookie.setSameSite("Test"); + assertThat(ServerCookieEncoder.LAX.encode(cookie), containsString("SameSite=Test")); } } \ No newline at end of file diff --git a/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/HttpRequestOperation.java b/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/HttpRequestOperation.java index fd44a39048..99a3f6405f 100644 --- a/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/HttpRequestOperation.java +++ b/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/HttpRequestOperation.java @@ -105,7 +105,7 @@ static DefaultHttpRequest toNettyRequest(LiveHttpRequest request) { HttpVersion version = request.version(); HttpMethod method = request.method(); String url = request.url().toString(); - DefaultHttpRequest nettyRequest = new DefaultHttpRequest(toNettyVersion(version), toNettyMethod(method), url, false); + DefaultHttpRequest nettyRequest = new DefaultHttpRequest(toNettyVersion(version), toNettyMethod(method), url, true); request.headers().forEach((name, value) -> nettyRequest.headers().add(name, value)); diff --git a/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyToStyxResponsePropagator.java b/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyToStyxResponsePropagator.java index ef5367387e..2718821ff4 100644 --- a/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyToStyxResponsePropagator.java +++ b/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyToStyxResponsePropagator.java @@ -61,6 +61,7 @@ final class NettyToStyxResponsePropagator extends SimpleChannelInboundHandler { private static final Logger LOGGER = getLogger(NettyToStyxResponsePropagator.class); private final AtomicBoolean responseCompleted = new AtomicBoolean(false); + private final AtomicBoolean responseReceived = new AtomicBoolean(false); private final FluxSink sink; private final LiveHttpRequest request; @@ -119,11 +120,19 @@ protected void channelRead0(ChannelHandlerContext ctx, Object msg) throws Except if (msg instanceof io.netty.handler.codec.http.HttpResponse) { io.netty.handler.codec.http.HttpResponse nettyResponse = (io.netty.handler.codec.http.HttpResponse) msg; + + if (!responseReceived.compareAndSet(false, true)) { + LOGGER.warn("Unexpected additional response received: " + nettyResponse); + ctx.channel().close(); + return; + } + if (nettyResponse.getDecoderResult().isFailure()) { emitResponseError(new BadHttpResponseException(origin, nettyResponse.getDecoderResult().cause())); return; } + ctx.channel().config().setAutoRead(false); ctx.channel().read(); 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..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 @@ -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. diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt b/components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt index 19f7fc54c6..530e035e8b 100644 --- a/components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt +++ b/components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt @@ -85,14 +85,14 @@ internal class YamlFileConfigurationService( } override fun start() = fileMonitoringService.start() - .thenAccept { + .thenAcceptAsync { LOGGER.info("service starting - {}", config.originsFile) initialised.await() LOGGER.info("service started - {}", config.originsFile) } override fun stop() = fileMonitoringService.stop() - .thenAccept { + .thenAcceptAsync { LOGGER.info("service stopped") } diff --git a/components/server/src/main/java/com/hotels/styx/server/netty/connectors/StyxToNettyResponseTranslator.java b/components/server/src/main/java/com/hotels/styx/server/netty/connectors/StyxToNettyResponseTranslator.java index 02fb91a3fa..759be4364c 100644 --- a/components/server/src/main/java/com/hotels/styx/server/netty/connectors/StyxToNettyResponseTranslator.java +++ b/components/server/src/main/java/com/hotels/styx/server/netty/connectors/StyxToNettyResponseTranslator.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2019 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -27,7 +27,7 @@ public HttpResponse toNettyResponse(LiveHttpResponse httpResponse) { io.netty.handler.codec.http.HttpVersion version = toNettyVersion(httpResponse.version()); HttpResponseStatus httpResponseStatus = HttpResponseStatus.valueOf(httpResponse.status().code()); - DefaultHttpResponse nettyResponse = new DefaultHttpResponse(version, httpResponseStatus, false); + DefaultHttpResponse nettyResponse = new DefaultHttpResponse(version, httpResponseStatus, true); httpResponse.headers().forEach(httpHeader -> nettyResponse.headers().add(httpHeader.name(), httpHeader.value())); 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..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 @@ -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. 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 diff --git a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/BadResponseFromOriginSpec.scala b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/BadResponseFromOriginSpec.scala index 22624a1323..915c921082 100644 --- a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/BadResponseFromOriginSpec.scala +++ b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/BadResponseFromOriginSpec.scala @@ -29,7 +29,7 @@ import com.hotels.styx.support.{NettyOrigins, TestClientSupport} import com.hotels.styx.{DefaultStyxConfiguration, StyxProxySpec} import io.netty.buffer.Unpooled import io.netty.channel.ChannelHandlerContext -import io.netty.handler.codec.http.HttpHeaders.Names.{CONTENT_LENGTH, HOST, TRANSFER_ENCODING} +import io.netty.handler.codec.http.HttpHeaders.Names._ import io.netty.handler.codec.http.HttpHeaders.Values.CHUNKED import io.netty.handler.codec.http.HttpMethod._ import io.netty.handler.codec.http.HttpResponseStatus.OK @@ -110,6 +110,30 @@ class BadResponseFromOriginSpec extends FunSpec styxServer.metricsSnapshot.count("styx.response.status.502").get should be(1) } } + + + it("ignores second response from an origin") { + val twoResponses = (ctx: ChannelHandlerContext, msg: scala.Any) => { + val response = new DefaultFullHttpResponse(HTTP_1_1, OK) + response.headers().add(CONTENT_LENGTH, 0) + response.headers().add(ACCEPT_ENCODING, "ENCODING") + ctx.write(response) + + val response2 = new DefaultFullHttpResponse(HTTP_1_1, HttpResponseStatus.NOT_FOUND) + response2.headers().add(ACCEPT_CHARSET, "CHARSET") + ctx.writeAndFlush(response2) + } + + originRespondingWith(twoResponses) + val req = get("/badResponseFromOriginSpec/4") + .addHeader(HOST, styxServer.proxyHost) + .build() + + val resp = decodedRequest(req) + assert(resp.status().code() == 200) + assert(resp.header(ACCEPT_ENCODING).get() == "ENCODING") + assert(!resp.header(ACCEPT_CHARSET).isPresent) + } } def response200OkWithoutWaitingForFullRequest(messageBody: String): (ChannelHandlerContext, Any) => Unit = { diff --git a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/HeadersSpec.scala b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/HeadersSpec.scala index fd346e6293..fe279ece4e 100644 --- a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/HeadersSpec.scala +++ b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/HeadersSpec.scala @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2019 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -373,6 +373,41 @@ class HeadersSpec extends FunSpec assert(!resp.contentLength.isPresent, "content length header should be empty") assert(resp.header(TRANSFER_ENCODING).get() == CHUNKED) } + + it("should support new lines from received request headers in folding format (no CR)") { + recordingBackend.stub(urlPathEqualTo("/headers"), aResponse.withStatus(200)) + + val req = get("/headers") + .addHeader(HOST, styxServer.proxyHost) + .addHeader(ACCEPT_ENCODING, "UTF\n 8") + .build() + + assert(req.header(ACCEPT_ENCODING).get() == "UTF\n 8") + + decodedRequest(req) + + recordingBackend.verify( + getRequestedFor(urlPathEqualTo("/headers")) + .withHeader(ACCEPT_ENCODING, equalTo("UTF 8")) + ) + } + it("should support new lines in received request headers in obsolete folding format (CRLF)") { + recordingBackend.stub(urlPathEqualTo("/headers"), aResponse.withStatus(200)) + + val req = get("/headers") + .addHeader(HOST, styxServer.proxyHost) + .addHeader(ACCEPT_ENCODING, "UTF\r\n 8") + .build() + + assert(req.header(ACCEPT_ENCODING).get() == "UTF\r\n 8") + + decodedRequest(req) + + recordingBackend.verify( + getRequestedFor(urlPathEqualTo("/headers")) + .withHeader(ACCEPT_ENCODING, equalTo("UTF 8")) + ) + } } describe("handling cookies") { 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