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

Patch #186 into styx 1.0 branch. * Cookie names should be treatead case-sensitively (#186) * Cookie names should be considered case sensitive. Fixes #185 (cherry picked from commit 0f9ea5a)
ExpediaGroup · Jun 18, 2018 · f115f5f · f115f5f
1 parent f2219f5
commit f115f5f
Show file tree

Hide file tree

Showing 8 changed files with 74 additions and 34 deletions.
diff --git a/components/api/src/main/java/com/hotels/styx/api/FullHttpMessage.java b/components/api/src/main/java/com/hotels/styx/api/FullHttpMessage.java
@@ -23,6 +23,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.
@@ -95,9 +96,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/FullHttpRequest.java b/components/api/src/main/java/com/hotels/styx/api/FullHttpRequest.java
@@ -43,6 +43,7 @@
 import static com.hotels.styx.api.messages.HttpMethod.POST;
 import static com.hotels.styx.api.messages.HttpMethod.PUT;
 import static com.hotels.styx.api.messages.HttpVersion.HTTP_1_1;
+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;
@@ -576,9 +577,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/FullHttpResponse.java b/components/api/src/main/java/com/hotels/styx/api/FullHttpResponse.java
@@ -35,6 +35,7 @@
 import static com.hotels.styx.api.HttpHeaderValues.CHUNKED;
 import static com.hotels.styx.api.messages.HttpResponseStatus.OK;
 import static com.hotels.styx.api.messages.HttpVersion.HTTP_1_1;
+import static com.hotels.styx.api.support.CookiesSupport.findCookie;
 import static io.netty.buffer.Unpooled.copiedBuffer;
 import static java.lang.Integer.parseInt;
 import static java.util.Objects.requireNonNull;
@@ -216,14 +217,6 @@ public Builder(FullHttpResponse response) {
             this.cookies = new ArrayList<>(response.cookies());
         }
 
-//        public Builder(HttpResponse response, byte[] encodedBody) {
-//            this.status = statusWithCode(response.status().code());
-//            this.version = httpVersion(response.version().toString());
-//            this.headers = response.headers().newBuilder();
-//            this.body = encodedBody;
-//            this.cookies = new ArrayList<>(response.cookies());
-//        }
-
         public Builder(HttpResponse response, byte[] decoded) {
             this.status = response.status();
             this.version = response.version();
@@ -359,9 +352,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/HttpRequest.java b/components/api/src/main/java/com/hotels/styx/api/HttpRequest.java
@@ -45,6 +45,7 @@
 import static com.hotels.styx.api.messages.HttpMethod.PATCH;
 import static com.hotels.styx.api.messages.HttpMethod.POST;
 import static com.hotels.styx.api.messages.HttpMethod.PUT;
+import static com.hotels.styx.api.support.CookiesSupport.findCookie;
 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;
@@ -150,9 +151,9 @@ public static Builder patch(String uri) {
     /**
      * Creates a request with the POST method.
      *
-     * @param uri URI
+     * @param uri  URI
      * @param body body
-     * @param  body type
+     * @param body type
      * @return {@code this}
      */
     public static Builder post(String uri, StyxObservable<ByteBuf> body) {
@@ -162,9 +163,9 @@ public static Builder post(String uri, StyxObservable<ByteBuf> body) {
     /**
      * Creates a request with the PUT method.
      *
-     * @param uri URI
+     * @param uri  URI
      * @param body body
-     * @param  body type
+     * @param body type
      * @return {@code this}
      */
     public static Builder put(String uri, StyxObservable<ByteBuf> body) {
@@ -174,9 +175,9 @@ public static Builder put(String uri, StyxObservable<ByteBuf> body) {
     /**
      * Creates a request with the PATCH method.
      *
-     * @param uri URI
+     * @param uri  URI
      * @param body body
-     * @param  body type
+     * @param body type
      * @return {@code this}
      */
     public static Builder patch(String uri, StyxObservable<ByteBuf> body) {
@@ -198,6 +199,18 @@ public List<HttpCookie> cookies() {
         return cookies;
     }
 
+    /*
+     * Returns an {@link Optional} containing the {@link HttpCookie} with the specified {@code name}
+     * if such a cookie exists.
+     *
+     * @param name the name of the cookie
+     * @return returns an optional cookie object from the header
+     */
+    public Optional<HttpCookie> cookie(String name) {
+        return findCookie(cookies, name);
+    }
+
+
     @Override
     public List<String> headers(CharSequence name) {
         return headers.getAll(name);
@@ -614,13 +627,11 @@ public Builder addCookie(String name, String value) {
         /**
          * Removes a cookie if present (removes its Set-Cookie header).
          *
-         * @param name name of the cookie
+         * @param name cookie name
          * @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/HttpResponse.java b/components/api/src/main/java/com/hotels/styx/api/HttpResponse.java
@@ -44,6 +44,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.buffer.Unpooled.copiedBuffer;
@@ -366,11 +367,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(cookies::remove);
-
             return this;
         }
 

diff --git a/components/api/src/main/java/com/hotels/styx/api/StreamingHttpMessage.java b/components/api/src/main/java/com/hotels/styx/api/StreamingHttpMessage.java
@@ -23,6 +23,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.
@@ -83,9 +84,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/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()));
+    }
 }