finagle-base-http: Remove a side effect of c.t.f.http.Message#cookies

Problem

Accessing the `cookies` field of a `Message` may add an empty `Cookie` entry
in its `headerMap`:
```
import com.twitter.finagle.http.Request

val request = Request()
request.headerMap // Map()
request.cookies
request.headerMap // Map(Cookie -> )
```
This behavior is unexpected because `.cookies` should be without any side
effect.

It only happens when the lazy `cookies` field of a `Message` is accessed for
the first time, i.e. during the initialization of a `CookieMap`.

Solution

Call the header resetting logic in `CookieMap#rewriteCookieHeaders` only
when there exist some cookie(s) in the cookie map.

This solution not only resolves the problem above, but also removes the
`Cookie` header of a `Message` whenever its cookie map becomes empty.

JIRA Issues: CSL-8743

Differential Revision: https://phabricator.twitter.biz/D361326

CHANGELOG.rst

@@ -44,6 +44,12 @@ Runtime Behavior Changes
   the Ping Failure Detection implementation has been removed completely from the
   non-MultiplexHandler based HTTP/2 client. ``PHAB_ID=D360712``
 
+Bug Fixes
+~~~~~~~~~
+
+* finagle-base-http: Removes the `Cookie` header of a `c.t.f.http.Message` whenever its cookie map
+  becomes empty. ``PHAB_ID=D361326``
+
 19.8.0
 ------
 

finagle-base-http/src/main/scala/com/twitter/finagle/http/CookieMap.scala

@@ -61,13 +61,15 @@ class CookieMap private[finagle] (message: Message, cookieCodec: CookieCodec)
     // Clear all cookies - there may be more than one with this name.
     message.headerMap.remove(cookieHeaderName)
 
-    // Add cookies back again
-    if (message.isRequest) {
-      message.headerMap.set(cookieHeaderName, cookieCodec.encodeClient(values))
-    } else {
-      foreach {
-        case (_, cookie) =>
-          message.headerMap.add(cookieHeaderName, cookieCodec.encodeServer(cookie))
+    // Add cookies back again if there are any
+    if (nonEmpty) {
+      if (message.isRequest) {
+        message.headerMap.set(cookieHeaderName, cookieCodec.encodeClient(values))
+      } else {
+        foreach {
+          case (_, cookie) =>
+            message.headerMap.add(cookieHeaderName, cookieCodec.encodeServer(cookie))
+        }
       }
     }
   }

finagle-base-http/src/test/scala/com/twitter/finagle/http/CookieMapTest.scala

@@ -16,6 +16,12 @@ abstract class CookieMapTest(codec: CookieCodec, codecName: String) extends FunS
       assert(cookieMap.isEmpty)
     }
 
+    test(s"$codec: Accessing cookies of a $messageType does not change its header") {
+      val message = newMessage()
+      message.cookies
+      assert(message.headerMap.isEmpty)
+    }
+
     test(s"$codec: Invalid cookies on a $messageType are ignored") {
       val message = newMessage()
       lazy val cookieMap = new CookieMap(message, codec)
@@ -122,6 +128,15 @@ abstract class CookieMapTest(codec: CookieCodec, codecName: String) extends FunS
 
       assert(cookieMap.size == 0)
     }
+
+    test(s"$codec: Removing all cookies of a $messageType also removes its Cookie header") {
+      val message = newMessage()
+      message.headerMap.add("Cookie", "name=value")
+      lazy val cookieMap = new CookieMap(message, codec)
+      cookieMap -= "name"
+
+      assert(!message.headerMap.contains(headerName))
+    }
   }
 
   // Request tests