Cookie names should be treatead case-sensitively (ExpediaGroup#186)

* Cookie names should be considered case sensitive. Fixes ExpediaGroup#185
mikkokar · Jun 13, 2018 · 0f9ea5a · 0f9ea5a
1 parent 8e8058b
commit 0f9ea5a
Show file tree

Hide file tree

Showing 10 changed files with 61 additions and 31 deletions.
diff --git a/components/api/src/main/java/com/hotels/styx/api/HttpRequest.java b/components/api/src/main/java/com/hotels/styx/api/HttpRequest.java
@@ -42,6 +42,7 @@
 import static com.hotels.styx.api.HttpHeaderNames.CONTENT_LENGTH;
 import static com.hotels.styx.api.HttpHeaderNames.HOST;
 import static com.hotels.styx.api.HttpHeaderValues.KEEP_ALIVE;
+import static com.hotels.styx.api.support.CookiesSupport.findCookie;
 import static io.netty.handler.codec.http.HttpMethod.CONNECT;
 import static io.netty.handler.codec.http.HttpMethod.DELETE;
 import static io.netty.handler.codec.http.HttpMethod.GET;
@@ -334,9 +335,7 @@ public Iterable<String> queryParamNames() {
      * @return returns an optional cookie object from the header
      */
     public Optional<HttpCookie> cookie(String name) {
-        return cookies().stream()
-                .filter(cookie -> name.equalsIgnoreCase(cookie.name()))
-                .findFirst();
+        return findCookie(cookies, name);
     }
 
     /**
@@ -648,9 +647,7 @@ public Builder addCookie(String name, String value) {
          * @return {@code this}
          */
         public Builder removeCookie(String name) {
-            cookies.stream()
-                    .filter(cookie -> cookie.name().equals(name))
-                    .findFirst()
+            findCookie(cookies, name)
                     .ifPresent(cookies::remove);
 
             return this;

diff --git a/components/api/src/main/java/com/hotels/styx/api/HttpResponse.java b/components/api/src/main/java/com/hotels/styx/api/HttpResponse.java
@@ -37,6 +37,7 @@
 import static com.hotels.styx.api.HttpHeaderNames.CONTENT_LENGTH;
 import static com.hotels.styx.api.HttpHeaderNames.CONTENT_TYPE;
 import static com.hotels.styx.api.HttpMessageBody.NO_BODY;
+import static com.hotels.styx.api.support.CookiesSupport.findCookie;
 import static io.netty.buffer.Unpooled.copiedBuffer;
 import static io.netty.handler.codec.http.HttpResponseStatus.FOUND;
 import static io.netty.handler.codec.http.HttpResponseStatus.MOVED_PERMANENTLY;
@@ -151,9 +152,7 @@ public ImmutableList<HttpCookie> cookies() {
      * @return the cookie if present
      */
     public Optional<HttpCookie> cookie(String name) {
-        return cookies().stream()
-                .filter(cookie -> name.equalsIgnoreCase(cookie.name()))
-                .findFirst();
+        return findCookie(cookies, name);
     }
 
     /**
@@ -515,11 +514,8 @@ public Builder addCookie(String name, String value) {
          * @return {@code this}
          */
         public Builder removeCookie(String name) {
-            cookies.stream()
-                    .filter(cookie -> cookie.name().equalsIgnoreCase(name))
-                    .findFirst()
+            findCookie(cookies, name)
                     .ifPresent(cookie -> cookies.remove(cookie));
-
             return this;
         }
 

diff --git a/components/api/src/main/java/com/hotels/styx/api/messages/FullHttpMessage.java b/components/api/src/main/java/com/hotels/styx/api/messages/FullHttpMessage.java
@@ -25,6 +25,7 @@
 
 import static com.hotels.styx.api.HttpHeaderNames.CONTENT_LENGTH;
 import static com.hotels.styx.api.HttpHeaderNames.CONTENT_TYPE;
+import static com.hotels.styx.api.support.CookiesSupport.findCookie;
 
 /**
  * All behaviour common to both full requests and full responses.
@@ -97,9 +98,7 @@ default List<String> headers(CharSequence name) {
      * @return the cookie if present
      */
     default Optional<HttpCookie> cookie(String name) {
-        return cookies().stream()
-                .filter(cookie -> name.equalsIgnoreCase(cookie.name()))
-                .findFirst();
+        return findCookie(cookies(), name);
     }
 
     /**

diff --git a/components/api/src/main/java/com/hotels/styx/api/messages/FullHttpRequest.java b/components/api/src/main/java/com/hotels/styx/api/messages/FullHttpRequest.java
@@ -48,6 +48,7 @@
 import static com.hotels.styx.api.messages.HttpMethod.httpMethod;
 import static com.hotels.styx.api.messages.HttpVersion.HTTP_1_1;
 import static com.hotels.styx.api.messages.HttpVersion.httpVersion;
+import static com.hotels.styx.api.support.CookiesSupport.findCookie;
 import static com.hotels.styx.api.support.CookiesSupport.isCookieHeader;
 import static java.lang.Integer.parseInt;
 import static java.net.InetSocketAddress.createUnresolved;
@@ -592,9 +593,7 @@ public Builder addCookie(String name, String value) {
          * @return {@code this}
          */
         public Builder removeCookie(String name) {
-            cookies.stream()
-                    .filter(cookie -> cookie.name().equalsIgnoreCase(name))
-                    .findFirst()
+           findCookie(cookies, name)
                     .ifPresent(cookies::remove);
 
             return this;

diff --git a/components/api/src/main/java/com/hotels/styx/api/messages/FullHttpResponse.java b/components/api/src/main/java/com/hotels/styx/api/messages/FullHttpResponse.java
@@ -37,6 +37,7 @@
 import static com.hotels.styx.api.messages.HttpResponseStatus.statusWithCode;
 import static com.hotels.styx.api.messages.HttpVersion.HTTP_1_1;
 import static com.hotels.styx.api.messages.HttpVersion.httpVersion;
+import static com.hotels.styx.api.support.CookiesSupport.findCookie;
 import static java.lang.Integer.parseInt;
 import static java.util.Objects.requireNonNull;
 import static rx.Observable.just;
@@ -361,9 +362,7 @@ public Builder addCookie(String name, String value) {
          * @return {@code this}
          */
         public Builder removeCookie(String name) {
-            cookies.stream()
-                    .filter(cookie -> cookie.name().equalsIgnoreCase(name))
-                    .findFirst()
+            findCookie(cookies, name)
                     .ifPresent(cookies::remove);
 
             return this;

diff --git a/components/api/src/main/java/com/hotels/styx/api/messages/StreamingHttpMessage.java b/components/api/src/main/java/com/hotels/styx/api/messages/StreamingHttpMessage.java
@@ -26,6 +26,7 @@
 
 import static com.hotels.styx.api.HttpHeaderNames.CONTENT_LENGTH;
 import static com.hotels.styx.api.HttpHeaderNames.CONTENT_TYPE;
+import static com.hotels.styx.api.support.CookiesSupport.findCookie;
 
 /**
  * All behaviour common to both streaming requests and streaming responses.
@@ -86,9 +87,7 @@ default List<String> headers(CharSequence name) {
      * @return the cookie if present
      */
     default Optional<HttpCookie> cookie(String name) {
-        return cookies().stream()
-                .filter(cookie -> name.equalsIgnoreCase(cookie.name()))
-                .findFirst();
+        return findCookie(cookies(), name);
     }
 
     /**

diff --git a/components/api/src/main/java/com/hotels/styx/api/messages/StreamingHttpRequest.java b/components/api/src/main/java/com/hotels/styx/api/messages/StreamingHttpRequest.java
@@ -50,6 +50,7 @@
 import static com.hotels.styx.api.messages.HttpMethod.httpMethod;
 import static com.hotels.styx.api.messages.HttpVersion.HTTP_1_1;
 import static com.hotels.styx.api.messages.HttpVersion.httpVersion;
+import static com.hotels.styx.api.support.CookiesSupport.findCookie;
 import static com.hotels.styx.api.support.CookiesSupport.isCookieHeader;
 import static io.netty.buffer.ByteBufUtil.getBytes;
 import static io.netty.buffer.Unpooled.compositeBuffer;
@@ -577,9 +578,7 @@ public Builder addCookie(String name, String value) {
          * @return {@code this}
          */
         public Builder removeCookie(String name) {
-            cookies.stream()
-                    .filter(cookie -> cookie.name().equalsIgnoreCase(name))
-                    .findFirst()
+            findCookie(cookies, name)
                     .ifPresent(cookies::remove);
 
             return this;

diff --git a/components/api/src/main/java/com/hotels/styx/api/messages/StreamingHttpResponse.java b/components/api/src/main/java/com/hotels/styx/api/messages/StreamingHttpResponse.java
@@ -40,6 +40,7 @@
 import static com.hotels.styx.api.messages.HttpResponseStatus.statusWithCode;
 import static com.hotels.styx.api.messages.HttpVersion.HTTP_1_1;
 import static com.hotels.styx.api.messages.HttpVersion.httpVersion;
+import static com.hotels.styx.api.support.CookiesSupport.findCookie;
 import static io.netty.buffer.ByteBufUtil.getBytes;
 import static io.netty.buffer.Unpooled.compositeBuffer;
 import static io.netty.util.ReferenceCountUtil.release;
@@ -320,9 +321,7 @@ public Builder addCookie(String name, String value) {
          * @return {@code this}
          */
         public Builder removeCookie(String name) {
-            cookies.stream()
-                    .filter(cookie -> cookie.name().equalsIgnoreCase(name))
-                    .findFirst()
+           findCookie(cookies, name)
                     .ifPresent(cookies::remove);
 
             return this;

diff --git a/components/api/src/main/java/com/hotels/styx/api/support/CookiesSupport.java b/components/api/src/main/java/com/hotels/styx/api/support/CookiesSupport.java
@@ -15,6 +15,13 @@
  */
 package com.hotels.styx.api.support;
 
+import com.hotels.styx.api.HttpCookie;
+
+import java.util.Collection;
+import java.util.Optional;
+
+import static java.util.Objects.requireNonNull;
+
 /**
  * Does some validation for cookie header names.
  */
@@ -31,4 +38,20 @@ private CookiesSupport() {
     public static boolean isCookieHeader(String header) {
         return "Set-Cookie".equalsIgnoreCase(header) || "Cookie".equalsIgnoreCase(header);
     }
+
+    /**
+     * Find a cookie with the specified {@code name}.
+     *
+     * @param cookies list of cookies
+     * @param name cookie name
+     * @return the cookie if present
+     */
+    public static Optional<HttpCookie> findCookie(Collection<HttpCookie> cookies, String name){
+        requireNonNull(cookies);
+        requireNonNull(name);
+        return cookies
+                .stream()
+                .filter(cookie -> name.equals(cookie.name()))
+                .findFirst();
+    }
 }
diff --git a/components/api/src/test/java/com/hotels/styx/api/support/CookiesSupportTest.java b/components/api/src/test/java/com/hotels/styx/api/support/CookiesSupportTest.java
@@ -15,8 +15,15 @@
  */
 package com.hotels.styx.api.support;
 
+import com.google.common.collect.ImmutableList;
+import com.hotels.styx.api.HttpCookie;
+import com.hotels.styx.api.HttpCookieAttribute;
 import org.testng.annotations.Test;
 
+import java.util.List;
+import java.util.Optional;
+
+import static com.hotels.styx.api.support.CookiesSupport.findCookie;
 import static com.hotels.styx.api.support.CookiesSupport.isCookieHeader;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.core.Is.is;
@@ -35,4 +42,17 @@ public void onlyRequestAndResponseCookieHeaderIsAllowed() {
         assertThat(isCookieHeader("cookiex"), is(false));
     }
 
+    @Test
+    public void canFindCookieName(){
+        HttpCookie expectedCookie = HttpCookie.cookie("cookie1", "value1", HttpCookieAttribute.httpOnly());
+        List<HttpCookie> cookies = ImmutableList.of(expectedCookie);
+        assertThat(findCookie(cookies, expectedCookie.name()),is(Optional.of(expectedCookie)));
+    }
+
+    @Test
+    public void cookieNamesAreCaseSensitive(){
+        HttpCookie expectedCookie = HttpCookie.cookie("cookie1", "value1", HttpCookieAttribute.httpOnly());
+        List<HttpCookie> cookies = ImmutableList.of(expectedCookie);
+        assertThat(findCookie(cookies, expectedCookie.name().toUpperCase()),is(Optional.empty()));
+    }
 }