Repository: eclipse-vertx/vert.x

Problem statement:
addCookie method does not replace existing cookie (from browser) and fails when calling RoutingContext.cookieMap()
### Version

4.2.1

### Context

We were using (in version 3.9.9) HttpServerResponse.addCookie to set cookie irrespective of whether the cookie was already present (came from browser) and when we iterate over cookies using RoutingContext.cookieMap() we would get a single cookie with that name (i.e. cookie info was updated)
After upgrading to 4.2.1, calling RoutingContext.cookieMap() fails with this exception stack trace


This method assumes cookies are unique by name. However the implementation for addCookie() always adds a cookie to collection (this seems to be a feature request).

```
default Map<String, Cookie> cookieMap() {
    return cookies()
      .stream()
      .collect(Collectors.toMap(Cookie::getName, cookie -> cookie));
  }
```

### Do you have a reproducer?

No. 

### Steps to reproduce

Create sample web app that renders simple form and addCookie("test", "somevalue"). On form submit, call addCookie("test", "anothervalie") to modify the cookie value without removing it first. Then invoke RoutingContext.cookieMap() to get all cookies.

### Extra





Hints:
It would be good to have setCookie or replaceCookie method that would have the previous semantics (add or replace by cookie name) for convenience 
@narras-oss I'm not following the error:

```java
  @Test
  public void testReplaceCookie() throws Exception {
    // receive a HTTP request with a cookie header with value `test=somevalue` 
    testCookies("test=somevalue", req -> {
      // OK
      assertEquals(1, req.cookieCount());
      // is it supposed to fail here? It currently (master) doesn't
      req.response().addCookie(Cookie.cookie("test", "othervalue"));
      Map<String, Cookie> deprecatedMap = req.cookieMap();
      req.response().end();
    }, resp -> {
      // received 1 cookie back (the new one replaced the one received)
      // test=othervalue
      // ?? what was expected?
      List<String> cookies = resp.headers().getAll("set-cookie");
    });
  }
```
This is the code i am specifically using

```
Cookie cookie = Cookie.cookie("test", "somestring")
cookie.setSecure(true);
cookie.setPath("/");
routingContext.addCookie(cookie);
```

Stack trace of exception when calling routingContext.cookieMap() after that is below.

```
Caused by: java.lang.IllegalStateException: Duplicate key XSRF-TOKEN (attempted merging values io.vertx.core.http.impl.CookieImpl@434aa077 and io.vertx.core.http.impl.CookieImpl@4dd9a0b1)
	at java.base/java.util.stream.Collectors.duplicateKeyException(Unknown Source)
	at java.base/java.util.stream.Collectors.lambda$uniqKeysMapAccumulator$1(Unknown Source)
	at java.base/java.util.stream.ReduceOps$3ReducingSink.accept(Unknown Source)
	at java.base/java.util.ArrayList$Itr.forEachRemaining(Unknown Source)
	at java.base/java.util.Spliterators$IteratorSpliterator.forEachRemaining(Unknown Source)
	at java.base/java.util.stream.AbstractPipeline.copyInto(Unknown Source)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(Unknown Source)
	at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(Unknown Source)
	at java.base/java.util.stream.AbstractPipeline.evaluate(Unknown Source)
	at java.base/java.util.stream.ReferencePipeline.collect(Unknown Source)
	at io.vertx.core.http.HttpServerRequest.cookieMap(HttpServerRequest.java:518)
	at io.vertx.ext.web.impl.RoutingContextImpl.cookieMap(RoutingContextImpl.java:282)
	at io.vertx.ext.web.impl.RoutingContextDecorator.cookieMap(RoutingContextDecorator.java:84)
```

Updated steps for my specific scenario:

Specifically this is the original value of that comes from browser
XSRF-TOKEN=c359b44aef83415

Then when you try to add a different value for cookie "XSRF-TOKEN" with path set to "/" like shown below
XSRF-TOKEN=88533580000c314, path=/

it is added again instead of being replaced. The bug seems to be here. cookieUniqueIdComparator returns < 0 for XSRF-TOKEN since it also compares path. This is a recent change (probably intentional). but it breaks the deprecated method request.cookieMap()

```
public boolean add(ServerCookie cookie) {
    if (cookie == null) {
      throw new NullPointerException("cookie cannot be null");
    }

for (int i = 0; i < list.size(); i++) {
      int cmp = cookieUniqueIdComparator(list.get(i), cookie.getName(), cookie.getDomain(), cookie.getPath());

      if (cmp > 0) {
        // insert
        list.add(i, cookie);
        return true;
      }
      if (cmp == 0) {
        // replace
        list.set(i, cookie);
        return true;
      }
    }
    // reached the end
    list.add(cookie);
```

