finagle-base-http: Decouple Cookie from Netty 3 Cookie

Summary: Problem

In order to remove Netty 3, we need to decouple Cookie from
Netty 3 Cookie.

Solution

Cookie is no longer backed by a Netty 3 cookie. We are moving
towards making Cookie immutable, so we deprecate all set methods
(a "CookieBuilder" will be introduced in another commit). We add
bijections to go from one type to the other.

JIRA Issues: CSL-3669

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

CHANGES

@@ -14,6 +14,30 @@ Runtime Behavior Changes
     validate loaded certificates in all cases to ensure that the current date
     range is within the validity range specified in the certificate. ``PHAB_ID=D88664``
 
+Deprecations
+~~~~~~~~~~~~
+
+  * finagle-base-http: With the intention to make `c.t.f.http.Cookie` immutable,
+  `set` methods on `c.t.f.http.Cookie` have been deprecated:
+      - `comment_=`
+      - `commentUrl_=`
+      - `domain_=`
+      - `maxAge_=`
+      - `path_=`
+      - `ports_=`
+      - `value_=`
+      - `version_=`
+      - `httpOnly_=`
+      - `isDiscard_=`
+      - `isSecure_=`
+    Use the `c.t.f.http.Cookie` to set `domain`, `maxAge`, `path`, `value`, `httpOnly`,
+    and `secure`. `comment`, `commentUrl`, `ports`, `version`, and `discard` have been removed
+    per RFC-6265. ``PHAB_ID=D82164``
+
+  * finagle-base-http: `c.t.f.http.Cookie.isSecure` and `c.t.f.http.Cookie.isDiscard`
+    have been deprecated. Use `c.t.f.http.Cookie.secure` for `c.t.f.http.Cookie.isSecure`.
+    `isDiscard` has been removed per RFC-6265. ``PHAB_ID=D82164``
+
 7.1.0
 ------
 
@@ -82,6 +106,9 @@ Breaking API Changes
   * finagle-memcached: Remove deprecated method `BaseClient.release()`. Use
     `BaseClient.close()` instead. ``PHAB_ID=D83168``
 
+Deprecations
+~~~~~~~~~~~~
+
   * finagle-memcached: Move `c.t.f.memcached.java.Client` to `c.t.f.memcached.JavaClient`,
     `c.t.f.memcached.java.ClientBase` to `c.t.f.memcached.JavaClientBase`, and
     `c.t.f.memcached.java.ResultWithCAS` to `c.t.f.memcached.ResultWithCAS`. ``PHAB_ID=D83719``

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

@@ -2,46 +2,296 @@ package com.twitter.finagle.http
 
 import com.twitter.conversions.time._
 import com.twitter.util.Duration
-import org.jboss.netty.handler.codec.http.{Cookie => NettyCookie, DefaultCookie}
+import java.util.{BitSet => JBitSet}
+import org.jboss.netty.handler.codec.http.{Cookie => NettyCookie}
 import scala.collection.JavaConverters._
 