Patch:
diff --git a/src/main/java/io/vertx/core/http/impl/CookieImpl.java b/src/main/java/io/vertx/core/http/impl/CookieImpl.java
--- a/src/main/java/io/vertx/core/http/impl/CookieImpl.java
+++ b/src/main/java/io/vertx/core/http/impl/CookieImpl.java
@@ -26,17 +26,26 @@
 public class CookieImpl implements ServerCookie {
 
   private final io.netty.handler.codec.http.cookie.Cookie nettyCookie;
+  // denotes if a cookie has been created from an HTTP request (true) or during the
+  // application/response life cycle (false)
+  private final boolean fromUserAgent;
+
   private boolean changed;
-  private boolean fromUserAgent;
-  // extension features
+  // extension feature(s)
   private CookieSameSite sameSite;
 
   public CookieImpl(String name, String value) {
     this.nettyCookie = new DefaultCookie(name, value);
+    fromUserAgent = false;
     this.changed = true;
   }
 
-  public CookieImpl(io.netty.handler.codec.http.cookie.Cookie nettyCookie) {
+  /**
+   * Internal constructor, only used by the CookieJar.
+   *
+   * @param nettyCookie the underlying cookie object
+   */
+  CookieImpl(io.netty.handler.codec.http.cookie.Cookie nettyCookie) {
     this.nettyCookie = nettyCookie;
     fromUserAgent = true;
   }
diff --git a/src/main/java/io/vertx/core/http/impl/CookieJar.java b/src/main/java/io/vertx/core/http/impl/CookieJar.java
--- a/src/main/java/io/vertx/core/http/impl/CookieJar.java
+++ b/src/main/java/io/vertx/core/http/impl/CookieJar.java
@@ -143,35 +143,47 @@ private static int cookieUniqueIdComparator(ServerCookie cookie, String name, St
     Objects.requireNonNull(name);
 
     int v = cookie.getName().compareTo(name);
-    if (v != 0) {
-      return v;
-    }
 
-    if (cookie.getPath() == null) {
-      if (path != null) {
-        return -1;
-      }
-    } else if (path == null) {
-      return 1;
+    if (cookie.isFromUserAgent()) {
+      // user-agent cookies never include a path or domain, so we must assume equality
+      // just by comparing the name
+      return v;
     } else {
-      v = cookie.getPath().compareTo(path);
+      // perform the tuple check:
+
+      // 1. name comparison (on equals check the next parameter)
       if (v != 0) {
         return v;
       }
-    }
 
-    if (cookie.getDomain() == null) {
-      if (domain != null) {
-        return -1;
+      // 2. path comparison (on equals check the next parameter)
+      if (cookie.getPath() == null) {
+        if (path != null) {
+          return -1;
+        }
+      } else if (path == null) {
+        return 1;
+      } else {
+        v = cookie.getPath().compareTo(path);
+        if (v != 0) {
+          return v;
+        }
+      }
+
+      // 3. domain comparison (on equals terminate with 0)
+      if (cookie.getDomain() == null) {
+        if (domain != null) {
+          return -1;
+        }
+      } else if (domain == null) {
+        return 1;
+      } else {
+        v = cookie.getDomain().compareToIgnoreCase(domain);
+        return v;
       }
-    } else if (domain == null) {
-      return 1;
-    } else {
-      v = cookie.getDomain().compareToIgnoreCase(domain);
-      return v;
-    }
 
-    return 0;
+      return 0;
+    }
   }
 
 


Test patch:
diff --git a/src/test/java/io/vertx/core/http/HttpTest.java b/src/test/java/io/vertx/core/http/HttpTest.java
--- a/src/test/java/io/vertx/core/http/HttpTest.java
+++ b/src/test/java/io/vertx/core/http/HttpTest.java
@@ -23,6 +23,7 @@
 import io.vertx.core.file.AsyncFile;
 import io.vertx.core.http.impl.CookieImpl;
 import io.vertx.core.http.impl.HttpServerRequestInternal;
+import io.vertx.core.http.impl.ServerCookie;
 import io.vertx.core.impl.Utils;
 import io.vertx.core.net.*;
 import io.vertx.core.net.impl.HAProxyMessageCompletionHandler;
@@ -6035,6 +6036,24 @@ public void testNoCookiesAddCookie() throws Exception {
     });
   }
 
+  @Test
+  public void testReplaceCookie() throws Exception {
+    testCookies("XSRF-TOKEN=c359b44aef83415", req -> {
+      assertEquals(1, req.cookieCount());
+      req.response().addCookie(Cookie.cookie("XSRF-TOKEN", "88533580000c314").setPath("/"));
+      Map<String, Cookie> deprecatedMap = req.cookieMap();
+      assertFalse(((ServerCookie) deprecatedMap.get("XSRF-TOKEN")).isFromUserAgent());
+      assertEquals("/", deprecatedMap.get("XSRF-TOKEN").getPath());
+      req.response().end();
+    }, resp -> {
+      List<String> cookies = resp.headers().getAll("set-cookie");
+      // the expired cookie must be sent back
+      assertEquals(1, cookies.size());
+      // ensure that the cookie jar was updated correctly
+      assertEquals("XSRF-TOKEN=88533580000c314; Path=/", cookies.get(0));
+    });
+  }
+
   private void testCookies(String cookieHeader, Consumer<HttpServerRequest> serverChecker, Consumer<HttpClientResponse> clientChecker) throws Exception {
     server.requestHandler(serverChecker::accept);
     startServer(testAddress);