-/** Scala wrapper around Netty cookies. */
-class Cookie(private[http] val underlying: NettyCookie) {
-  def this(name: String, value: String) = {
-    this(new DefaultCookie(name, value))
-  }
-
-  def comment: String = underlying.getComment
-  def commentUrl: String = underlying.getCommentUrl
-  def domain: String = underlying.getDomain
-  def maxAge: Duration = underlying.getMaxAge.seconds
-  def name: String = underlying.getName
-  def path: String = underlying.getPath
-  def ports: Set[Int] = underlying.getPorts.asScala.toSet map { i: Integer =>
-    i.intValue
-  }
-  def value: String = underlying.getValue
-  def version: Int = underlying.getVersion
-  def httpOnly: Boolean = underlying.isHttpOnly
-  def isDiscard: Boolean = underlying.isDiscard
-  def isSecure: Boolean = underlying.isSecure
-
-  def comment_=(comment: String) { underlying.setComment(comment) }
-  def commentUrl_=(commentUrl: String) { underlying.setCommentUrl(commentUrl) }
-  def domain_=(domain: String) { underlying.setDomain(domain) }
-  def maxAge_=(maxAge: Duration) { underlying.setMaxAge(maxAge.inSeconds) }
-  def path_=(path: String) { underlying.setPath(path) }
-  def ports_=(ports: Seq[Int]) { underlying.setPorts(ports: _*) }
-  def value_=(value: String) { underlying.setValue(value) }
-  def version_=(version: Int) { underlying.setVersion(version) }
-  def httpOnly_=(httpOnly: Boolean) { underlying.setHttpOnly(httpOnly) }
-  def isDiscard_=(discard: Boolean) { underlying.setDiscard(discard) }
-  def isSecure_=(secure: Boolean) { underlying.setSecure(secure) }
+object Cookie {
 
+  private val DefaultMaxAge = Int.MinValue.seconds // Netty's DefaultCookie default.
+
+  private[this] val IllegalNameChars = Set('\t', '\n', '\u000b', '\f', '\r', ' ', ',', ';', '=')
+  private[this] val IllegalValueChars = Set('\n', '\u000b', '\f', '\r', ';')
+
+  private[this] val IllegalNameCharsBitSet: JBitSet = {
+    val bs = new JBitSet
+    IllegalNameChars.foreach(bs.set(_))
+    bs
+  }
+
+  private[this] val IllegalValueCharsBitSet: JBitSet = {
+    val bs = new JBitSet
+    IllegalValueChars.foreach(bs.set(_))
+    bs
+  }
+
+  private[this] def stringContains(string: String, chars: JBitSet): Boolean = {
+    var i = 0
+    val until = string.length
+    while (i < until && !chars.get(string.charAt(i))) {
+      i += 1
+    }
+    i != until
+  }
+
+  // These are the same checks made by Netty's DefaultCookie
+  private def validateName(name: String): String = {
+    val trimmed = name.trim
+    if (trimmed.isEmpty) throw new IllegalArgumentException("Cookie name cannot be empty")
+    else {
+      if (trimmed.head == '$')
+        throw new IllegalArgumentException(
+          s"Cookie name starting with '$$' not allowed: $trimmed"
+        )
+      else if (stringContains(trimmed, IllegalNameCharsBitSet))
+        throw new IllegalArgumentException(
+          s"Cookie name contains one of the following prohibited characters: ${IllegalNameChars
+            .mkString(",")}: $trimmed"
+        )
+      else
+        trimmed
+    }
+  }
+
+  // These are the same checks made by Netty's DefaultCookie
+  private def validateField(field: String): String = {
+    if (field == null) field
+    else {
+      val trimmed = field.trim
+      if (trimmed.isEmpty) {
+        null
+      } else if (stringContains(trimmed, IllegalValueCharsBitSet))
+        throw new IllegalArgumentException(
+          s"Cookie field contains one of the following prohibited characters: ${IllegalValueChars
+            .mkString(",")}: $trimmed"
+        )
+      else trimmed
+    }
+  }
+
+  private[this] val invalidPort: Int => Boolean = p => p <= 0 || p > '\uffff'
+
+  // These are the same checks made by Netty's DefaultCookie
+  private def validatePorts(ports: Set[Int]): Set[Int] = {
+    val p = ports.find(invalidPort)
+    if (p.isDefined)
+      throw new IllegalArgumentException("Cookie port out of range: " + p.get);
+    ports
+  }
+}
+
+// A small amount of naming hopscotch is needed while we deprecate the `set` methods in preparation
+// for removal; the param names can't conflict with the `get` method names, hence the underscored
+// param names.
+//
+// `_forDeprecation` is an unused param; it's needed so we can offer a unique constructor (below)
+// that takes all fields with the proper names while we deprecate the `set` methods. Once we remove
+// the `set` methods, we can clean this up.
+class Cookie private (
+  private[this] var _name: String,
+  private[this] var _value: String,
+  private[this] var _domain: String,
+  private[this] var _path: String,
+  private[this] var _comment: String,
+  private[this] var _commentUrl: String,
+  private[this] var _discard: Boolean,
+  private[this] var _ports: Set[Int],
+  private[this] var _maxAge: Duration,
+  private[this] var _version: Int,
+  private[this] var _secure: Boolean,
+  private[this] var _httpOnly: Boolean,
+  private[this] var _forDeprecation: Boolean
+) {
+
+  /**
+   * Create a cookie.
+   */
+  def this(
+    name: String,
+    value: String,
+    domain: Option[String] = None,
+    path: Option[String] = None,
+    maxAge: Option[Duration] = Some(Cookie.DefaultMaxAge),
+    secure: Boolean = false,
+    httpOnly: Boolean = false
+  ) = this(
+    _name = Cookie.validateName(name),
+    _value = value,
+    _domain = Cookie.validateField(domain.orNull),
+    _path = Cookie.validateField(path.orNull),
+    null,
+    null,
+    false,
+    Set.empty,
+    maxAge.orNull,
+    0,
+    secure,
+    httpOnly,
+    true
+  )
+
+  def this(
+    name: String,
+    value: String
+  ) = this(
+    name,
+    value,
+    None,
+    None,
+    Some(Cookie.DefaultMaxAge),
+    false,
+    false
+  )
+
+  @deprecated("Use Bijections.from to create a Cookie from a Netty Cookie")
+  def this(underlying: NettyCookie) = {
+    this(
+      underlying.getName,
+      underlying.getValue,
+      underlying.getDomain,
+      underlying.getPath,
+      underlying.getComment,
+      underlying.getCommentUrl,
+      underlying.isDiscard,
+      underlying.getPorts.asScala.toSet.map { i: Integer =>
+        i.intValue
+      },
+      underlying.getMaxAge.seconds,
+      underlying.getVersion,
+      underlying.isSecure,
+      underlying.isHttpOnly,
+      true
+    )
+  }
+
+  /**
+   * Get the comment.
+   * @note May be null.
+   */
+  def comment: String = _comment
+
+  /**
+   * Get the commentUrl.
+   * @note May be null.
+   */
+  def commentUrl: String = _commentUrl
+
+  /**
+   * Get the domain.
+   * @note May be null.
+   */
+  def domain: String = _domain
+
+  /**
+   * Get the path.
+   * @note May be null.
+   */
+  def path: String = _path
+  def name: String = _name
+  def value: String = _value
+  def maxAge: Duration = _maxAge
+  def ports: Set[Int] = _ports
+  def version: Int = _version
+  def httpOnly: Boolean = _httpOnly
+  def discard: Boolean = _discard
+  def secure: Boolean = _secure
+
+  @deprecated("Removed per RFC-6265", "2017-08-16")
+  def isDiscard: Boolean = _discard
+
+  @deprecated("Use secure instead", "2017-08-16")
+  def isSecure: Boolean = _secure
+
+  /**
+   * Set the comment.
+   * @note `comment` may be null.
+   */
+  @deprecated("Removed per RFC-6265", "2017-08-16")
+  def comment_=(comment: String): Unit =
+    _comment = Cookie.validateField(comment)
+
+  /**
+   * Set the commentUrl.
+   * @note `commentUrl` may be null.
+   */
+  @deprecated("Removed per RFC-6265", "2017-08-16")
+  def commentUrl_=(commentUrl: String): Unit =
+    _commentUrl = Cookie.validateField(commentUrl)
+
+  /**
+   * Set the domain.
+   * @note `domain` may be null.
+   */
+  @deprecated("Set domain in the Cookie constructor", "2017-08-16")
+  def domain_=(domain: String): Unit =
+    _domain = Cookie.validateField(domain)
+
+  @deprecated("Set maxAge in the Cookie constructor", "2017-08-16")
+  def maxAge_=(maxAge: Duration): Unit =
+    _maxAge = maxAge
+
+  /**
+   * Set the path.
+   * @note `path` may be null.
+   */
+  @deprecated("Set path in the Cookie constructor", "2017-08-16")
+  def path_=(path: String): Unit =
+    _path = Cookie.validateField(path)
+
+  @deprecated("Removed per RFC-6265", "2017-08-16")
+  def ports_=(ports: Seq[Int]): Unit =
+    _ports = Cookie.validatePorts(ports.toSet)
+
+  /**
+   * Set the value.
+   * @note `value` must not be null.
+   */
+  @deprecated("Set value in the Cookie constructor", "2017-08-16")
+  def value_=(value: String): Unit =
+    _value = value
+
+  @deprecated("Removed per RFC-6265", "2017-08-16")
+  def version_=(version: Int): Unit =
+    _version = version
+
+  @deprecated("Set httpOnly in the Cookie constructor", "2017-08-16")
+  def httpOnly_=(httpOnly: Boolean): Unit =
+    _httpOnly = httpOnly
+
+  @deprecated("Removed per RFC-6265", "2017-08-16")
+  def isDiscard_=(discard: Boolean): Unit =
+    _discard = discard
+
+  @deprecated("Set secure in the Cookie constructor", "2017-08-16")
+  def isSecure_=(secure: Boolean): Unit =
+    _secure = secure
+
+  // Helper method for `equals` that returns true if two strings are both null, or have the
+  // same value (ignoring case)
+  private[this] def stringsEqual(s0: String, s1: String): Boolean = {
+    if (s0 == null) {
+      if (s1 == null) true
+      else false
+    } else {
+      if (s1 == null) false
+      else s0.equalsIgnoreCase(s1)
+    }
+  }
+
+  /**
+   * Returns true if `obj` equals `this`. Two cookies are considered equal if their names,
+   * paths, and domains are the same (ignoring case). This mimics the `equals` method on Netty's
+   * DefaultCookie.
+   */
   override def equals(obj: Any): Boolean = obj match {
-    case c: Cookie => underlying.equals(c.underlying)
+    case c: Cookie =>
+      name.equalsIgnoreCase(c.name) && stringsEqual(path, c.path) && stringsEqual(domain, c.domain)
     case _ => false
   }
 
-  override def hashCode(): Int = underlying.hashCode()
+  /**
+   * From Netty 3's DefaultCookie
+   */
+  override def hashCode(): Int =
+    name.hashCode
 }